diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index a87d0a9e1daa6..41048899e0f0b 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -759,7 +759,18 @@ namespace FourSlash { ts.zipWith(endMarkers, definitions, (endMarker, definition, i) => { const marker = this.getMarkerByName(endMarker); if (ts.comparePaths(marker.fileName, definition.fileName, /*ignoreCase*/ true) !== ts.Comparison.EqualTo || marker.position !== definition.textSpan.start) { - this.raiseError(`${testName} failed for definition ${endMarker} (${i}): expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}`); + const filesToDisplay = ts.deduplicate([marker.fileName, definition.fileName], ts.equateValues); + const markers = [{ text: "EXPECTED", fileName: marker.fileName, position: marker.position }, { text: "ACTUAL", fileName: definition.fileName, position: definition.textSpan.start }]; + const text = filesToDisplay.map(fileName => { + const markersToRender = markers.filter(m => m.fileName === fileName).sort((a, b) => b.position - a.position); + let fileContent = this.getFileContent(fileName); + for (const marker of markersToRender) { + fileContent = fileContent.slice(0, marker.position) + `\x1b[1;4m/*${marker.text}*/\x1b[0;31m` + fileContent.slice(marker.position); + } + return `// @Filename: ${fileName}\n${fileContent}`; + }).join("\n\n"); + + this.raiseError(`${testName} failed for definition ${endMarker} (${i}): expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`); } }); } @@ -777,7 +788,7 @@ namespace FourSlash { const fileContent = this.getFileContent(startFile); const spanContent = fileContent.slice(defs.textSpan.start, ts.textSpanEnd(defs.textSpan)); const spanContentWithMarker = spanContent.slice(0, marker.position - defs.textSpan.start) + `/*${startMarkerName}*/` + spanContent.slice(marker.position - defs.textSpan.start); - const suggestedFileContent = (fileContent.slice(0, defs.textSpan.start) + `\x1b[1;4m[|${spanContentWithMarker}|]\x1b[31m` + fileContent.slice(ts.textSpanEnd(defs.textSpan))) + const suggestedFileContent = (fileContent.slice(0, defs.textSpan.start) + `\x1b[1;4m[|${spanContentWithMarker}|]\x1b[0;31m` + fileContent.slice(ts.textSpanEnd(defs.textSpan))) .split(/\r?\n/).map(line => " ".repeat(6) + line).join(ts.sys.newLine); this.raiseError(`goToDefinitionsAndBoundSpan failed. Found a starting TextSpan around '${spanContent}' in '${startFile}' (at position ${defs.textSpan.start}). ` + `If this is the correct input span, put a fourslash range around it: \n\n${suggestedFileContent}\n`); diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index f0ec33c88842b..de473a79c97e8 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -92,9 +92,16 @@ namespace ts.GoToDefinition { getDefinitionFromSymbol(typeChecker, propertySymbol, node)); } } + return getDefinitionFromSymbol(typeChecker, symbol, node); } + function isShorthandPropertyAssignmentOfModuleExports(symbol: Symbol): boolean { + const shorthandProperty = tryCast(symbol.valueDeclaration, isShorthandPropertyAssignment); + const binaryExpression = tryCast(shorthandProperty?.parent.parent, isAssignmentExpression); + return !!binaryExpression && getAssignmentDeclarationKind(binaryExpression) === AssignmentDeclarationKind.ModuleExports; + } + /** * True if we should not add definitions for both the signature symbol and the definition symbol. * True for `const |f = |() => 0`, false for `function |f() {} const |g = f;`. @@ -197,15 +204,29 @@ namespace ts.GoToDefinition { } function getSymbol(node: Node, checker: TypeChecker): Symbol | undefined { - const symbol = checker.getSymbolAtLocation(node); + let symbol = checker.getSymbolAtLocation(node); // If this is an alias, and the request came at the declaration location // 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 && symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) { - const aliased = checker.getAliasedSymbol(symbol); - if (aliased.declarations) { - return aliased; + while (symbol) { + if (symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) { + const aliased = checker.getAliasedSymbol(symbol); + if (!aliased.declarations) { + break; + } + symbol = aliased; + } + else if (isShorthandPropertyAssignmentOfModuleExports(symbol)) { + // Skip past `module.exports = { Foo }` even though 'Foo' is not a real alias + const shorthandTarget = checker.resolveName(symbol.name, symbol.valueDeclaration, SymbolFlags.Value, /*excludeGlobals*/ false); + if (!some(shorthandTarget?.declarations)) { + break; + } + symbol = shorthandTarget; + } + else { + break; } } return symbol; diff --git a/tests/cases/fourslash/goToDefinitionDestructuredRequire1.ts b/tests/cases/fourslash/goToDefinitionDestructuredRequire1.ts new file mode 100644 index 0000000000000..68c6cb35ca2b3 --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionDestructuredRequire1.ts @@ -0,0 +1,13 @@ +/// + +// @allowJs: true + +// @Filename: util.js +//// class /*2*/Util {} +//// module.exports = { Util }; + +// @Filename: index.js +//// const { Util } = require('./util'); +//// new [|Util/*1*/|]() + +verify.goToDefinition("1", "2"); diff --git a/tests/cases/fourslash/goToDefinitionDestructuredRequire2.ts b/tests/cases/fourslash/goToDefinitionDestructuredRequire2.ts new file mode 100644 index 0000000000000..2fa27564ff9bb --- /dev/null +++ b/tests/cases/fourslash/goToDefinitionDestructuredRequire2.ts @@ -0,0 +1,17 @@ +/// + +// @allowJs: true + +// @Filename: util.js +//// class /*2*/Util {} +//// module.exports = { Util }; + +// @Filename: reexport.js +//// const { Util } = require('./util'); +//// module.exports = { Util }; + +// @Filename: index.js +//// const { Util } = require('./reexport'); +//// new [|Util/*1*/|]() + +verify.goToDefinition("1", "2"); diff --git a/tests/cases/fourslash/goToDefinitionImportedNames11.ts b/tests/cases/fourslash/goToDefinitionImportedNames11.ts index f34b3543881a6..1eb3494c3a74e 100644 --- a/tests/cases/fourslash/goToDefinitionImportedNames11.ts +++ b/tests/cases/fourslash/goToDefinitionImportedNames11.ts @@ -2,10 +2,10 @@ // @allowjs: true // @Filename: a.js -//// class Class { +//// class /*classDefinition*/Class { //// f; //// } -//// module.exports = { /*classDefinition*/Class }; +//// module.exports = { Class }; // @Filename: b.js ////const { Class } = require("./a");