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

Find Source Definition #48264

Merged
merged 49 commits into from
Apr 14, 2022
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
81ec4fd
Prototype resolving to JS when go-to-def aliases all resolve to ambie…
andrewbranch Feb 25, 2022
422d845
Add test infrastructure
andrewbranch Mar 1, 2022
1b43352
Start fleshing out test coverage
andrewbranch Mar 2, 2022
7aeb92f
Fix some go-to-def stuff
andrewbranch Mar 8, 2022
f2915e6
Finish lodash test case
andrewbranch Mar 10, 2022
309c4bb
Make go-to-implementation never return ambient results
andrewbranch Mar 10, 2022
381799d
Build new functionality into go-to-implementation
andrewbranch Mar 11, 2022
0727702
Update baselines
andrewbranch Mar 11, 2022
ebeda0b
Two more test cases
andrewbranch Mar 14, 2022
03ea413
Refine definition searches for unresolved imports
andrewbranch Mar 14, 2022
e5556b8
Revert "Build new functionality into go-to-implementation"
andrewbranch Mar 14, 2022
6104a5c
Fix tests
andrewbranch Mar 14, 2022
bea3de0
Merge branch 'main' into go-to-js
andrewbranch Mar 14, 2022
4e64659
Revert go-to-implementation changes
andrewbranch Mar 14, 2022
1cb2ba6
Wow a bunch of code was unnecessary
andrewbranch Mar 14, 2022
7e57890
Update baselines and go-to-def test
andrewbranch Mar 15, 2022
43c01e2
Fix navigation on symbols that are not aliases but resolve through al…
andrewbranch Mar 16, 2022
34c6cfd
Temporarily replace go-to-def with new command implementation
andrewbranch Mar 15, 2022
7357593
Revert "Temporarily replace go-to-def with new command implementation"
andrewbranch Mar 16, 2022
d14e43d
Revert "Wow a bunch of code was unnecessary"
andrewbranch Mar 21, 2022
4e1cf1c
Bring back some deleted code needed for a new test case
andrewbranch Mar 21, 2022
52a79d5
Clean up a little
andrewbranch Mar 22, 2022
e011f2d
Rename more stuff
andrewbranch Mar 22, 2022
05cfd27
Update test
andrewbranch Mar 22, 2022
b95f496
Update API baseline
andrewbranch Mar 22, 2022
b883898
Temporarily replace go-to-def with new command implementation
andrewbranch Mar 15, 2022
373bca2
PR review fixes
andrewbranch Apr 4, 2022
c05adbd
Merge branch 'main' into go-to-js
andrewbranch Apr 4, 2022
3a85dc8
Fix getTopMostDeclarationNamesInFile
andrewbranch Apr 4, 2022
7dd9db8
Rename local
andrewbranch Apr 4, 2022
0f3b29c
Use hash set
andrewbranch Apr 4, 2022
234c139
Remove option from commandLineParser
andrewbranch Apr 4, 2022
92ff999
Merge branch 'main' into go-to-js
andrewbranch Apr 6, 2022
1698930
Keep noDtsResolution project around
andrewbranch Apr 7, 2022
859ed79
Handle AuxiliaryProject kind in ScriptInfo getDefaultProject etc.
andrewbranch Apr 7, 2022
7be215a
Do not run updateGraph in the background for AuxiliaryProject
andrewbranch Apr 7, 2022
8f4e622
Don’t create auxiliary project outside of semantic mode
andrewbranch Apr 7, 2022
d633356
No-op on scheduled invalidation
andrewbranch Apr 7, 2022
b89b7cb
Add comments to unit test
andrewbranch Apr 7, 2022
a59024c
Sync compiler options to auxiliary project
andrewbranch Apr 7, 2022
7f290bb
Fix case sensitivity
andrewbranch Apr 7, 2022
c5bddbe
Update extensionIsOk with new file extensions
andrewbranch Apr 7, 2022
24c0d8a
PR feedback
andrewbranch Apr 13, 2022
f6bd9d8
Update API baseline
andrewbranch Apr 13, 2022
2cb2048
Mark scheduleInvalidateResolutionsOfFailedLookupLocations internal
andrewbranch Apr 13, 2022
f9f4592
Use same heuristics on property accesses of loosely-resolvable aliase…
andrewbranch Apr 14, 2022
e302f56
Rename command, and no need to return the bound span
andrewbranch Apr 14, 2022
3f15ed3
Update API baseline
andrewbranch Apr 14, 2022
f80c9ef
Merge branch 'main' into go-to-js
andrewbranch Apr 14, 2022
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9162,7 +9162,7 @@ namespace ts {
}
}
const sourceTypes = some(constructorTypes, t => !!(t.flags & ~TypeFlags.Nullable)) ? constructorTypes : types; // TODO: GH#18217
type = getUnionType(sourceTypes!, UnionReduction.Subtype);
type = getUnionType(sourceTypes!);
}
}
const widened = getWidenedType(addOptionality(type, /*isProperty*/ false, definedInMethod && !definedInConstructor));
Expand Down
43 changes: 33 additions & 10 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ namespace ts {
JavaScript, /** '.js' or '.jsx' */
Json, /** '.json' */
TSConfig, /** '.json' with `tsconfig` used instead of `index` */
DtsOnly /** Only '.d.ts' */
DtsOnly, /** Only '.d.ts' */
TsOnly, /** '.[cm]tsx?' but not .d.ts variants */
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure you've already bike-shedded this, but I think I'd prefer "TypeScriptNoDts".

}

