Skip to content

Fix find all refs in shorthand properties for imports and exports #6376

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
merged 6 commits into from
Jan 14, 2016
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
10 changes: 9 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ namespace ts {
getSymbolsInScope,
getSymbolAtLocation,
getShorthandAssignmentValueSymbol,
getExportSpecifierLocalTargetSymbol,
getTypeAtLocation: getTypeOfNode,
typeToString,
getSymbolDisplayBuilder,
Expand Down Expand Up @@ -15062,11 +15063,18 @@ namespace ts {
// This is necessary as an identifier in short-hand property assignment can contains two meaning:
// property name and property value.
if (location && location.kind === SyntaxKind.ShorthandPropertyAssignment) {
return resolveEntityName((<ShorthandPropertyAssignment>location).name, SymbolFlags.Value);
return resolveEntityName((<ShorthandPropertyAssignment>location).name, SymbolFlags.Value | SymbolFlags.Alias);
}
return undefined;
}

/** Returns the target of an export specifier without following aliases */
function getExportSpecifierLocalTargetSymbol(node: ExportSpecifier): Symbol {
return (<ExportDeclaration>node.parent.parent).moduleSpecifier ?
getExternalModuleMember(<ExportDeclaration>node.parent.parent, node) :
resolveEntityName(node.propertyName || node.name, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias);
}

function getTypeOfNode(node: Node): Type {
if (isInsideWithStatementBody(node)) {
// We cannot answer semantic questions within a with block, do not proceed any further
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,7 @@ namespace ts {
getSymbolAtLocation(node: Node): Symbol;
getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[];
getShorthandAssignmentValueSymbol(location: Node): Symbol;
getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the other exposed methods use the word "target" so should this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not the symbol of the export, it is the thing it aliases. others do not have this behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "target" clarifies that any better, but I'm not familiar enough with where else we make this distinction. I just mention this because we're making this a part of our API, so we'll have to commit to this naming scheme.

getTypeAtLocation(node: Node): Type;
typeToString(type: Type, enclosingDeclaration?: Node, flags?: TypeFormatFlags): string;
symbolToString(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): string;
Expand Down
32 changes: 25 additions & 7 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5517,10 +5517,8 @@ namespace ts {
};
}

function isImportOrExportSpecifierImportSymbol(symbol: Symbol) {
return (symbol.flags & SymbolFlags.Alias) && forEach(symbol.declarations, declaration => {
return declaration.kind === SyntaxKind.ImportSpecifier || declaration.kind === SyntaxKind.ExportSpecifier;
});
function isImportSpecifierSymbol(symbol: Symbol) {
return (symbol.flags & SymbolFlags.Alias) && !!getDeclarationOfKind(symbol, SyntaxKind.ImportSpecifier);
}

function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string {
Expand Down Expand Up @@ -5964,8 +5962,17 @@ namespace ts {
let result = [symbol];

// If the symbol is an alias, add what it alaises to the list
if (isImportOrExportSpecifierImportSymbol(symbol)) {
result.push(typeChecker.getAliasedSymbol(symbol));
if (isImportSpecifierSymbol(symbol)) {
result.push(typeChecker.getAliasedSymbol(symbol));
}

// For export specifiers, the exported name can be refering to a local symbol, e.g.:
// import {a} from "mod";
// export {a as somethingElse}
// We want the *local* declaration of 'a' as declared in the import,
// *not* as declared within "mod" (or farther)
if (location.parent.kind === SyntaxKind.ExportSpecifier) {
result.push(typeChecker.getExportSpecifierLocalTargetSymbol(<ExportSpecifier>location.parent));
}

// If the location is in a context sensitive location (i.e. in an object literal) try
Expand Down Expand Up @@ -6055,13 +6062,24 @@ namespace ts {

// If the reference symbol is an alias, check if what it is aliasing is one of the search
// symbols.
if (isImportOrExportSpecifierImportSymbol(referenceSymbol)) {
if (isImportSpecifierSymbol(referenceSymbol)) {
const aliasedSymbol = typeChecker.getAliasedSymbol(referenceSymbol);
if (searchSymbols.indexOf(aliasedSymbol) >= 0) {
return aliasedSymbol;
}
}

// For export specifiers, it can be a local symbol, e.g.
// import {a} from "mod";
// export {a as somethingElse}
// We want the local target of the export (i.e. the import symbol) and not the final target (i.e. "mod".a)
if (referenceLocation.parent.kind === SyntaxKind.ExportSpecifier) {
const aliasedSymbol = typeChecker.getExportSpecifierLocalTargetSymbol(<ExportSpecifier>referenceLocation.parent);
if (searchSymbols.indexOf(aliasedSymbol) >= 0) {
return aliasedSymbol;
}
}

// If the reference location is in an object literal, try to get the contextual type for the
// object literal, lookup the property symbol in the contextual type, and use this symbol to
// compare to our searchSymbol
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/fourslash/renameImportAndExport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

////import [|a|] from "module";
////export { [|a|] };

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
}
10 changes: 10 additions & 0 deletions tests/cases/fourslash/renameImportAndShorthand.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

////import [|foo|] from 'bar';
////const bar = { [|foo|] };

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
}
10 changes: 10 additions & 0 deletions tests/cases/fourslash/renameImportNamespaceAndShorthand.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

////import * as [|foo|] from 'bar';
////const bar = { [|foo|] };

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
}
18 changes: 18 additions & 0 deletions tests/cases/fourslash/renameImportOfExportEquals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

////declare namespace N {
//// export var x: number;
////}
////declare module "mod" {
//// export = N;
////}
////declare module "test" {
//// import * as [|N|] from "mod";
//// export { [|N|] }; // Renaming N here would rename
////}

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
}
12 changes: 12 additions & 0 deletions tests/cases/fourslash/renameImportRequire.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

////import [|e|] = require("mod4");
////[|e|];
////a = { [|e|] };
////export { [|e|] };

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false);
}