From c24f75fd73e37b843870d9478c818c732a065291 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Dec 2014 11:48:46 -0800 Subject: [PATCH 1/5] defer decision whether import used on the right side of import declaration should be considered referenced --- src/compiler/checker.ts | 70 +++++++++++++++---- src/compiler/types.ts | 1 + .../importedAliasesInTypePositions.js | 55 +++++++++++++++ .../importedAliasesInTypePositions.types | 40 +++++++++++ .../importedAliasesInTypePositions.ts | 19 +++++ 5 files changed, 170 insertions(+), 15 deletions(-) create mode 100644 tests/baselines/reference/importedAliasesInTypePositions.js create mode 100644 tests/baselines/reference/importedAliasesInTypePositions.types create mode 100644 tests/cases/compiler/importedAliasesInTypePositions.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9cff537b07f..24f5a2f528d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4734,14 +4734,47 @@ module ts { return type; } } + /*Transitively mark all linked imports as referenced*/ + function markLinkedImportsAsReferenced(node: ImportDeclaration): void { + var nodeLinks = getNodeLinks(node); + while (nodeLinks.importOnRightSide) { + var rightSide = nodeLinks.importOnRightSide; + nodeLinks.importOnRightSide = undefined; + + getSymbolLinks(rightSide).referenced = true; + nodeLinks = getNodeLinks(rightSide.declarations[0]) + } + } function checkIdentifier(node: Identifier): Type { var symbol = getResolvedSymbol(node); if (symbol.flags & SymbolFlags.Import) { - // Mark the import as referenced so that we emit it in the final .js file. - // exception: identifiers that appear in type queries, const enums, modules that contain only const enums - getSymbolLinks(symbol).referenced = getSymbolLinks(symbol).referenced || (!isInTypeQuery(node) && !isConstEnumOrConstEnumOnlyModule(resolveImport(symbol))); + var symbolLinks = getSymbolLinks(symbol); + if (!symbolLinks.referenced) { + var importOrExportAssignment = getLeftSideOfImportOrExportAssignment(node); + + // decision about whether import is referenced can be made now if + // - import that are used anywhere except right side of import declarations + // - imports that are used on the right side of exported import declarations + // for other cases defer decision until the check of left side + if (!importOrExportAssignment || + (importOrExportAssignment.flags & NodeFlags.Export) || + (importOrExportAssignment.kind === SyntaxKind.ExportAssignment)) { + // Mark the import as referenced so that we emit it in the final .js file. + // exception: identifiers that appear in type queries, const enums, modules that contain only const enums + symbolLinks.referenced = !isInTypeQuery(node) && !isConstEnumOrConstEnumOnlyModule(resolveImport(symbol)); + } + else { + var nodeLinks = getNodeLinks(importOrExportAssignment); + Debug.assert(!nodeLinks.importOnRightSide); + nodeLinks.importOnRightSide = symbol; + } + } + + if (symbolLinks.referenced) { + markLinkedImportsAsReferenced(symbol.declarations[0]); + } } checkCollisionWithCapturedSuperVariable(node, node); @@ -8872,6 +8905,8 @@ module ts { if (symbol && symbol.flags & SymbolFlags.Import) { // Mark the import as referenced so that we emit it in the final .js file. getSymbolLinks(symbol).referenced = true; + // mark any import declarations that depend upon this import as referenced + markLinkedImportsAsReferenced(symbol.declarations[0]) } } @@ -9097,19 +9132,24 @@ module ts { return false; } + function getLeftSideOfImportOrExportAssignment(nodeOnRightSide: EntityName): ImportDeclaration | ExportAssignment { + while (nodeOnRightSide.parent.kind === SyntaxKind.QualifiedName) { + nodeOnRightSide = nodeOnRightSide.parent; + } + + if (nodeOnRightSide.parent.kind === SyntaxKind.ImportDeclaration) { + return (nodeOnRightSide.parent).moduleReference === nodeOnRightSide && nodeOnRightSide.parent; + } + + if (nodeOnRightSide.parent.kind === SyntaxKind.ExportAssignment) { + return (nodeOnRightSide.parent).exportName === nodeOnRightSide && nodeOnRightSide.parent; + } + + return undefined; + } + function isInRightSideOfImportOrExportAssignment(node: EntityName) { - while (node.parent.kind === SyntaxKind.QualifiedName) { - node = node.parent; - } - - if (node.parent.kind === SyntaxKind.ImportDeclaration) { - return (node.parent).moduleReference === node; - } - if (node.parent.kind === SyntaxKind.ExportAssignment) { - return (node.parent).exportName === node; - } - - return false; + return getLeftSideOfImportOrExportAssignment(node) !== undefined; } function isRightSideOfQualifiedNameOrPropertyAccess(node: Node) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index a77961da586..9ba8f3aa21c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1216,6 +1216,7 @@ module ts { isVisible?: boolean; // Is this node visible localModuleName?: string; // Local name for module instance assignmentChecks?: Map; // Cache of assignment checks + importOnRightSide?: Symbol; // for import declarations - import that appear on the right side } export const enum TypeFlags { diff --git a/tests/baselines/reference/importedAliasesInTypePositions.js b/tests/baselines/reference/importedAliasesInTypePositions.js new file mode 100644 index 00000000000..fb56b1ac0d7 --- /dev/null +++ b/tests/baselines/reference/importedAliasesInTypePositions.js @@ -0,0 +1,55 @@ +//// [tests/cases/compiler/importedAliasesInTypePositions.ts] //// + +//// [file1.ts] +export module elaborate.nested.mod.name { + export class ReferredTo { + doSomething(): void { + } + } +} + +//// [file2.ts] +import RT_ALIAS = require("file1"); +import ReferredTo = RT_ALIAS.elaborate.nested.mod.name.ReferredTo; + +export module ImportingModule { + class UsesReferredType { + constructor(private referred: ReferredTo) { } + } +} + +//// [file1.js] +define(["require", "exports"], function (require, exports) { + var elaborate; + (function (elaborate) { + var nested; + (function (nested) { + var mod; + (function (mod) { + var name; + (function (name) { + var ReferredTo = (function () { + function ReferredTo() { + } + ReferredTo.prototype.doSomething = function () { + }; + return ReferredTo; + })(); + name.ReferredTo = ReferredTo; + })(name = mod.name || (mod.name = {})); + })(mod = nested.mod || (nested.mod = {})); + })(nested = elaborate.nested || (elaborate.nested = {})); + })(elaborate = exports.elaborate || (exports.elaborate = {})); +}); +//// [file2.js] +define(["require", "exports"], function (require, exports) { + var ImportingModule; + (function (ImportingModule) { + var UsesReferredType = (function () { + function UsesReferredType(referred) { + this.referred = referred; + } + return UsesReferredType; + })(); + })(ImportingModule = exports.ImportingModule || (exports.ImportingModule = {})); +}); diff --git a/tests/baselines/reference/importedAliasesInTypePositions.types b/tests/baselines/reference/importedAliasesInTypePositions.types new file mode 100644 index 00000000000..ed78188d275 --- /dev/null +++ b/tests/baselines/reference/importedAliasesInTypePositions.types @@ -0,0 +1,40 @@ +=== tests/cases/compiler/file2.ts === +import RT_ALIAS = require("file1"); +>RT_ALIAS : typeof RT_ALIAS + +import ReferredTo = RT_ALIAS.elaborate.nested.mod.name.ReferredTo; +>ReferredTo : typeof ReferredTo +>RT_ALIAS : typeof RT_ALIAS +>elaborate : typeof RT_ALIAS.elaborate +>nested : typeof RT_ALIAS.elaborate.nested +>mod : typeof RT_ALIAS.elaborate.nested.mod +>name : typeof RT_ALIAS.elaborate.nested.mod.name +>ReferredTo : ReferredTo + +export module ImportingModule { +>ImportingModule : typeof ImportingModule + + class UsesReferredType { +>UsesReferredType : UsesReferredType + + constructor(private referred: ReferredTo) { } +>referred : ReferredTo +>ReferredTo : ReferredTo + } +} +=== tests/cases/compiler/file1.ts === +export module elaborate.nested.mod.name { +>elaborate : typeof elaborate +>nested : typeof nested +>mod : typeof mod +>name : typeof name + + export class ReferredTo { +>ReferredTo : ReferredTo + + doSomething(): void { +>doSomething : () => void + } + } +} + diff --git a/tests/cases/compiler/importedAliasesInTypePositions.ts b/tests/cases/compiler/importedAliasesInTypePositions.ts new file mode 100644 index 00000000000..f1dd66f60a7 --- /dev/null +++ b/tests/cases/compiler/importedAliasesInTypePositions.ts @@ -0,0 +1,19 @@ +// @module:amd +// @Filename: file1.ts +export module elaborate.nested.mod.name { + export class ReferredTo { + doSomething(): void { + } + } +} + +// @Filename: file2.ts +// @module: amd +import RT_ALIAS = require("file1"); +import ReferredTo = RT_ALIAS.elaborate.nested.mod.name.ReferredTo; + +export module ImportingModule { + class UsesReferredType { + constructor(private referred: ReferredTo) { } + } +} \ No newline at end of file From d3bfed13f8cc1ca1de972ed200bd5bb3012111ba Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 14 Dec 2014 10:03:54 -0800 Subject: [PATCH 2/5] Simplify the binder so it does not need to double recurse down constructor parameter nodes. --- src/compiler/binder.ts | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index dfb5482b09c..19ee45a419a 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -292,15 +292,6 @@ module ts { bindChildren(node, symbolKind, isBlockScopeContainer); } - function bindConstructorDeclaration(node: ConstructorDeclaration) { - bindDeclaration(node, SymbolFlags.Constructor, 0, /*isBlockScopeContainer*/ true); - forEach(node.parameters, p => { - if (p.flags & (NodeFlags.Public | NodeFlags.Private | NodeFlags.Protected)) { - bindDeclaration(p, SymbolFlags.Property, SymbolFlags.PropertyExcludes, /*isBlockScopeContainer*/ false); - } - }); - } - function bindModuleDeclaration(node: ModuleDeclaration) { if (node.name.kind === SyntaxKind.StringLiteral) { bindDeclaration(node, SymbolFlags.ValueModule, SymbolFlags.ValueModuleExcludes, /*isBlockScopeContainer*/ true); @@ -389,12 +380,7 @@ module ts { bindDeclaration(node, SymbolFlags.TypeParameter, SymbolFlags.TypeParameterExcludes, /*isBlockScopeContainer*/ false); break; case SyntaxKind.Parameter: - if (isBindingPattern((node).name)) { - bindAnonymousDeclaration(node, SymbolFlags.FunctionScopedVariable, getDestructuringParameterName(node), /*isBlockScopeContainer*/ false); - } - else { - bindDeclaration(node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes, /*isBlockScopeContainer*/ false); - } + bindParameter(node); break; case SyntaxKind.VariableDeclaration: case SyntaxKind.BindingElement: @@ -437,7 +423,7 @@ module ts { bindDeclaration(node, SymbolFlags.Function, SymbolFlags.FunctionExcludes, /*isBlockScopeContainer*/ true); break; case SyntaxKind.Constructor: - bindConstructorDeclaration(node); + bindDeclaration(node, SymbolFlags.Constructor, /*symbolExcludes:*/ 0, /*isBlockScopeContainer:*/ true); break; case SyntaxKind.GetAccessor: bindDeclaration(node, SymbolFlags.GetAccessor, SymbolFlags.GetAccessorExcludes, /*isBlockScopeContainer*/ true); @@ -508,5 +494,24 @@ module ts { parent = saveParent; } } + + function bindParameter(node: ParameterDeclaration) { + if (isBindingPattern(node.name)) { + bindAnonymousDeclaration(node, SymbolFlags.FunctionScopedVariable, getDestructuringParameterName(node), /*isBlockScopeContainer*/ false); + } + else { + bindDeclaration(node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes, /*isBlockScopeContainer*/ false); + } + + // If this is a property-parameter, then also declare the property symbol into the + // containing class. + if (node.flags & NodeFlags.AccessibilityModifier && + node.parent.kind === SyntaxKind.Constructor && + node.parent.parent.kind === SyntaxKind.ClassDeclaration) { + + var classDeclaration = node.parent.parent; + declareSymbol(classDeclaration.symbol.members, classDeclaration.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes); + } + } } } From cb8d2f28ae7127878aea38609cad8d98782eb150 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 15 Dec 2014 14:43:31 -0800 Subject: [PATCH 3/5] Simplify how we set container.nextContainer now that we don't double recurse. --- src/compiler/binder.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 19ee45a419a..4f8141fd866 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -227,23 +227,27 @@ module ts { if (symbolKind & SymbolFlags.HasLocals) { node.locals = {}; } + var saveParent = parent; var saveContainer = container; var savedBlockScopeContainer = blockScopeContainer; parent = node; if (symbolKind & SymbolFlags.IsContainer) { container = node; - // If container is not on container list, add it to the list - if (lastContainer !== container && !container.nextContainer) { - if (lastContainer) { - lastContainer.nextContainer = container; - } - lastContainer = container; + Debug.assert(container.nextContainer === undefined); + + if (lastContainer) { + Debug.assert(lastContainer.nextContainer === undefined); + lastContainer.nextContainer = container; } + + lastContainer = container; } + if (isBlockScopeContainer) { blockScopeContainer = node; } + forEachChild(node, bind); container = saveContainer; parent = saveParent; From 30f9a5ca2adbee04df74e957d57bb69db5325139 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 15 Dec 2014 21:41:02 -0800 Subject: [PATCH 4/5] Do not pass context flags downward while parsing binding elements. This prevents an unnecessary allocation, simplifies parsing code, and prevents an issue where parsing depends on context flags not stored in the final tree. This is an issue for incremental parsing that can lead to nodes being reused inappropriately. --- src/compiler/parser.ts | 43 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index ea5b35cbce1..582924df04b 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -3277,38 +3277,39 @@ module ts { // DECLARATIONS - function parseBindingElement(context: ParsingContext): BindingElement { - if (context === ParsingContext.ArrayBindingElements && token === SyntaxKind.CommaToken) { + function parseArrayBindingElement(): BindingElement { + if (token === SyntaxKind.CommaToken) { return createNode(SyntaxKind.OmittedExpression); } + var node = createNode(SyntaxKind.BindingElement); - if (context === ParsingContext.ObjectBindingElements) { - // TODO(andersh): Handle computed properties - var id = parsePropertyName(); - if (id.kind === SyntaxKind.Identifier && token !== SyntaxKind.ColonToken) { - node.name = id; - } - else { - parseExpected(SyntaxKind.ColonToken); - node.propertyName = id; - node.name = parseIdentifierOrPattern(); - } - } - else { - node.name = parseIdentifierOrPattern(); - } + node.name = parseIdentifierOrPattern(); node.initializer = parseInitializer(/*inParameter*/ false); return finishNode(node); } - function parseBindingList(context: ParsingContext): NodeArray { - return parseDelimitedList(context, () => parseBindingElement(context)); + function parseObjectBindingElement(): BindingElement { + var node = createNode(SyntaxKind.BindingElement); + + // TODO(andersh): Handle computed properties + var id = parsePropertyName(); + if (id.kind === SyntaxKind.Identifier && token !== SyntaxKind.ColonToken) { + node.name = id; + } + else { + parseExpected(SyntaxKind.ColonToken); + node.propertyName = id; + node.name = parseIdentifierOrPattern(); + } + + node.initializer = parseInitializer(/*inParameter*/ false); + return finishNode(node); } function parseObjectBindingPattern(): BindingPattern { var node = createNode(SyntaxKind.ObjectBindingPattern); parseExpected(SyntaxKind.OpenBraceToken); - node.elements = parseBindingList(ParsingContext.ObjectBindingElements); + node.elements = parseDelimitedList(ParsingContext.ObjectBindingElements, parseObjectBindingElement); parseExpected(SyntaxKind.CloseBraceToken); return finishNode(node); } @@ -3316,7 +3317,7 @@ module ts { function parseArrayBindingPattern(): BindingPattern { var node = createNode(SyntaxKind.ArrayBindingPattern); parseExpected(SyntaxKind.OpenBracketToken); - node.elements = parseBindingList(ParsingContext.ArrayBindingElements); + node.elements = parseDelimitedList(ParsingContext.ArrayBindingElements, parseArrayBindingElement); parseExpected(SyntaxKind.CloseBracketToken); return finishNode(node); } From 5b38cb9a6975c0a72526db4a92190dcbf483eb29 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 16 Dec 2014 00:34:51 -0800 Subject: [PATCH 5/5] harden 'get import declaration' logic --- src/compiler/checker.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 24f5a2f528d..20033d4c80b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4742,7 +4742,9 @@ module ts { nodeLinks.importOnRightSide = undefined; getSymbolLinks(rightSide).referenced = true; - nodeLinks = getNodeLinks(rightSide.declarations[0]) + Debug.assert((rightSide.flags & SymbolFlags.Import) !== 0); + + nodeLinks = getNodeLinks(getDeclarationOfKind(rightSide, SyntaxKind.ImportDeclaration)) } } @@ -4773,7 +4775,7 @@ module ts { } if (symbolLinks.referenced) { - markLinkedImportsAsReferenced(symbol.declarations[0]); + markLinkedImportsAsReferenced(getDeclarationOfKind(symbol, SyntaxKind.ImportDeclaration)); } } @@ -8906,7 +8908,7 @@ module ts { // Mark the import as referenced so that we emit it in the final .js file. getSymbolLinks(symbol).referenced = true; // mark any import declarations that depend upon this import as referenced - markLinkedImportsAsReferenced(symbol.declarations[0]) + markLinkedImportsAsReferenced(getDeclarationOfKind(symbol, SyntaxKind.ImportDeclaration)) } }