interface PathAndPackageId {
Expand Down Expand Up @@ -1290,7 +1291,19 @@ namespace ts {
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference): ResolvedModuleWithFailedLookupLocations;
/* @internal */ export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations; // eslint-disable-line @typescript-eslint/unified-signatures
export function nodeModuleNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache?: ModuleResolutionCache, redirectedReference?: ResolvedProjectReference, lookupConfig?: boolean): ResolvedModuleWithFailedLookupLocations {
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, lookupConfig ? tsconfigExtensions : (compilerOptions.resolveJsonModule ? tsPlusJsonExtensions : tsExtensions), redirectedReference);
let extensions;
if (lookupConfig) {
extensions = tsconfigExtensions;
}
else if (compilerOptions.noDtsResolution) {
extensions = [Extensions.TsOnly];
if (compilerOptions.allowJs) extensions.push(Extensions.JavaScript);
if (compilerOptions.resolveJsonModule) extensions.push(Extensions.Json);
}
else {
extensions = compilerOptions.resolveJsonModule ? tsPlusJsonExtensions : tsExtensions;
}
return nodeModuleNameResolverWorker(NodeResolutionFeatures.None, moduleName, getDirectoryPath(containingFile), compilerOptions, host, cache, extensions, redirectedReference);
}

