Skip to content

Commit cc2a2a7

Browse files
author
Andy
authored
Use NodeFlags to detect nodes in ambient contexts instead of climbing ancestors (#17831)
* Use NodeFlags to detect nodes in ambient contexts instead of climbing ancestors * Set context flags on tokens * Remove 'isDeclarationFile' parameter to 'initializeState' and move to 'parseSourceFileWorker' * Changes based on code review * Update API baselines
1 parent 1321d2a commit cc2a2a7

15 files changed

+108
-88
lines changed

‎src/compiler/binder.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -1550,7 +1550,7 @@ namespace ts {
15501550
function setExportContextFlag(node: ModuleDeclaration | SourceFile) {
15511551
// A declaration source file or ambient module declaration that contains no export declarations (but possibly regular
15521552
// declarations with export modifiers) is an export context in which declarations are implicitly exported.
1553-
if (isInAmbientContext(node) && !hasExportDeclarations(node)) {
1553+
if (node.flags & NodeFlags.Ambient && !hasExportDeclarations(node)) {
15541554
node.flags |= NodeFlags.ExportContext;
15551555
}
15561556
else {
@@ -1726,7 +1726,7 @@ namespace ts {
17261726
node.originalKeywordKind >= SyntaxKind.FirstFutureReservedWord &&
17271727
node.originalKeywordKind <= SyntaxKind.LastFutureReservedWord &&
17281728
!isIdentifierName(node) &&
1729-
!isInAmbientContext(node)) {
1729+
!(node.flags & NodeFlags.Ambient)) {
17301730

17311731
// Report error only if there are no parse errors in file
17321732
if (!file.parseDiagnostics.length) {
@@ -2481,7 +2481,7 @@ namespace ts {
24812481
}
24822482

24832483
function bindParameter(node: ParameterDeclaration) {
2484-
if (inStrictMode && !isInAmbientContext(node)) {
2484+
if (inStrictMode && !(node.flags & NodeFlags.Ambient)) {
24852485
// It is a SyntaxError if the identifier eval or arguments appears within a FormalParameterList of a
24862486
// strict mode FunctionLikeDeclaration or FunctionExpression(13.1)
24872487
checkStrictModeEvalOrArguments(node, node.name);
@@ -2503,7 +2503,7 @@ namespace ts {
25032503
}
25042504

25052505
function bindFunctionDeclaration(node: FunctionDeclaration) {
2506-
if (!file.isDeclarationFile && !isInAmbientContext(node)) {
2506+
if (!file.isDeclarationFile && !(node.flags & NodeFlags.Ambient)) {
25072507
if (isAsyncFunction(node)) {
25082508
emitFlags |= NodeFlags.HasAsyncFunctions;
25092509
}
@@ -2520,7 +2520,7 @@ namespace ts {
25202520
}
25212521

25222522
function bindFunctionExpression(node: FunctionExpression) {
2523-
if (!file.isDeclarationFile && !isInAmbientContext(node)) {
2523+
if (!file.isDeclarationFile && !(node.flags & NodeFlags.Ambient)) {
25242524
if (isAsyncFunction(node)) {
25252525
emitFlags |= NodeFlags.HasAsyncFunctions;
25262526
}
@@ -2534,7 +2534,7 @@ namespace ts {
25342534
}
25352535

25362536
function bindPropertyOrMethodOrAccessor(node: Declaration, symbolFlags: SymbolFlags, symbolExcludes: SymbolFlags) {
2537-
if (!file.isDeclarationFile && !isInAmbientContext(node) && isAsyncFunction(node)) {
2537+
if (!file.isDeclarationFile && !(node.flags & NodeFlags.Ambient) && isAsyncFunction(node)) {
25382538
emitFlags |= NodeFlags.HasAsyncFunctions;
25392539
}
25402540

@@ -2583,7 +2583,7 @@ namespace ts {
25832583
// On the other side we do want to report errors on non-initialized 'lets' because of TDZ
25842584
const reportUnreachableCode =
25852585
!options.allowUnreachableCode &&
2586-
!isInAmbientContext(node) &&
2586+
!(node.flags & NodeFlags.Ambient) &&
25872587
(
25882588
node.kind !== SyntaxKind.VariableStatement ||
25892589
getCombinedNodeFlags((<VariableStatement>node).declarationList) & NodeFlags.BlockScoped ||

‎src/compiler/checker.ts

+44-44
Large diffs are not rendered by default.

‎src/compiler/parser.ts

+39-8
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ namespace ts {
631631
}
632632

633633
export function parseIsolatedEntityName(content: string, languageVersion: ScriptTarget): EntityName {
634+
// Choice of `isDeclarationFile` should be arbitrary
634635
initializeState(content, languageVersion, /*syntaxCursor*/ undefined, ScriptKind.JS);
635636
// Prime the scanner.
636637
nextToken();
@@ -643,7 +644,7 @@ namespace ts {
643644
export function parseJsonText(fileName: string, sourceText: string): JsonSourceFile {
644645
initializeState(sourceText, ScriptTarget.ES2015, /*syntaxCursor*/ undefined, ScriptKind.JSON);
645646
// Set source file so that errors will be reported with this file name
646-
sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON);
647+
sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON, /*isDeclaration*/ false);
647648
const result = <JsonSourceFile>sourceFile;
648649

649650
// Prime the scanner.
@@ -685,7 +686,16 @@ namespace ts {
685686
identifierCount = 0;
686687
nodeCount = 0;
687688

688-
contextFlags = scriptKind === ScriptKind.JS || scriptKind === ScriptKind.JSX || scriptKind === ScriptKind.JSON ? NodeFlags.JavaScriptFile : NodeFlags.None;
689+
switch (scriptKind) {
690+
case ScriptKind.JS:
691+
case ScriptKind.JSX:
692+
case ScriptKind.JSON:
693+
contextFlags = NodeFlags.JavaScriptFile;
694+
break;
695+
default:
696+
contextFlags = NodeFlags.None;
697+
break;
698+
}
689699
parseErrorBeforeNextFinishedNode = false;
690700

691701
// Initialize and prime the scanner before parsing the source elements.
@@ -709,7 +719,12 @@ namespace ts {
709719
}
710720

711721
function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean, scriptKind: ScriptKind): SourceFile {
712-
sourceFile = createSourceFile(fileName, languageVersion, scriptKind);
722+
const isDeclarationFile = isDeclarationFileName(fileName);
723+
if (isDeclarationFile) {
724+
contextFlags |= NodeFlags.Ambient;
725+
}
726+
727+
sourceFile = createSourceFile(fileName, languageVersion, scriptKind, isDeclarationFile);
713728
sourceFile.flags = contextFlags;
714729

715730
// Prime the scanner.
@@ -786,7 +801,7 @@ namespace ts {
786801
}
787802
}
788803

789-
function createSourceFile(fileName: string, languageVersion: ScriptTarget, scriptKind: ScriptKind): SourceFile {
804+
function createSourceFile(fileName: string, languageVersion: ScriptTarget, scriptKind: ScriptKind, isDeclarationFile: boolean): SourceFile {
790805
// code from createNode is inlined here so createNode won't have to deal with special case of creating source files
791806
// this is quite rare comparing to other nodes and createNode should be as fast as possible
792807
const sourceFile = <SourceFile>new SourceFileConstructor(SyntaxKind.SourceFile, /*pos*/ 0, /* end */ sourceText.length);
@@ -797,7 +812,7 @@ namespace ts {
797812
sourceFile.languageVersion = languageVersion;
798813
sourceFile.fileName = normalizePath(fileName);
799814
sourceFile.languageVariant = getLanguageVariant(scriptKind);
800-
sourceFile.isDeclarationFile = fileExtensionIs(sourceFile.fileName, Extension.Dts);
815+
sourceFile.isDeclarationFile = isDeclarationFile;
801816
sourceFile.scriptKind = scriptKind;
802817

803818
return sourceFile;
@@ -5135,6 +5150,18 @@ namespace ts {
51355150
const fullStart = getNodePos();
51365151
const decorators = parseDecorators();
51375152
const modifiers = parseModifiers();
5153+
if (some(modifiers, m => m.kind === SyntaxKind.DeclareKeyword)) {
5154+
for (const m of modifiers) {
5155+
m.flags |= NodeFlags.Ambient;
5156+
}
5157+
return doInsideOfContext(NodeFlags.Ambient, () => parseDeclarationWorker(fullStart, decorators, modifiers));
5158+
}
5159+
else {
5160+
return parseDeclarationWorker(fullStart, decorators, modifiers);
5161+
}
5162+
}
5163+
5164+
function parseDeclarationWorker(fullStart: number, decorators: NodeArray<Decorator> | undefined, modifiers: NodeArray<Modifier> | undefined): Statement {
51385165
switch (token()) {
51395166
case SyntaxKind.VarKeyword:
51405167
case SyntaxKind.LetKeyword:
@@ -5492,8 +5519,8 @@ namespace ts {
54925519
return false;
54935520
}
54945521

5495-
function parseDecorators(): NodeArray<Decorator> {
5496-
let list: Decorator[];
5522+
function parseDecorators(): NodeArray<Decorator> | undefined {
5523+
let list: Decorator[] | undefined;
54975524
const listPos = getNodePos();
54985525
while (true) {
54995526
const decoratorStart = getNodePos();
@@ -6177,7 +6204,7 @@ namespace ts {
61776204
export namespace JSDocParser {
61786205
export function parseJSDocTypeExpressionForTests(content: string, start: number, length: number): { jsDocTypeExpression: JSDocTypeExpression, diagnostics: Diagnostic[] } | undefined {
61796206
initializeState(content, ScriptTarget.Latest, /*_syntaxCursor:*/ undefined, ScriptKind.JS);
6180-
sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS);
6207+
sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS, /*isDeclarationFile*/ false);
61816208
scanner.setText(content, start, length);
61826209
currentToken = scanner.scan();
61836210
const jsDocTypeExpression = parseJSDocTypeExpression();
@@ -7507,4 +7534,8 @@ namespace ts {
75077534
Value = -1
75087535
}
75097536
}
7537+
7538+
function isDeclarationFileName(fileName: string): boolean {
7539+
return fileExtensionIs(fileName, Extension.Dts);
7540+
}
75107541
}

‎src/compiler/types.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -454,15 +454,16 @@ namespace ts {
454454
/* @internal */
455455
PossiblyContainsDynamicImport = 1 << 19,
456456
JSDoc = 1 << 20, // If node was parsed inside jsdoc
457-
/* @internal */ InWithStatement = 1 << 21, // If any ancestor of node was the `statement` of a WithStatement (not the `expression`)
457+
/* @internal */ Ambient = 1 << 21, // If node was inside an ambient context -- a declaration file, or inside something with the `declare` modifier.
458+
/* @internal */ InWithStatement = 1 << 22, // If any ancestor of node was the `statement` of a WithStatement (not the `expression`)
458459

459460
BlockScoped = Let | Const,
460461

461462
ReachabilityCheckFlags = HasImplicitReturn | HasExplicitReturn,
462463
ReachabilityAndEmitFlags = ReachabilityCheckFlags | HasAsyncFunctions,
463464

464465
// Parsing context flags
465-
ContextFlags = DisallowInContext | YieldContext | DecoratorContext | AwaitContext | JavaScriptFile | InWithStatement,
466+
ContextFlags = DisallowInContext | YieldContext | DecoratorContext | AwaitContext | JavaScriptFile | InWithStatement | Ambient,
466467

467468
// Exclude these flags when parsing a Type
468469
TypeExcludesFlags = YieldContext | AwaitContext,

‎src/compiler/utilities.ts

-10
Original file line numberDiff line numberDiff line change
@@ -1728,16 +1728,6 @@ namespace ts {
17281728
return false;
17291729
}
17301730

1731-
export function isInAmbientContext(node: Node): boolean {
1732-
while (node) {
1733-
if (hasModifier(node, ModifierFlags.Ambient) || (node.kind === SyntaxKind.SourceFile && (node as SourceFile).isDeclarationFile)) {
1734-
return true;
1735-
}
1736-
node = node.parent;
1737-
}
1738-
return false;
1739-
}
1740-
17411731
// True if the given identifier, string literal, or number literal is the name of a declaration node
17421732
export function isDeclarationName(name: Node): boolean {
17431733
switch (name.kind) {

‎src/harness/unittests/incrementalParser.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ module m3 { }\
591591
const index = 0;
592592
const newTextAndChange = withInsert(oldText, index, "declare ");
593593

594-
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 3);
594+
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 0);
595595
});
596596

597597
it("Insert function above arrow function with comment", () => {
@@ -674,7 +674,7 @@ module m3 { }\
674674
const oldText = ScriptSnapshot.fromString(source);
675675
const newTextAndChange = withInsert(oldText, 0, "{");
676676

677-
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 9);
677+
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
678678
});
679679

680680
it("Removing block around function declarations", () => {
@@ -683,7 +683,7 @@ module m3 { }\
683683
const oldText = ScriptSnapshot.fromString(source);
684684
const newTextAndChange = withDelete(oldText, 0, "{".length);
685685

686-
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 9);
686+
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
687687
});
688688

689689
it("Moving methods from class to object literal", () => {

‎src/services/breakpoints.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ namespace ts.BreakpointResolver {
3131
}
3232

3333
// Cannot set breakpoint in ambient declarations
34-
if (isInAmbientContext(tokenAtLocation)) {
34+
if (tokenAtLocation.flags & NodeFlags.Ambient) {
3535
return undefined;
3636
}
3737

‎src/services/refactors/extractSymbol.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ namespace ts.refactor.extractSymbol {
335335
return [createDiagnosticForNode(nodeToCheck, Messages.StatementOrExpressionExpected)];
336336
}
337337

338-
if (isInAmbientContext(nodeToCheck)) {
338+
if (nodeToCheck.flags & NodeFlags.Ambient) {
339339
return [createDiagnosticForNode(nodeToCheck, Messages.CannotExtractAmbientBlock)];
340340
}
341341

‎src/services/services.ts

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ namespace ts {
4141
kind === SyntaxKind.Identifier ? new IdentifierObject(SyntaxKind.Identifier, pos, end) :
4242
new TokenObject(kind, pos, end);
4343
node.parent = parent;
44+
node.flags = parent.flags & NodeFlags.ContextFlags;
4445
return node;
4546
}
4647

‎src/services/utilities.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ namespace ts {
947947
if (flags & ModifierFlags.Static) result.push(ScriptElementKindModifier.staticModifier);
948948
if (flags & ModifierFlags.Abstract) result.push(ScriptElementKindModifier.abstractModifier);
949949
if (flags & ModifierFlags.Export) result.push(ScriptElementKindModifier.exportedModifier);
950-
if (isInAmbientContext(node)) result.push(ScriptElementKindModifier.ambientModifier);
950+
if (node.flags & NodeFlags.Ambient) result.push(ScriptElementKindModifier.ambientModifier);
951951

952952
return result.length > 0 ? result.join(",") : ScriptElementKindModifier.none;
953953
}

‎tests/baselines/reference/api/tsserverlibrary.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ declare namespace ts {
411411
BlockScoped = 3,
412412
ReachabilityCheckFlags = 384,
413413
ReachabilityAndEmitFlags = 1408,
414-
ContextFlags = 2193408,
414+
ContextFlags = 6387712,
415415
TypeExcludesFlags = 20480,
416416
}
417417
enum ModifierFlags {

‎tests/baselines/reference/api/typescript.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ declare namespace ts {
411411
BlockScoped = 3,
412412
ReachabilityCheckFlags = 384,
413413
ReachabilityAndEmitFlags = 1408,
414-
ContextFlags = 2193408,
414+
ContextFlags = 6387712,
415415
TypeExcludesFlags = 20480,
416416
}
417417
enum ModifierFlags {

‎tests/baselines/reference/parserConstructorDeclaration4.errors.txt

+1-4
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts(2,3): error TS1031: 'declare' modifier cannot appear on a class element.
2-
tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts(2,25): error TS1183: An implementation cannot be declared in ambient contexts.
32

43

5-
==== tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts (2 errors) ====
4+
==== tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts (1 errors) ====
65
class C {
76
declare constructor() { }
87
~~~~~~~
98
!!! error TS1031: 'declare' modifier cannot appear on a class element.
10-
~
11-
!!! error TS1183: An implementation cannot be declared in ambient contexts.
129
}

‎tests/baselines/reference/parserMemberAccessorDeclaration11.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts(2,5): error TS1031: 'declare' modifier cannot appear on a class element.
2+
tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts(2,17): error TS2378: A 'get' accessor must return a value.
23

34

4-
==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts (1 errors) ====
5+
==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts (2 errors) ====
56
class C {
67
declare get Foo() { }
78
~~~~~~~
89
!!! error TS1031: 'declare' modifier cannot appear on a class element.
10+
~~~
11+
!!! error TS2378: A 'get' accessor must return a value.
912
}

‎tests/baselines/reference/parserMemberFunctionDeclaration5.errors.txt

+1-4
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts(2,5): error TS1031: 'declare' modifier cannot appear on a class element.
2-
tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts(2,19): error TS1183: An implementation cannot be declared in ambient contexts.
32

43

5-
==== tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts (2 errors) ====
4+
==== tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts (1 errors) ====
65
class C {
76
declare Foo() { }
87
~~~~~~~
98
!!! error TS1031: 'declare' modifier cannot appear on a class element.
10-
~
11-
!!! error TS1183: An implementation cannot be declared in ambient contexts.
129
}

0 commit comments

Comments
 (0)