From d6fa91edcd1cf3bf277958881dc1e5f81e8a183a Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 11 May 2017 15:48:17 -0700 Subject: [PATCH 1/2] goToDefinition: Skip default and `=` imports --- src/services/goToDefinition.ts | 39 ++++++++++++------- .../cases/fourslash/goToDefinitionImports.ts | 26 +++++++++++++ 2 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/goToDefinitionImports.ts diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 07b1e4448bd..946924bcf87 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -50,20 +50,8 @@ namespace ts.GoToDefinition { // get the aliased symbol instead. This allows for goto def on an import e.g. // import {A, B} from "mod"; // to jump to the implementation directly. - if (symbol.flags & SymbolFlags.Alias) { - const declaration = symbol.declarations[0]; - - // Go to the original declaration for cases: - // - // (1) when the aliased symbol was declared in the location(parent). - // (2) when the aliased symbol is originating from a named import. - // - if (node.kind === SyntaxKind.Identifier && - (node.parent === declaration || - (declaration.kind === SyntaxKind.ImportSpecifier && declaration.parent && declaration.parent.kind === SyntaxKind.NamedImports))) { - - symbol = typeChecker.getAliasedSymbol(symbol); - } + if (symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) { + symbol = typeChecker.getAliasedSymbol(symbol); } // Because name in short-hand property assignment has two different meanings: property name and property value, @@ -136,6 +124,29 @@ namespace ts.GoToDefinition { return getDefinitionFromSymbol(typeChecker, type.symbol, node); } + // Go to the original declaration for cases: + // + // (1) when the aliased symbol was declared in the location(parent). + // (2) when the aliased symbol is originating from an import. + // + function shouldSkipAlias(node: Node, declaration: Node): boolean { + if (node.kind !== SyntaxKind.Identifier) { + return false; + } + if (node.parent === declaration) { + return true; + } + switch (declaration.kind) { + case SyntaxKind.ImportClause: + case SyntaxKind.ImportEqualsDeclaration: + return true; + case SyntaxKind.ImportSpecifier: + return declaration.parent.kind === SyntaxKind.NamedImports; + default: + return false; + } + } + function getDefinitionFromSymbol(typeChecker: TypeChecker, symbol: Symbol, node: Node): DefinitionInfo[] { const result: DefinitionInfo[] = []; const declarations = symbol.getDeclarations(); diff --git a/tests/cases/fourslash/goToDefinitionImports.ts b/tests/cases/fourslash/goToDefinitionImports.ts new file mode 100644 index 00000000000..4fdaedb5c3d --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionImports.ts @@ -0,0 +1,26 @@ +/// + +// @Filename: /a.ts +////export default function /*fDef*/f() {} +////export const /*xDef*/x = 0; + +// @Filename: /b.ts +/////*bDef*/declare const b: number; +////export = b; + +// @Filename: /b.ts +////import f, { x } from "./a"; +////import * as /*aDef*/a from "./a"; +////import b = require("./b"); +/////*fUse*/f; +/////*xUse*/x; +/////*aUse*/a; +/////*bUse*/b; + +verify.goToDefinition({ + aUse: "aDef", // Namespace import isn't "skipped" + fUse: "fDef", + xUse: "xDef", + bUse: "bDef", +}); + From aaf6b83cb573c33c838f85a05d5029135a620b7c Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 12 May 2017 07:32:00 -0700 Subject: [PATCH 2/2] Don't goto aliased symbol with no declarations; and update tests --- src/services/goToDefinition.ts | 5 ++++- tests/cases/fourslash/ambientShorthandGotoDefinition.ts | 4 ++-- tests/cases/fourslash/goToDefinition_untypedModule.ts | 6 +++--- tests/cases/fourslash/quickInfoMeaning.ts | 8 ++++---- .../fourslash/tsxGoToDefinitionClassInDifferentFile.ts | 4 ++-- tests/cases/fourslash/untypedModuleImport.ts | 2 +- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 946924bcf87..d0901de9a47 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -51,7 +51,10 @@ namespace ts.GoToDefinition { // import {A, B} from "mod"; // to jump to the implementation directly. if (symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) { - symbol = typeChecker.getAliasedSymbol(symbol); + const aliased = typeChecker.getAliasedSymbol(symbol); + if (aliased.declarations) { + symbol = aliased; + } } // Because name in short-hand property assignment has two different meanings: property name and property value, diff --git a/tests/cases/fourslash/ambientShorthandGotoDefinition.ts b/tests/cases/fourslash/ambientShorthandGotoDefinition.ts index 410fc932cec..74e348beb74 100644 --- a/tests/cases/fourslash/ambientShorthandGotoDefinition.ts +++ b/tests/cases/fourslash/ambientShorthandGotoDefinition.ts @@ -12,7 +12,7 @@ verify.quickInfoAt("useFoo", "import foo"); verify.goToDefinition({ - useFoo: "importFoo", + useFoo: "module", importFoo: "module" }); @@ -27,6 +27,6 @@ verify.goToDefinition({ verify.quickInfoAt("useBang", "import bang = require(\"jquery\")"); verify.goToDefinition({ - useBang: "importBang", + useBang: "module", importBang: "module" }); diff --git a/tests/cases/fourslash/goToDefinition_untypedModule.ts b/tests/cases/fourslash/goToDefinition_untypedModule.ts index b4571438434..fe29f7104cf 100644 --- a/tests/cases/fourslash/goToDefinition_untypedModule.ts +++ b/tests/cases/fourslash/goToDefinition_untypedModule.ts @@ -4,7 +4,7 @@ ////not read // @Filename: /a.ts -////import { f } from "foo"; -/////**/f(); +////import { /*def*/f } from "foo"; +/////*use*/f(); -verify.goToDefinition("", []); +verify.goToDefinition("use", "def"); diff --git a/tests/cases/fourslash/quickInfoMeaning.ts b/tests/cases/fourslash/quickInfoMeaning.ts index a3b9f988e1d..2c7aa5a0ccd 100644 --- a/tests/cases/fourslash/quickInfoMeaning.ts +++ b/tests/cases/fourslash/quickInfoMeaning.ts @@ -8,13 +8,13 @@ // @Filename: foo.d.ts ////declare const /*foo_value_declaration*/foo: number; ////declare module "foo_module" { -//// interface I { x: number; y: number } +//// interface /*foo_type_declaration*/I { x: number; y: number } //// export = I; ////} // @Filename: foo_user.ts /////// -////import /*foo_type_declaration*/foo = require("foo_module"); +////import foo = require("foo_module"); ////const x = foo/*foo_value*/; ////const i: foo/*foo_type*/ = { x: 1, y: 2 }; @@ -39,13 +39,13 @@ verify.goToDefinitionIs("foo_type_declaration"); // @Filename: bar.d.ts ////declare interface /*bar_type_declaration*/bar { x: number; y: number } ////declare module "bar_module" { -//// const x: number; +//// const /*bar_value_declaration*/x: number; //// export = x; ////} // @Filename: bar_user.ts /////// -////import /*bar_value_declaration*/bar = require("bar_module"); +////import bar = require("bar_module"); ////const x = bar/*bar_value*/; ////const i: bar/*bar_type*/ = { x: 1, y: 2 }; diff --git a/tests/cases/fourslash/tsxGoToDefinitionClassInDifferentFile.ts b/tests/cases/fourslash/tsxGoToDefinitionClassInDifferentFile.ts index f217e4096fc..7aa1c11fdc3 100644 --- a/tests/cases/fourslash/tsxGoToDefinitionClassInDifferentFile.ts +++ b/tests/cases/fourslash/tsxGoToDefinitionClassInDifferentFile.ts @@ -3,10 +3,10 @@ // @jsx: preserve // @Filename: C.tsx -////export default class C {} +////export default class /*def*/C {} // @Filename: a.tsx -////import /*def*/C from "./C"; +////import C from "./C"; ////const foo = ; verify.noErrors(); diff --git a/tests/cases/fourslash/untypedModuleImport.ts b/tests/cases/fourslash/untypedModuleImport.ts index 4dac6ac76bb..5501cec4792 100644 --- a/tests/cases/fourslash/untypedModuleImport.ts +++ b/tests/cases/fourslash/untypedModuleImport.ts @@ -17,6 +17,6 @@ const [r0, r1, r2] = test.ranges(); verify.singleReferenceGroup('"foo"', [r1]); goTo.marker("foo"); -verify.goToDefinitionIs([]); +verify.goToDefinitionIs("foo"); verify.quickInfoIs("import foo"); verify.singleReferenceGroup("import foo", [r0, r2]);