function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleName: string, containingDirectory: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost, cache: ModuleResolutionCache | undefined, extensions: Extensions[], redirectedReference: ResolvedProjectReference | undefined): ResolvedModuleWithFailedLookupLocations {
Expand All @@ -1299,14 +1312,19 @@ namespace ts {
const failedLookupLocations: string[] = [];
// conditions are only used by the node12/nodenext resolver - there's no priority order in the list,
//it's essentially a set (priority is determined by object insertion order in the object we look at).
const conditions = features & NodeResolutionFeatures.EsmMode ? ["node", "import", "types"] : ["node", "require", "types"];
if (compilerOptions.noDtsResolution) {
conditions.pop();
}

const state: ModuleResolutionState = {
compilerOptions,
host,
traceEnabled,
failedLookupLocations,
packageJsonInfoCache: cache,
features,
conditions: features & NodeResolutionFeatures.EsmMode ? ["node", "import", "types"] : ["node", "require", "types"]
conditions,
};

const result = forEach(extensions, ext => tryResolve(ext));
Expand Down Expand Up @@ -1533,20 +1551,22 @@ namespace ts {
default: return tryExtension(Extension.Dts);
}
case Extensions.TypeScript:
case Extensions.TsOnly:
const useDts = extensions === Extensions.TypeScript;
switch (originalExtension) {
case Extension.Mjs:
case Extension.Mts:
case Extension.Dmts:
return tryExtension(Extension.Mts) || tryExtension(Extension.Dmts);
return tryExtension(Extension.Mts) || (useDts ? tryExtension(Extension.Dmts) : undefined);
case Extension.Cjs:
case Extension.Cts:
case Extension.Dcts:
return tryExtension(Extension.Cts) || tryExtension(Extension.Dcts);
return tryExtension(Extension.Cts) || (useDts ? tryExtension(Extension.Dcts) : undefined);
case Extension.Json:
candidate += Extension.Json;
return tryExtension(Extension.Dts);
return useDts ? tryExtension(Extension.Dts) : undefined;
default:
return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || tryExtension(Extension.Dts);
return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || (useDts ? tryExtension(Extension.Dts) : undefined);
}
case Extensions.JavaScript:
switch (originalExtension) {
Expand Down Expand Up @@ -1813,6 +1833,7 @@ namespace ts {
switch (extensions) {
case Extensions.JavaScript:
case Extensions.Json:
case Extensions.TsOnly:
packageFile = readPackageJsonMainField(jsonContent, candidate, state);
break;
case Extensions.TypeScript:
Expand Down Expand Up @@ -1893,14 +1914,16 @@ namespace ts {
function extensionIsOk(extensions: Extensions, extension: Extension): boolean {
switch (extensions) {
case Extensions.JavaScript:
return extension === Extension.Js || extension === Extension.Jsx;
return extension === Extension.Js || extension === Extension.Jsx || extension === Extension.Mjs || extension === Extension.Cjs;
case Extensions.TSConfig:
case Extensions.Json:
return extension === Extension.Json;
case Extensions.TypeScript:
return extension === Extension.Ts || extension === Extension.Tsx || extension === Extension.Dts;
return extension === Extension.Ts || extension === Extension.Tsx || extension === Extension.Mts || extension === Extension.Cts || extension === Extension.Dts || extension === Extension.Dmts || extension === Extension.Dcts;
case Extensions.TsOnly:
return extension === Extension.Ts || extension === Extension.Tsx || extension === Extension.Mts || extension === Extension.Cts;
case Extensions.DtsOnly:
return extension === Extension.Dts;
return extension === Extension.Dts || extension === Extension.Dmts || extension === Extension.Dcts;
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4669,8 +4669,10 @@ namespace ts {
export type AnyImportOrRequire = AnyImportSyntax | VariableDeclarationInitializedTo<RequireOrImportCall>;

/* @internal */
export type AnyImportOrRequireStatement = AnyImportSyntax | RequireVariableStatement;
export type AnyImportOrBareOrAccessedRequire = AnyImportSyntax | VariableDeclarationInitializedTo<RequireOrImportCall | AccessExpression>;

/* @internal */
export type AnyImportOrRequireStatement = AnyImportSyntax | RequireVariableStatement;

/* @internal */
export type AnyImportOrReExport = AnyImportSyntax | ExportDeclaration;
Expand Down Expand Up @@ -6178,6 +6180,8 @@ namespace ts {
assumeChangesOnlyAffectDirectDependencies?: boolean;
noLib?: boolean;
noResolve?: boolean;
/*@internal*/
noDtsResolution?: boolean;
noUncheckedIndexedAccess?: boolean;
out?: string;
outDir?: string;
Expand Down
37 changes: 26 additions & 11 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,10 @@ namespace ts {
}
}

export function isAnyImportOrBareOrAccessedRequire(node: Node): node is AnyImportOrBareOrAccessedRequire {
return isAnyImportSyntax(node) || isVariableDeclarationInitializedToBareOrAccessedRequire(node);
}

export function isLateVisibilityPaintedStatement(node: Node): node is LateVisibilityPaintedStatement {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
Expand Down Expand Up @@ -2563,14 +2567,14 @@ namespace ts {
return decl.kind === SyntaxKind.FunctionDeclaration || isVariableDeclaration(decl) && decl.initializer && isFunctionLike(decl.initializer);
}

export function tryGetModuleSpecifierFromDeclaration(node: AnyImportOrRequire): string | undefined {
export function tryGetModuleSpecifierFromDeclaration(node: AnyImportOrBareOrAccessedRequire): StringLiteralLike | undefined {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
return node.initializer.arguments[0].text;
return findAncestor(node.initializer, (node): node is RequireOrImportCall => isRequireCall(node, /*requireStringLiteralLikeArgument*/ true))?.arguments[0];
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
case SyntaxKind.ImportDeclaration:
return tryCast(node.moduleSpecifier, isStringLiteralLike)?.text;
return tryCast(node.moduleSpecifier, isStringLiteralLike);
case SyntaxKind.ImportEqualsDeclaration:
return tryCast(tryCast(node.moduleReference, isExternalModuleReference)?.expression, isStringLiteralLike)?.text;
return tryCast(tryCast(node.moduleReference, isExternalModuleReference)?.expression, isStringLiteralLike);
default:
Debug.assertNever(node);
}
Expand Down Expand Up @@ -3133,21 +3137,31 @@ namespace ts {
// export = <EntityNameExpression>
// export default <EntityNameExpression>
// module.exports = <EntityNameExpression>
// {<Identifier>}
// {name: <EntityNameExpression>}
// module.exports.x = <EntityNameExpression>
// const x = require("...")
// const { x } = require("...")
// const x = require("...").y
// const { x } = require("...").y
export function isAliasSymbolDeclaration(node: Node): boolean {
return node.kind === SyntaxKind.ImportEqualsDeclaration ||
if (node.kind === SyntaxKind.ImportEqualsDeclaration ||
node.kind === SyntaxKind.NamespaceExportDeclaration ||
node.kind === SyntaxKind.ImportClause && !!(node as ImportClause).name ||
node.kind === SyntaxKind.NamespaceImport ||
node.kind === SyntaxKind.NamespaceExport ||
node.kind === SyntaxKind.ImportSpecifier ||
node.kind === SyntaxKind.ExportSpecifier ||
node.kind === SyntaxKind.ExportAssignment && exportAssignmentIsAlias(node as ExportAssignment) ||
node.kind === SyntaxKind.ExportAssignment && exportAssignmentIsAlias(node as ExportAssignment)
) {
return true;
}

return isInJSFile(node) && (
isBinaryExpression(node) && getAssignmentDeclarationKind(node) === AssignmentDeclarationKind.ModuleExports && exportAssignmentIsAlias(node) ||
isPropertyAccessExpression(node) && isBinaryExpression(node.parent) && node.parent.left === node && node.parent.operatorToken.kind === SyntaxKind.EqualsToken && isAliasableExpression(node.parent.right) ||
node.kind === SyntaxKind.ShorthandPropertyAssignment ||
node.kind === SyntaxKind.PropertyAssignment && isAliasableExpression((node as PropertyAssignment).initializer);
isPropertyAccessExpression(node)
&& isBinaryExpression(node.parent)
&& node.parent.left === node
&& node.parent.operatorToken.kind === SyntaxKind.EqualsToken
&& isAliasableExpression(node.parent.right));
}

export function getAliasDeclarationFromName(node: EntityName): Declaration | undefined {
Expand All @@ -3158,6 +3172,7 @@ namespace ts {
case SyntaxKind.ExportSpecifier:
case SyntaxKind.ExportAssignment:
case SyntaxKind.ImportEqualsDeclaration:
case SyntaxKind.NamespaceExport:
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
return node.parent as Declaration;
case SyntaxKind.QualifiedName:
do {
Expand Down
22 changes: 21 additions & 1 deletion src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ namespace ts.server {
getDefinitionAndBoundSpan(fileName: string, position: number): DefinitionInfoAndBoundSpan {
const args: protocol.FileLocationRequestArgs = this.createFileLocationRequestArgs(fileName, position);

const request = this.processRequest<protocol.DefinitionRequest>(CommandNames.DefinitionAndBoundSpan, args);
const request = this.processRequest<protocol.DefinitionAndBoundSpanRequest>(CommandNames.DefinitionAndBoundSpan, args);
const response = this.processResponse<protocol.DefinitionInfoAndBoundSpanResponse>(request);
const body = Debug.checkDefined(response.body); // TODO: GH#18217

Expand Down Expand Up @@ -332,6 +332,26 @@ namespace ts.server {
}));
}

getSourceDefinitionAndBoundSpan(fileName: string, position: number): DefinitionInfoAndBoundSpan {
const args: protocol.FileLocationRequestArgs = this.createFileLocationRequestArgs(fileName, position);
const request = this.processRequest<protocol.SourceDefinitionAndBoundSpanRequest>(CommandNames.SourceDefinitionAndBoundSpan, args);
const response = this.processResponse<protocol.DefinitionInfoAndBoundSpanResponse>(request);
const body = Debug.checkDefined(response.body); // TODO: GH#18217

return {
definitions: body.definitions.map(entry => ({
containerKind: ScriptElementKind.unknown,
containerName: "",
fileName: entry.file,
textSpan: this.decodeSpan(entry),
kind: ScriptElementKind.unknown,
name: "",
unverified: entry.unverified,
})),
textSpan: this.decodeSpan(body.textSpan, request.arguments.file)
};
}

getImplementationAtPosition(fileName: string, position: number): ImplementationLocation[] {
const args = this.createFileLocationRequestArgs(fileName, position);

Expand Down
34 changes: 23 additions & 11 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,13 @@ namespace FourSlash {
this.verifyGoToX(arg0, endMarkerNames, () => this.getGoToDefinitionAndBoundSpan());
}

public verifyGoToSourceDefinition(startMarkerNames: ArrayOrSingle<string>, end?: ArrayOrSingle<string | { marker: string, unverified?: boolean }> | { file: string, unverified?: boolean }) {
if (this.testType !== FourSlashTestType.Server) {
this.raiseError("goToSourceDefinition may only be used in fourslash/server tests.");
}
this.verifyGoToX(startMarkerNames, end, () => (this.languageService as ts.server.SessionClient).getSourceDefinitionAndBoundSpan(this.activeFile.fileName, this.currentCaretPosition)!);
}

private getGoToDefinition(): readonly ts.DefinitionInfo[] {
return this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition)!;
}
Expand Down Expand Up @@ -764,7 +771,9 @@ namespace FourSlash {
}

if (endMarkers.length !== definitions.length) {
this.raiseError(`${testName} failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`);
const markers = definitions.map(d => ({ text: "HERE", fileName: d.fileName, position: d.textSpan.start }));
const actual = this.renderMarkers(markers);
this.raiseError(`${testName} failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}\n\n${actual}`);
}

ts.zipWith(endMarkers, definitions, (endMarkerOrFileResult, definition, i) => {
Expand All @@ -774,17 +783,8 @@ namespace FourSlash {
ts.Debug.assert(typeof expectedFileName === "string");
const expectedPosition = marker?.position || 0;
if (ts.comparePaths(expectedFileName, definition.fileName, /*ignoreCase*/ true) !== ts.Comparison.EqualTo || expectedPosition !== definition.textSpan.start) {
const filesToDisplay = ts.deduplicate([expectedFileName, definition.fileName], ts.equateValues);
const markers = [{ text: "EXPECTED", fileName: expectedFileName, position: expectedPosition }, { 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.tryGetFileContent(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");

const text = this.renderMarkers(markers);
this.raiseError(`${testName} failed for definition ${markerName || expectedFileName} (${i}): expected ${expectedFileName} at ${expectedPosition}, got ${definition.fileName} at ${definition.textSpan.start}\n\n${text}\n`);
}
if (definition.unverified && (typeof endMarkerOrFileResult === "string" || !endMarkerOrFileResult.unverified)) {
Expand All @@ -798,6 +798,18 @@ namespace FourSlash {
});
}

private renderMarkers(markers: { text: string, fileName: string, position: number }[]) {
const filesToDisplay = ts.deduplicate(markers.map(m => m.fileName), ts.equateValues);
return filesToDisplay.map(fileName => {
const markersToRender = markers.filter(m => m.fileName === fileName).sort((a, b) => b.position - a.position);
let fileContent = this.tryGetFileContent(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");
}

private verifyDefinitionTextSpan(defs: ts.DefinitionInfoAndBoundSpan, startMarkerName: string) {
const range = this.testData.ranges.find(range => this.markerName(range.marker!) === startMarkerName);

Expand Down
4 changes: 4 additions & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ namespace FourSlashInterface {
this.state.verifyGoToType(arg0, endMarkerName);
}

public goToSourceDefinition(startMarkerNames: ArrayOrSingle<string>, end: { file: string } | ArrayOrSingle<string>) {
this.state.verifyGoToSourceDefinition(startMarkerNames, end);
}

public goToDefinitionForMarkers(...markerNames: string[]) {
this.state.verifyGoToDefinitionForMarkers(markerNames);
}
Expand Down
Loading