Skip to content

Commit 6453811

Browse files
CyrusNajmabadiCyrusNajmabadi
committed
Merge pull request microsoft#1981 from Microsoft/incrementalChecks.ts
Add additional aggressive checks during incremental parsing.
2 parents ff31b96 + d0aa789 commit 6453811

11 files changed

+128
-63
lines changed

‎src/compiler/parser.ts

+93-19
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,16 @@ module ts {
336336
}
337337

338338
function fixupParentReferences(sourceFile: SourceFile) {
339-
// normally parent references are set during binding.
340-
// however here SourceFile data is used only for syntactic features so running the whole binding process is an overhead.
341-
// walk over the nodes and set parent references
339+
// normally parent references are set during binding. However, for clients that only need
340+
// a syntax tree, and no semantic features, then the binding process is an unnecessary
341+
// overhead. This functions allows us to set all the parents, without all the expense of
342+
// binding.
343+
342344
var parent: Node = sourceFile;
343-
function walk(n: Node): void {
345+
forEachChild(sourceFile, visitNode);
346+
return;
347+
348+
function visitNode(n: Node): void {
344349
// walk down setting parents that differ from the parent we think it should be. This
345350
// allows us to quickly bail out of setting parents for subtrees during incremental
346351
// parsing
@@ -349,30 +354,49 @@ module ts {
349354

350355
var saveParent = parent;
351356
parent = n;
352-
forEachChild(n, walk);
357+
forEachChild(n, visitNode);
353358
parent = saveParent;
354359
}
355360
}
361+
}
362+
363+
function shouldCheckNode(node: Node) {
364+
switch (node.kind) {
365+
case SyntaxKind.StringLiteral:
366+
case SyntaxKind.NumericLiteral:
367+
case SyntaxKind.Identifier:
368+
return true;
369+
}
356370

357-
forEachChild(sourceFile, walk);
371+
return false;
358372
}
359373

360-
function moveElementEntirelyPastChangeRange(element: IncrementalElement, delta: number) {
374+
function moveElementEntirelyPastChangeRange(element: IncrementalElement, delta: number, oldText: string, newText: string, aggressiveChecks: boolean) {
361375
if (element.length) {
362376
visitArray(<IncrementalNodeArray>element);
363377
}
364378
else {
365379
visitNode(<IncrementalNode>element);
366380
}
381+
return;
367382

368383
function visitNode(node: IncrementalNode) {
384+
if (aggressiveChecks && shouldCheckNode(node)) {
385+
var text = oldText.substring(node.pos, node.end);
386+
}
387+
369388
// Ditch any existing LS children we may have created. This way we can avoid
370389
// moving them forward.
371390
node._children = undefined;
372391
node.pos += delta;
373392
node.end += delta;
374393

394+
if (aggressiveChecks && shouldCheckNode(node)) {
395+
Debug.assert(text === newText.substring(node.pos, node.end));
396+
}
397+
375398
forEachChild(node, visitNode, visitArray);
399+
checkNodePositions(node, aggressiveChecks);
376400
}
377401

378402
function visitArray(array: IncrementalNodeArray) {
@@ -459,14 +483,35 @@ module ts {
459483
}
460484
}
461485

462-
function updateTokenPositionsAndMarkElements(node: IncrementalNode, changeStart: number, changeRangeOldEnd: number, changeRangeNewEnd: number, delta: number): void {
463-
visitNode(node);
486+
function checkNodePositions(node: Node, aggressiveChecks: boolean) {
487+
if (aggressiveChecks) {
488+
var pos = node.pos;
489+
forEachChild(node, child => {
490+
Debug.assert(child.pos >= pos);
491+
pos = child.end;
492+
});
493+
Debug.assert(pos <= node.end);
494+
}
495+
}
496+
497+
function updateTokenPositionsAndMarkElements(
498+
sourceFile: IncrementalNode,
499+
changeStart: number,
500+
changeRangeOldEnd: number,
501+
changeRangeNewEnd: number,
502+
delta: number,
503+
oldText: string,
504+
newText: string,
505+
aggressiveChecks: boolean): void {
506+
507+
visitNode(sourceFile);
508+
return;
464509

465510
function visitNode(child: IncrementalNode) {
466511
if (child.pos > changeRangeOldEnd) {
467512
// Node is entirely past the change range. We need to move both its pos and
468513
// end, forward or backward appropriately.
469-
moveElementEntirelyPastChangeRange(child, delta);
514+
moveElementEntirelyPastChangeRange(child, delta, oldText, newText, aggressiveChecks);
470515
return;
471516
}
472517

@@ -480,6 +525,8 @@ module ts {
480525
// Adjust the pos or end (or both) of the intersecting element accordingly.
481526
adjustIntersectingElement(child, changeStart, changeRangeOldEnd, changeRangeNewEnd, delta);
482527
forEachChild(child, visitNode, visitArray);
528+
529+
checkNodePositions(child, aggressiveChecks);
483530
return;
484531
}
485532

@@ -490,7 +537,7 @@ module ts {
490537
if (array.pos > changeRangeOldEnd) {
491538
// Array is entirely after the change range. We need to move it, and move any of
492539
// its children.
493-
moveElementEntirelyPastChangeRange(array, delta);
540+
moveElementEntirelyPastChangeRange(array, delta, oldText, newText, aggressiveChecks);
494541
}
495542
else {
496543
// Check if the element intersects the change range. If it does, then it is not
@@ -513,7 +560,6 @@ module ts {
513560
}
514561
}
515562

516-
517563
function extendToAffectedRange(sourceFile: SourceFile, changeRange: TextChangeRange): TextChangeRange {
518564
// Consider the following code:
519565
// void foo() { /; }
@@ -534,6 +580,7 @@ module ts {
534580
// start of the tree.
535581
for (var i = 0; start > 0 && i <= maxLookahead; i++) {
536582
var nearestNode = findNearestNodeStartingBeforeOrAtPosition(sourceFile, start);
583+
Debug.assert(nearestNode.pos <= start);
537584
var position = nearestNode.pos;
538585

539586
start = Math.max(0, position - 1);
@@ -640,6 +687,22 @@ module ts {
640687
}
641688
}
642689

690+
function checkChangeRange(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks: boolean) {
691+
var oldText = sourceFile.text;
692+
if (textChangeRange) {
693+
Debug.assert((oldText.length - textChangeRange.span.length + textChangeRange.newLength) === newText.length);
694+
695+
if (aggressiveChecks || Debug.shouldAssert(AssertionLevel.VeryAggressive)) {
696+
var oldTextPrefix = oldText.substr(0, textChangeRange.span.start);
697+
var newTextPrefix = newText.substr(0, textChangeRange.span.start);
698+
Debug.assert(oldTextPrefix === newTextPrefix);
699+
700+
var oldTextSuffix = oldText.substring(textSpanEnd(textChangeRange.span), oldText.length);
701+
var newTextSuffix = newText.substring(textSpanEnd(textChangeRangeNewSpan(textChangeRange)), newText.length);
702+
Debug.assert(oldTextSuffix === newTextSuffix);
703+
}
704+
}
705+
}
643706

644707
// Produces a new SourceFile for the 'newText' provided. The 'textChangeRange' parameter
645708
// indicates what changed between the 'text' that this SourceFile has and the 'newText'.
@@ -650,7 +713,10 @@ module ts {
650713
// from this SourceFile that are being held onto may change as a result (including
651714
// becoming detached from any SourceFile). It is recommended that this SourceFile not
652715
// be used once 'update' is called on it.
653-
export function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange): SourceFile {
716+
export function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile {
717+
aggressiveChecks = aggressiveChecks || Debug.shouldAssert(AssertionLevel.Aggressive);
718+
719+
checkChangeRange(sourceFile, newText, textChangeRange, aggressiveChecks);
654720
if (textChangeRangeIsUnchanged(textChangeRange)) {
655721
// if the text didn't change, then we can just return our current source file as-is.
656722
return sourceFile;
@@ -659,23 +725,31 @@ module ts {
659725
if (sourceFile.statements.length === 0) {
660726
// If we don't have any statements in the current source file, then there's no real
661727
// way to incrementally parse. So just do a full parse instead.
662-
return parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion,/*syntaxCursor*/ undefined, /*setNodeParents*/ true)
728+
return parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, /*syntaxCursor*/ undefined, /*setNodeParents*/ true)
663729
}
664730

731+
var oldText = sourceFile.text;
665732
var syntaxCursor = createSyntaxCursor(sourceFile);
666733

667734
// Make the actual change larger so that we know to reparse anything whose lookahead
668735
// might have intersected the change.
669736
var changeRange = extendToAffectedRange(sourceFile, textChangeRange);
737+
checkChangeRange(sourceFile, newText, changeRange, aggressiveChecks);
738+
739+
// Ensure that extending the affected range only moved the start of the change range
740+
// earlier in the file.
741+
Debug.assert(changeRange.span.start <= textChangeRange.span.start);
742+
Debug.assert(textSpanEnd(changeRange.span) === textSpanEnd(textChangeRange.span));
743+
Debug.assert(textSpanEnd(textChangeRangeNewSpan(changeRange)) === textSpanEnd(textChangeRangeNewSpan(textChangeRange)));
670744

671745
// The is the amount the nodes after the edit range need to be adjusted. It can be
672746
// positive (if the edit added characters), negative (if the edit deleted characters)
673747
// or zero (if this was a pure overwrite with nothing added/removed).
674748
var delta = textChangeRangeNewSpan(changeRange).length - changeRange.span.length;
675749

676750
// If we added or removed characters during the edit, then we need to go and adjust all
677-
// the nodes after the edit. Those nodes may move forward down (if we inserted chars)
678-
// or they may move backward (if we deleted chars).
751+
// the nodes after the edit. Those nodes may move forward (if we inserted chars) or they
752+
// may move backward (if we deleted chars).
679753
//
680754
// Doing this helps us out in two ways. First, it means that any nodes/tokens we want
681755
// to reuse are already at the appropriate position in the new text. That way when we
@@ -693,7 +767,7 @@ module ts {
693767
// Also, mark any syntax elements that intersect the changed span. We know, up front,
694768
// that we cannot reuse these elements.
695769
updateTokenPositionsAndMarkElements(<IncrementalNode><Node>sourceFile,
696-
changeRange.span.start, textSpanEnd(changeRange.span), textSpanEnd(textChangeRangeNewSpan(changeRange)), delta);
770+
changeRange.span.start, textSpanEnd(changeRange.span), textSpanEnd(textChangeRangeNewSpan(changeRange)), delta, oldText, newText, aggressiveChecks);
697771

698772
// Now that we've set up our internal incremental state just proceed and parse the
699773
// source file in the normal fashion. When possible the parser will retrieve and
@@ -863,7 +937,6 @@ module ts {
863937
var identifiers: Map<string> = {};
864938
var identifierCount = 0;
865939
var nodeCount = 0;
866-
var scanner: Scanner;
867940
var token: SyntaxKind;
868941

869942
var sourceFile = <SourceFile>createNode(SyntaxKind.SourceFile, /*pos*/ 0);
@@ -956,7 +1029,7 @@ module ts {
9561029
var parseErrorBeforeNextFinishedNode: boolean = false;
9571030

9581031
// Create and prime the scanner before parsing the source elements.
959-
scanner = createScanner(languageVersion, /*skipTrivia*/ true, sourceText, scanError);
1032+
var scanner = createScanner(languageVersion, /*skipTrivia*/ true, sourceText, scanError);
9601033
token = nextToken();
9611034

9621035
processReferenceComments(sourceFile);
@@ -975,6 +1048,7 @@ module ts {
9751048
fixupParentReferences(sourceFile);
9761049
}
9771050

1051+
syntaxCursor = undefined;
9781052
return sourceFile;
9791053

9801054
function setContextFlag(val: Boolean, flag: ParserContextFlags) {

‎src/harness/harnessLanguageService.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ module Harness.LanguageService {
115115
version: string,
116116
textChangeRange: ts.TextChangeRange
117117
): ts.SourceFile {
118-
var result = ts.updateLanguageServiceSourceFile(document, scriptSnapshot, version, textChangeRange);
118+
var result = ts.updateLanguageServiceSourceFile(document, scriptSnapshot, version, textChangeRange, /*aggressiveChecks:*/ true);
119119
Utils.assertInvariants(result, /*parent:*/ undefined);
120120
return result;
121121
}

‎src/services/services.ts

+2-19
Original file line numberDiff line numberDiff line change
@@ -1624,31 +1624,14 @@ module ts {
16241624

16251625
export var disableIncrementalParsing = false;
16261626

1627-
export function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile {
1628-
if (textChangeRange && Debug.shouldAssert(AssertionLevel.Normal)) {
1629-
var oldText = sourceFile.scriptSnapshot;
1630-
var newText = scriptSnapshot;
1631-
1632-
Debug.assert((oldText.getLength() - textChangeRange.span.length + textChangeRange.newLength) === newText.getLength());
1633-
1634-
if (Debug.shouldAssert(AssertionLevel.VeryAggressive)) {
1635-
var oldTextPrefix = oldText.getText(0, textChangeRange.span.start);
1636-
var newTextPrefix = newText.getText(0, textChangeRange.span.start);
1637-
Debug.assert(oldTextPrefix === newTextPrefix);
1638-
1639-
var oldTextSuffix = oldText.getText(textSpanEnd(textChangeRange.span), oldText.getLength());
1640-
var newTextSuffix = newText.getText(textSpanEnd(textChangeRangeNewSpan(textChangeRange)), newText.getLength());
1641-
Debug.assert(oldTextSuffix === newTextSuffix);
1642-
}
1643-
}
1644-
1627+
export function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile {
16451628
// If we were given a text change range, and our version or open-ness changed, then
16461629
// incrementally parse this file.
16471630
if (textChangeRange) {
16481631
if (version !== sourceFile.version) {
16491632
// Once incremental parsing is ready, then just call into this function.
16501633
if (!disableIncrementalParsing) {
1651-
var newSourceFile = updateSourceFile(sourceFile, scriptSnapshot.getText(0, scriptSnapshot.getLength()), textChangeRange);
1634+
var newSourceFile = updateSourceFile(sourceFile, scriptSnapshot.getText(0, scriptSnapshot.getLength()), textChangeRange, aggressiveChecks);
16521635
setSourceFileFields(newSourceFile, scriptSnapshot, version);
16531636
// after incremental parsing nameTable might not be up-to-date
16541637
// drop it so it can be lazily recreated later

‎tests/baselines/reference/APISample_compile.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ declare module "typescript" {
14041404
function createNode(kind: SyntaxKind): Node;
14051405
function forEachChild<T>(node: Node, cbNode: (node: Node) => T, cbNodeArray?: (nodes: Node[]) => T): T;
14061406
function modifierToFlag(token: SyntaxKind): NodeFlags;
1407-
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange): SourceFile;
1407+
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
14081408
function isEvalOrArgumentsIdentifier(node: Node): boolean;
14091409
function createSourceFile(fileName: string, sourceText: string, languageVersion: ScriptTarget, setParentNodes?: boolean): SourceFile;
14101410
function isLeftHandSideExpression(expr: Expression): boolean;
@@ -1895,7 +1895,7 @@ declare module "typescript" {
18951895
}
18961896
function createLanguageServiceSourceFile(fileName: string, scriptSnapshot: IScriptSnapshot, scriptTarget: ScriptTarget, version: string, setNodeParents: boolean): SourceFile;
18971897
var disableIncrementalParsing: boolean;
1898-
function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile;
1898+
function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
18991899
function createDocumentRegistry(): DocumentRegistry;
19001900
function preProcessFile(sourceText: string, readImportFiles?: boolean): PreProcessedFileInfo;
19011901
function createLanguageService(host: LanguageServiceHost, documentRegistry?: DocumentRegistry): LanguageService;

‎tests/baselines/reference/APISample_compile.types

+6-4
Original file line numberDiff line numberDiff line change
@@ -4484,13 +4484,14 @@ declare module "typescript" {
44844484
>SyntaxKind : SyntaxKind
44854485
>NodeFlags : NodeFlags
44864486

4487-
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange): SourceFile;
4488-
>updateSourceFile : (sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange) => SourceFile
4487+
function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
4488+
>updateSourceFile : (sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean) => SourceFile
44894489
>sourceFile : SourceFile
44904490
>SourceFile : SourceFile
44914491
>newText : string
44924492
>textChangeRange : TextChangeRange
44934493
>TextChangeRange : TextChangeRange
4494+
>aggressiveChecks : boolean
44944495
>SourceFile : SourceFile
44954496

44964497
function isEvalOrArgumentsIdentifier(node: Node): boolean;
@@ -5895,15 +5896,16 @@ declare module "typescript" {
58955896
var disableIncrementalParsing: boolean;
58965897
>disableIncrementalParsing : boolean
58975898

5898-
function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange): SourceFile;
5899-
>updateLanguageServiceSourceFile : (sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange) => SourceFile
5899+
function updateLanguageServiceSourceFile(sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
5900+
>updateLanguageServiceSourceFile : (sourceFile: SourceFile, scriptSnapshot: IScriptSnapshot, version: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean) => SourceFile
59005901
>sourceFile : SourceFile
59015902
>SourceFile : SourceFile
59025903
>scriptSnapshot : IScriptSnapshot
59035904
>IScriptSnapshot : IScriptSnapshot
59045905
>version : string
59055906
>textChangeRange : TextChangeRange
59065907
>TextChangeRange : TextChangeRange
5908+
>aggressiveChecks : boolean
59075909
>SourceFile : SourceFile
59085910

59095911
function createDocumentRegistry(): DocumentRegistry;

0 commit comments

Comments
 (0)