Skip to content
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

Have getNameOfDeclaration return x for export default x. #18616

Merged
1 commit merged into from
Oct 6, 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
6 changes: 4 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ namespace ts {
// Should not be called on a declaration with a computed property name,
// unless it is a well known Symbol.
function getDeclarationName(node: Declaration): __String {
if (node.kind === SyntaxKind.ExportAssignment) {
return (<ExportAssignment>node).isExportEquals ? InternalSymbolName.ExportEquals : InternalSymbolName.Default;
}

const name = getNameOfDeclaration(node);
if (name) {
if (isAmbientModule(node)) {
Expand Down Expand Up @@ -261,8 +265,6 @@ namespace ts {
return InternalSymbolName.Index;
case SyntaxKind.ExportDeclaration:
return InternalSymbolName.ExportStar;
case SyntaxKind.ExportAssignment:
return (<ExportAssignment>node).isExportEquals ? InternalSymbolName.ExportEquals : InternalSymbolName.Default;
case SyntaxKind.BinaryExpression:
if (getSpecialPropertyAssignmentKind(node as BinaryExpression) === SpecialPropertyAssignmentKind.ModuleExports) {
// module.exports = ...
Expand Down
46 changes: 27 additions & 19 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4004,27 +4004,35 @@ namespace ts {
if (!declaration) {
return undefined;
}
if (isJSDocPropertyLikeTag(declaration) && declaration.name.kind === SyntaxKind.QualifiedName) {
return declaration.name.right;
}
if (declaration.kind === SyntaxKind.BinaryExpression) {
const expr = declaration as BinaryExpression;
switch (getSpecialPropertyAssignmentKind(expr)) {
case SpecialPropertyAssignmentKind.ExportsProperty:
case SpecialPropertyAssignmentKind.ThisProperty:
case SpecialPropertyAssignmentKind.Property:
case SpecialPropertyAssignmentKind.PrototypeProperty:
return (expr.left as PropertyAccessExpression).name;
default:
return undefined;
switch (declaration.kind) {
case SyntaxKind.JSDocPropertyTag:
case SyntaxKind.JSDocParameterTag: {
const { name } = declaration as JSDocPropertyLikeTag;
if (name.kind === SyntaxKind.QualifiedName) {
return name.right;
}
break;
}
case SyntaxKind.BinaryExpression: {
const expr = declaration as BinaryExpression;
switch (getSpecialPropertyAssignmentKind(expr)) {
case SpecialPropertyAssignmentKind.ExportsProperty:
case SpecialPropertyAssignmentKind.ThisProperty:
case SpecialPropertyAssignmentKind.Property:
case SpecialPropertyAssignmentKind.PrototypeProperty:
return (expr.left as PropertyAccessExpression).name;
default:
return undefined;
}
}
case SyntaxKind.JSDocTypedefTag:
return getNameOfJSDocTypedef(declaration as JSDocTypedefTag);
case SyntaxKind.ExportAssignment: {
const { expression } = declaration as ExportAssignment;
return isIdentifier(expression) ? expression : undefined;
}
}
else if (declaration.kind === SyntaxKind.JSDocTypedefTag) {
return getNameOfJSDocTypedef(declaration as JSDocTypedefTag);
}
else {
return (declaration as NamedDeclaration).name;
}
return (declaration as NamedDeclaration).name;
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,10 @@ namespace ts.FindAllReferences.Core {
/** @param allSearchSymbols set of additinal symbols for use by `includes`. */
createSearch(location: Node, symbol: Symbol, comingFrom: ImportExport | undefined, searchOptions: { text?: string, allSearchSymbols?: Symbol[] } = {}): Search {
// Note: if this is an external module symbol, the name doesn't include quotes.
const { text = stripQuotes(getDeclaredName(this.checker, symbol, location)), allSearchSymbols = undefined } = searchOptions;
const {
text = stripQuotes(unescapeLeadingUnderscores((getLocalSymbolForExportDefault(symbol) || symbol).escapedName)),
allSearchSymbols = undefined,
} = searchOptions;
const escapedText = escapeLeadingUnderscores(text);
const parents = this.options.implementations && getParentSymbolsOfPropertyAccess(location, symbol, this.checker);
return {
Expand Down
3 changes: 0 additions & 3 deletions src/services/importTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,9 +609,6 @@ namespace ts.FindAllReferences {
}

return forEach(symbol.declarations, decl => {
if (isExportAssignment(decl)) {
return isIdentifier(decl.expression) ? decl.expression.escapedText : undefined;
}
const name = getNameOfDeclaration(decl);
return name && name.kind === SyntaxKind.Identifier && name.escapedText;
});
Expand Down
20 changes: 13 additions & 7 deletions src/services/symbolDisplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,13 +345,19 @@ namespace ts.SymbolDisplay {
}
if (symbolFlags & SymbolFlags.Alias) {
addNewLineIfDisplayPartsExist();
if (symbol.declarations[0].kind === SyntaxKind.NamespaceExportDeclaration) {
displayParts.push(keywordPart(SyntaxKind.ExportKeyword));
displayParts.push(spacePart());
displayParts.push(keywordPart(SyntaxKind.NamespaceKeyword));
}
else {
displayParts.push(keywordPart(SyntaxKind.ImportKeyword));
switch (symbol.declarations[0].kind) {
case SyntaxKind.NamespaceExportDeclaration:
displayParts.push(keywordPart(SyntaxKind.ExportKeyword));
displayParts.push(spacePart());
displayParts.push(keywordPart(SyntaxKind.NamespaceKeyword));
break;
case SyntaxKind.ExportAssignment:
displayParts.push(keywordPart(SyntaxKind.ExportKeyword));
displayParts.push(spacePart());
displayParts.push(keywordPart((symbol.declarations[0] as ExportAssignment).isExportEquals ? SyntaxKind.EqualsToken : SyntaxKind.DefaultKeyword));
break;
default:
displayParts.push(keywordPart(SyntaxKind.ImportKeyword));
}
displayParts.push(spacePart());
addFullSymbolName(symbol);
Expand Down
44 changes: 22 additions & 22 deletions tests/baselines/reference/duplicateExportAssignments.errors.txt
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
tests/cases/conformance/externalModules/foo1.ts(3,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo1.ts(4,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo2.ts(3,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo2.ts(4,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo3.ts(7,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo3.ts(8,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo4.ts(1,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo4.ts(8,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(4,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(5,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(6,1): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo1.ts(3,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo1.ts(4,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo2.ts(3,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo2.ts(4,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo3.ts(7,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo3.ts(8,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo4.ts(1,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo4.ts(8,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(4,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(5,10): error TS2300: Duplicate identifier 'export='.
tests/cases/conformance/externalModules/foo5.ts(6,10): error TS2300: Duplicate identifier 'export='.


==== tests/cases/conformance/externalModules/foo1.ts (2 errors) ====
var x = 10;
var y = 20;
export = x;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = y;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.

==== tests/cases/conformance/externalModules/foo2.ts (2 errors) ====
var x = 10;
class y {};
export = x;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = y;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.

==== tests/cases/conformance/externalModules/foo3.ts (2 errors) ====
Expand All @@ -39,15 +39,15 @@ tests/cases/conformance/externalModules/foo5.ts(6,1): error TS2300: Duplicate id
y: number;
}
export = x;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = y;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.

==== tests/cases/conformance/externalModules/foo4.ts (2 errors) ====
export = x;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
function x(){
return 42;
Expand All @@ -56,20 +56,20 @@ tests/cases/conformance/externalModules/foo5.ts(6,1): error TS2300: Duplicate id
return 42;
}
export = y;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.

==== tests/cases/conformance/externalModules/foo5.ts (3 errors) ====
var x = 5;
var y = "test";
var z = {};
export = x;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = y;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = z;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.

2 changes: 1 addition & 1 deletion tests/baselines/reference/es5-commonjs7.symbols
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=== tests/cases/compiler/test.d.ts ===
export default undefined;
>undefined : Symbol(default)
>undefined : Symbol(undefined)

export var __esModule;
>__esModule : Symbol(__esModule, Decl(test.d.ts, 1, 10))
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/exportDefaultVariable.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ declare var io: any;

declare module 'module' {
export default io;
>io : Symbol(default, Decl(exportDefaultVariable.ts, 2, 11))
>io : Symbol(io, Decl(exportDefaultVariable.ts, 2, 11))
}

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
tests/cases/compiler/a.js(1,22): error TS2528: A module cannot have multiple default exports.
tests/cases/compiler/a.js(1,22): error TS2652: Merged declaration 'a' cannot include a default export declaration. Consider adding a separate 'export default a' declaration instead.
tests/cases/compiler/a.js(3,1): error TS2528: A module cannot have multiple default exports.
tests/cases/compiler/a.js(3,15): error TS2528: A module cannot have multiple default exports.
tests/cases/compiler/a.js(3,16): error TS1109: Expression expected.
tests/cases/compiler/a.js(3,20): error TS2652: Merged declaration 'a' cannot include a default export declaration. Consider adding a separate 'export default a' declaration instead.

Expand All @@ -13,7 +13,7 @@ tests/cases/compiler/a.js(3,20): error TS2652: Merged declaration 'a' cannot inc
!!! error TS2652: Merged declaration 'a' cannot include a default export declaration. Consider adding a separate 'export default a' declaration instead.
}
export default var a = 10;
~~~~~~~~~~~~~~
!!! error TS2528: A module cannot have multiple default exports.
~~~
!!! error TS1109: Expression expected.
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/multipleDefaultExports01.errors.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
tests/cases/conformance/es6/modules/m1.ts(1,22): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/es6/modules/m1.ts(5,25): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/es6/modules/m1.ts(10,1): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/es6/modules/m1.ts(10,16): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/es6/modules/m2.ts(3,1): error TS2348: Value of type 'typeof foo' is not callable. Did you mean to include 'new'?


Expand All @@ -19,7 +19,7 @@ tests/cases/conformance/es6/modules/m2.ts(3,1): error TS2348: Value of type 'typ

var x = 10;
export default x;
~~~~~~~~~~~~~~~~~
~
!!! error TS2528: A module cannot have multiple default exports.

==== tests/cases/conformance/es6/modules/m2.ts (1 errors) ====
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/multipleExportAssignments.ts(13,1): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignments.ts(14,1): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignments.ts(13,10): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignments.ts(14,10): error TS2300: Duplicate identifier 'export='.


==== tests/cases/compiler/multipleExportAssignments.ts (2 errors) ====
Expand All @@ -16,10 +16,10 @@ tests/cases/compiler/multipleExportAssignments.ts(14,1): error TS2300: Duplicate
test2(): connectModule;
};
export = server;
~~~~~~~~~~~~~~~~
~~~~~~
!!! error TS2300: Duplicate identifier 'export='.
export = connectExport;
~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~
!!! error TS2300: Duplicate identifier 'export='.


Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
tests/cases/compiler/multipleExportAssignmentsInAmbientDeclaration.ts(4,5): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignmentsInAmbientDeclaration.ts(5,5): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignmentsInAmbientDeclaration.ts(4,14): error TS2300: Duplicate identifier 'export='.
tests/cases/compiler/multipleExportAssignmentsInAmbientDeclaration.ts(5,14): error TS2300: Duplicate identifier 'export='.


==== tests/cases/compiler/multipleExportAssignmentsInAmbientDeclaration.ts (2 errors) ====
declare module "m1" {
var a: number
var b: number;
export = a;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
export = b;
~~~~~~~~~~~
~
!!! error TS2300: Duplicate identifier 'export='.
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/typeAliasExport.symbols
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
=== tests/cases/compiler/typeAliasExport.ts ===
declare module "a" {
export default undefined
>undefined : Symbol(default)
>undefined : Symbol(undefined)

export var a;
>a : Symbol(a, Decl(typeAliasExport.ts, 2, 12), Decl(typeAliasExport.ts, 2, 15))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export default function [|{| "isWriteAccess": true, "isDefinition": true |}f|]() {}
////export [|{| "isWriteAccess": true, "isDefinition": true |}default|] function [|{| "isWriteAccess": true, "isDefinition": true |}f|]() {}

// @Filename: /b.ts
////export import a = require("./a");
Expand All @@ -13,16 +13,17 @@
////declare const x: { [|{| "isWriteAccess": true, "isDefinition": true |}default|]: number };
////x.[|default|];

const [r0, r1, r2, r3] = test.ranges();
const [r0, r1, r2, r3, r4] = test.ranges();

verify.singleReferenceGroup("function f(): void", [r0, r1]);
verify.singleReferenceGroup("(property) default: number", [r2, r3]);
verify.referenceGroups([r0], [{ definition: "function f(): void", ranges: [r1, r2] }]);
verify.singleReferenceGroup("function f(): void", [r1, r2]);
verify.singleReferenceGroup("(property) default: number", [r3, r4]);

verify.rangesAreRenameLocations([r0]);
verify.rangesAreRenameLocations([r1]);

// Can't rename a default import.
goTo.rangeStart(r1);
goTo.rangeStart(r2);
verify.renameInfoFailed();

// Can rename a default property.
verify.rangesAreRenameLocations([r2, r3]);
verify.rangesAreRenameLocations([r3, r4]);
6 changes: 2 additions & 4 deletions tests/cases/fourslash/findAllRefsForDefaultExport04.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ verify.referenceGroups([r0, r2], [
{ definition: "import a", ranges: [r3, r4] }
]);
verify.referenceGroups(r1, [
// TODO:GH#17990
{ definition: "import default", ranges: [r1] },
{ definition: "export default a", ranges: [r1] },
{ definition: "import a", ranges: [r3, r4] },
]);
verify.referenceGroups([r3, r4], [
{ definition: "import a", ranges: [r3, r4] },
// TODO:GH#17990
{ definition: "import default", ranges: [r1] },
{ definition: "export default a", ranges: [r1] },
]);
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@
const ranges = test.ranges();
const [r0, r1, r2] = ranges;
verify.referenceGroups(r0, [{ definition: '(property) ["foo"]: number', ranges }]);
verify.referenceGroups([r1, r2], undefined); // TODO: fix
verify.referenceGroups([r1, r2], [
// TODO: these are the same thing, should be in the same group.
{ definition: "(property) [\"foo\"]: number", ranges: [r0] },
{ definition: "(property) [\"foo\"]: number", ranges: [r1, r2] },
]);