From 7d31b5c8a3e4f05e59ce86519306432ff420c268 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 6 Jan 2016 10:27:31 -0800 Subject: [PATCH 1/4] Fix find all refs in shorthand properties for imports and exports --- src/compiler/checker.ts | 10 +++++- src/compiler/types.ts | 1 + src/services/services.ts | 31 ++++++++++++++----- .../cases/fourslash/renameImportAndExport.ts | 10 ++++++ .../fourslash/renameImportAndShorthand.ts | 10 ++++++ .../renameImportNamespaceAndShorthand.ts | 10 ++++++ 6 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/renameImportAndExport.ts create mode 100644 tests/cases/fourslash/renameImportAndShorthand.ts create mode 100644 tests/cases/fourslash/renameImportNamespaceAndShorthand.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e4ad333988..201ad348be 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -82,6 +82,7 @@ namespace ts { getSymbolsInScope, getSymbolAtLocation, getShorthandAssignmentValueSymbol, + getExportSpecifierLocalTargetSymbol, getTypeAtLocation: getTypeOfNode, typeToString, getSymbolDisplayBuilder, @@ -15007,11 +15008,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((location).name, SymbolFlags.Value); + return resolveEntityName((location).name, SymbolFlags.Value | SymbolFlags.Alias); } return undefined; } + /** Returns the target of an export specifier without following aliases */ + function getExportSpecifierLocalTargetSymbol(node: ExportSpecifier): Symbol { + return (node.parent.parent).moduleSpecifier ? + getExternalModuleMember(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 diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 24f70373a8..3d074f5048 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1725,6 +1725,7 @@ namespace ts { getSymbolAtLocation(node: Node): Symbol; getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[]; getShorthandAssignmentValueSymbol(location: Node): Symbol; + getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol; getTypeAtLocation(node: Node): Type; typeToString(type: Type, enclosingDeclaration?: Node, flags?: TypeFormatFlags): string; symbolToString(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): string; diff --git a/src/services/services.ts b/src/services/services.ts index 0fd4df1990..f89ba4e94f 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5490,10 +5490,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) && forEach(symbol.declarations, declaration => declaration.kind === SyntaxKind.ImportSpecifier); } function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string { @@ -5937,8 +5935,16 @@ 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, 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 (location.parent.kind === SyntaxKind.ExportSpecifier) { + result.push(typeChecker.getExportSpecifierLocalTargetSymbol(location.parent)); } // If the location is in a context sensitive location (i.e. in an object literal) try @@ -6028,13 +6034,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(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 diff --git a/tests/cases/fourslash/renameImportAndExport.ts b/tests/cases/fourslash/renameImportAndExport.ts new file mode 100644 index 0000000000..495e15c1e7 --- /dev/null +++ b/tests/cases/fourslash/renameImportAndExport.ts @@ -0,0 +1,10 @@ +/// + +////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); +} diff --git a/tests/cases/fourslash/renameImportAndShorthand.ts b/tests/cases/fourslash/renameImportAndShorthand.ts new file mode 100644 index 0000000000..bc4746aebd --- /dev/null +++ b/tests/cases/fourslash/renameImportAndShorthand.ts @@ -0,0 +1,10 @@ +/// + +////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); +} diff --git a/tests/cases/fourslash/renameImportNamespaceAndShorthand.ts b/tests/cases/fourslash/renameImportNamespaceAndShorthand.ts new file mode 100644 index 0000000000..a6b06c1140 --- /dev/null +++ b/tests/cases/fourslash/renameImportNamespaceAndShorthand.ts @@ -0,0 +1,10 @@ +/// + +////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); +} From f09628900a37046d1c29af8338dffdea87d289db Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 6 Jan 2016 10:48:24 -0800 Subject: [PATCH 2/4] Add new test for import..require --- tests/cases/fourslash/renameImportRequire.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/cases/fourslash/renameImportRequire.ts diff --git a/tests/cases/fourslash/renameImportRequire.ts b/tests/cases/fourslash/renameImportRequire.ts new file mode 100644 index 0000000000..c26cc80b61 --- /dev/null +++ b/tests/cases/fourslash/renameImportRequire.ts @@ -0,0 +1,12 @@ +/// + +////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); +} From cdc33f51d2c79a655ed09769b357d1636637f29e Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 7 Jan 2016 13:30:54 -0800 Subject: [PATCH 3/4] Code review comments --- src/services/services.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index f89ba4e94f..0096838f31 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5491,7 +5491,7 @@ namespace ts { } function isImportSpecifierSymbol(symbol: Symbol) { - return (symbol.flags & SymbolFlags.Alias) && forEach(symbol.declarations, declaration => declaration.kind === SyntaxKind.ImportSpecifier); + return (symbol.flags & SymbolFlags.Alias) && !!getDeclarationOfKind(symbol, SyntaxKind.ImportSpecifier); } function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string { @@ -5939,10 +5939,11 @@ namespace ts { result.push(typeChecker.getAliasedSymbol(symbol)); } - // For export specifiers, it can be a local symbol, e.g. + // 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 target of the export (i.e. the import symbol) and not the final target (i.e. "mod".a) + // 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(location.parent)); } From 9b01783e2d6cf94ec0aa720b6e1ef6f03c59e1ba Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 7 Jan 2016 13:31:14 -0800 Subject: [PATCH 4/4] Add test for renaming accorss modules using export= --- .../fourslash/renameImportOfExportEquals.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tests/cases/fourslash/renameImportOfExportEquals.ts diff --git a/tests/cases/fourslash/renameImportOfExportEquals.ts b/tests/cases/fourslash/renameImportOfExportEquals.ts new file mode 100644 index 0000000000..9d71ee907c --- /dev/null +++ b/tests/cases/fourslash/renameImportOfExportEquals.ts @@ -0,0 +1,18 @@ +/// + +////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); +}