Skip to content

Use NodeFlags to detect nodes in ambient contexts instead of climbing ancestors #17831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
7 commits merged into from
Nov 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,7 @@ namespace ts {
function setExportContextFlag(node: ModuleDeclaration | SourceFile) {
// A declaration source file or ambient module declaration that contains no export declarations (but possibly regular
// declarations with export modifiers) is an export context in which declarations are implicitly exported.
if (isInAmbientContext(node) && !hasExportDeclarations(node)) {
if (node.flags & NodeFlags.Ambient && !hasExportDeclarations(node)) {
node.flags |= NodeFlags.ExportContext;
}
else {
Expand Down Expand Up @@ -1726,7 +1726,7 @@ namespace ts {
node.originalKeywordKind >= SyntaxKind.FirstFutureReservedWord &&
node.originalKeywordKind <= SyntaxKind.LastFutureReservedWord &&
!isIdentifierName(node) &&
!isInAmbientContext(node)) {
!(node.flags & NodeFlags.Ambient)) {

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

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

function bindFunctionDeclaration(node: FunctionDeclaration) {
if (!file.isDeclarationFile && !isInAmbientContext(node)) {
if (!file.isDeclarationFile && !(node.flags & NodeFlags.Ambient)) {
if (isAsyncFunction(node)) {
emitFlags |= NodeFlags.HasAsyncFunctions;
}
Expand All @@ -2520,7 +2520,7 @@ namespace ts {
}

function bindFunctionExpression(node: FunctionExpression) {
if (!file.isDeclarationFile && !isInAmbientContext(node)) {
if (!file.isDeclarationFile && !(node.flags & NodeFlags.Ambient)) {
if (isAsyncFunction(node)) {
emitFlags |= NodeFlags.HasAsyncFunctions;
}
Expand All @@ -2534,7 +2534,7 @@ namespace ts {
}

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

Expand Down Expand Up @@ -2583,7 +2583,7 @@ namespace ts {
// On the other side we do want to report errors on non-initialized 'lets' because of TDZ
const reportUnreachableCode =
!options.allowUnreachableCode &&
!isInAmbientContext(node) &&
!(node.flags & NodeFlags.Ambient) &&
(
node.kind !== SyntaxKind.VariableStatement ||
getCombinedNodeFlags((<VariableStatement>node).declarationList) & NodeFlags.BlockScoped ||
Expand Down
88 changes: 44 additions & 44 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

47 changes: 39 additions & 8 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ namespace ts {
}

export function parseIsolatedEntityName(content: string, languageVersion: ScriptTarget): EntityName {
// Choice of `isDeclarationFile` should be arbitrary
initializeState(content, languageVersion, /*syntaxCursor*/ undefined, ScriptKind.JS);
// Prime the scanner.
nextToken();
Expand All @@ -639,7 +640,7 @@ namespace ts {
export function parseJsonText(fileName: string, sourceText: string): JsonSourceFile {
initializeState(sourceText, ScriptTarget.ES2015, /*syntaxCursor*/ undefined, ScriptKind.JSON);
// Set source file so that errors will be reported with this file name
sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON);
sourceFile = createSourceFile(fileName, ScriptTarget.ES2015, ScriptKind.JSON, /*isDeclaration*/ false);
const result = <JsonSourceFile>sourceFile;

// Prime the scanner.
Expand Down Expand Up @@ -681,7 +682,16 @@ namespace ts {
identifierCount = 0;
nodeCount = 0;

contextFlags = scriptKind === ScriptKind.JS || scriptKind === ScriptKind.JSX || scriptKind === ScriptKind.JSON ? NodeFlags.JavaScriptFile : NodeFlags.None;
switch (scriptKind) {
case ScriptKind.JS:
case ScriptKind.JSX:
case ScriptKind.JSON:
contextFlags = NodeFlags.JavaScriptFile;
break;
default:
contextFlags = NodeFlags.None;
break;
}
parseErrorBeforeNextFinishedNode = false;

// Initialize and prime the scanner before parsing the source elements.
Expand All @@ -705,7 +715,12 @@ namespace ts {
}

function parseSourceFileWorker(fileName: string, languageVersion: ScriptTarget, setParentNodes: boolean, scriptKind: ScriptKind): SourceFile {
sourceFile = createSourceFile(fileName, languageVersion, scriptKind);
const isDeclarationFile = isDeclarationFileName(fileName);
if (isDeclarationFile) {
contextFlags |= NodeFlags.Ambient;
}

sourceFile = createSourceFile(fileName, languageVersion, scriptKind, isDeclarationFile);
sourceFile.flags = contextFlags;

// Prime the scanner.
Expand Down Expand Up @@ -782,7 +797,7 @@ namespace ts {
}
}

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

return sourceFile;
Expand Down Expand Up @@ -5089,6 +5104,18 @@ namespace ts {
const fullStart = getNodePos();
const decorators = parseDecorators();
const modifiers = parseModifiers();
if (some(modifiers, m => m.kind === SyntaxKind.DeclareKeyword)) {
for (const m of modifiers) {
m.flags |= NodeFlags.Ambient;
}
return doInsideOfContext(NodeFlags.Ambient, () => parseDeclarationWorker(fullStart, decorators, modifiers));
}
else {
return parseDeclarationWorker(fullStart, decorators, modifiers);
}
}

function parseDeclarationWorker(fullStart: number, decorators: NodeArray<Decorator> | undefined, modifiers: NodeArray<Modifier> | undefined): Statement {
switch (token()) {
case SyntaxKind.VarKeyword:
case SyntaxKind.LetKeyword:
Expand Down Expand Up @@ -5446,8 +5473,8 @@ namespace ts {
return false;
}

function parseDecorators(): NodeArray<Decorator> {
let list: Decorator[];
function parseDecorators(): NodeArray<Decorator> | undefined {
let list: Decorator[] | undefined;
const listPos = getNodePos();
while (true) {
const decoratorStart = getNodePos();
Expand Down Expand Up @@ -6131,7 +6158,7 @@ namespace ts {
export namespace JSDocParser {
export function parseJSDocTypeExpressionForTests(content: string, start: number, length: number): { jsDocTypeExpression: JSDocTypeExpression, diagnostics: Diagnostic[] } | undefined {
initializeState(content, ScriptTarget.Latest, /*_syntaxCursor:*/ undefined, ScriptKind.JS);
sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS);
sourceFile = createSourceFile("file.js", ScriptTarget.Latest, ScriptKind.JS, /*isDeclarationFile*/ false);
scanner.setText(content, start, length);
currentToken = scanner.scan();
const jsDocTypeExpression = parseJSDocTypeExpression();
Expand Down Expand Up @@ -7461,4 +7488,8 @@ namespace ts {
Value = -1
}
}

function isDeclarationFileName(fileName: string): boolean {
return fileExtensionIs(fileName, Extension.Dts);
}
}
5 changes: 3 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,15 +451,16 @@ namespace ts {
/* @internal */
PossiblyContainsDynamicImport = 1 << 19,
JSDoc = 1 << 20, // If node was parsed inside jsdoc
/* @internal */ InWithStatement = 1 << 21, // If any ancestor of node was the `statement` of a WithStatement (not the `expression`)
/* @internal */ Ambient = 1 << 21, // If node was inside an ambient context -- a declaration file, or inside something with the `declare` modifier.
/* @internal */ InWithStatement = 1 << 22, // If any ancestor of node was the `statement` of a WithStatement (not the `expression`)

BlockScoped = Let | Const,

ReachabilityCheckFlags = HasImplicitReturn | HasExplicitReturn,
ReachabilityAndEmitFlags = ReachabilityCheckFlags | HasAsyncFunctions,

// Parsing context flags
ContextFlags = DisallowInContext | YieldContext | DecoratorContext | AwaitContext | JavaScriptFile | InWithStatement,
ContextFlags = DisallowInContext | YieldContext | DecoratorContext | AwaitContext | JavaScriptFile | InWithStatement | Ambient,

// Exclude these flags when parsing a Type
TypeExcludesFlags = YieldContext | AwaitContext,
Expand Down
10 changes: 0 additions & 10 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1718,16 +1718,6 @@ namespace ts {
return false;
}

export function isInAmbientContext(node: Node): boolean {
while (node) {
if (hasModifier(node, ModifierFlags.Ambient) || (node.kind === SyntaxKind.SourceFile && (node as SourceFile).isDeclarationFile)) {
return true;
}
node = node.parent;
}
return false;
}

// True if the given identifier, string literal, or number literal is the name of a declaration node
export function isDeclarationName(name: Node): boolean {
switch (name.kind) {
Expand Down
6 changes: 3 additions & 3 deletions src/harness/unittests/incrementalParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ module m3 { }\
const index = 0;
const newTextAndChange = withInsert(oldText, index, "declare ");

compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 3);
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 0);
});

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

compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 9);
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
});

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

compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 9);
compareTrees(oldText, newTextAndChange.text, newTextAndChange.textChangeRange, 4);
});

it("Moving methods from class to object literal", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/services/breakpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace ts.BreakpointResolver {
}

// Cannot set breakpoint in ambient declarations
if (isInAmbientContext(tokenAtLocation)) {
if (tokenAtLocation.flags & NodeFlags.Ambient) {
return undefined;
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ namespace ts.refactor.extractSymbol {
return [createDiagnosticForNode(nodeToCheck, Messages.StatementOrExpressionExpected)];
}

if (isInAmbientContext(nodeToCheck)) {
if (nodeToCheck.flags & NodeFlags.Ambient) {
return [createDiagnosticForNode(nodeToCheck, Messages.CannotExtractAmbientBlock)];
}

Expand Down
1 change: 1 addition & 0 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ namespace ts {
kind === SyntaxKind.Identifier ? new IdentifierObject(SyntaxKind.Identifier, pos, end) :
new TokenObject(kind, pos, end);
node.parent = parent;
node.flags = parent.flags & NodeFlags.ContextFlags;
return node;
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ namespace ts {
if (flags & ModifierFlags.Static) result.push(ScriptElementKindModifier.staticModifier);
if (flags & ModifierFlags.Abstract) result.push(ScriptElementKindModifier.abstractModifier);
if (flags & ModifierFlags.Export) result.push(ScriptElementKindModifier.exportedModifier);
if (isInAmbientContext(node)) result.push(ScriptElementKindModifier.ambientModifier);
if (node.flags & NodeFlags.Ambient) result.push(ScriptElementKindModifier.ambientModifier);

return result.length > 0 ? result.join(",") : ScriptElementKindModifier.none;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ declare namespace ts {
BlockScoped = 3,
ReachabilityCheckFlags = 384,
ReachabilityAndEmitFlags = 1408,
ContextFlags = 2193408,
ContextFlags = 6387712,
TypeExcludesFlags = 20480,
}
enum ModifierFlags {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ declare namespace ts {
BlockScoped = 3,
ReachabilityCheckFlags = 384,
ReachabilityAndEmitFlags = 1408,
ContextFlags = 2193408,
ContextFlags = 6387712,
TypeExcludesFlags = 20480,
}
enum ModifierFlags {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts(2,3): error TS1031: 'declare' modifier cannot appear on a class element.
tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts(2,25): error TS1183: An implementation cannot be declared in ambient contexts.


==== tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts (2 errors) ====
==== tests/cases/conformance/parser/ecmascript5/ConstructorDeclarations/parserConstructorDeclaration4.ts (1 errors) ====
class C {
declare constructor() { }
~~~~~~~
!!! error TS1031: 'declare' modifier cannot appear on a class element.
~
!!! error TS1183: An implementation cannot be declared in ambient contexts.
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts(2,5): error TS1031: 'declare' modifier cannot appear on a class element.
tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts(2,17): error TS2378: A 'get' accessor must return a value.


==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts (1 errors) ====
==== tests/cases/conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts (2 errors) ====
class C {
declare get Foo() { }
~~~~~~~
!!! error TS1031: 'declare' modifier cannot appear on a class element.
~~~
!!! error TS2378: A 'get' accessor must return a value.
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts(2,5): error TS1031: 'declare' modifier cannot appear on a class element.
tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts(2,19): error TS1183: An implementation cannot be declared in ambient contexts.


==== tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts (2 errors) ====
==== tests/cases/conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration5.ts (1 errors) ====
class C {
declare Foo() { }
~~~~~~~
!!! error TS1031: 'declare' modifier cannot appear on a class element.
~
!!! error TS1183: An implementation cannot be declared in ambient contexts.
}