Skip to content

go-to-type-definition, and find-all-refs should work for import.meta #44364

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.

Sign up for GitHub

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 7 commits into from
Feb 25, 2022
Merged

go-to-type-definition, and find-all-refs should work for import.meta #44364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e8d59b1
go-to-type-definition, and find-all-refs should work for import.meta
Zzzen Jun 1, 2021
1e99298
search `meta` instead of `import.meta`
Zzzen Jun 2, 2021
89101bb
Merge remote-tracking branch 'origin/main' into meta-prop-misc
Zzzen Feb 23, 2022
a7b9dd6
remove `definition`
Zzzen Feb 23, 2022
6fa2684
fix compilation error
Zzzen Feb 23, 2022
ef09426
update baseline
Zzzen Feb 24, 2022
c0ef5d9
revert definition
Zzzen Feb 25, 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
17 changes: 17 additions & 0 deletions src/services/findAllReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ namespace ts.FindAllReferences {
node.kind === SyntaxKind.ReadonlyKeyword ? isReadonlyTypeOperator : undefined);
}

if (isImportMeta(node.parent) && node.parent.name === node) {
return getAllReferencesForImportMeta(sourceFiles, cancellationToken);
}

if (isStaticModifier(node) && isClassStaticBlockDeclaration(node.parent)) {
return [{ definition: { type: DefinitionKind.Keyword, node }, references: [nodeEntry(node)] }];
}
Expand Down Expand Up @@ -1435,6 +1439,19 @@ namespace ts.FindAllReferences {
}
}

function getAllReferencesForImportMeta(sourceFiles: readonly SourceFile[], cancellationToken: CancellationToken): SymbolAndEntries[] | undefined {
const references = flatMap(sourceFiles, sourceFile => {
cancellationToken.throwIfCancellationRequested();
return mapDefined(getPossibleSymbolReferenceNodes(sourceFile, "meta", sourceFile), node => {
const parent = node.parent;
if (isImportMeta(parent)) {
return nodeEntry(parent);
}
});
});
return references.length ? [{ definition: { type: DefinitionKind.Keyword, node: references[0].node }, references }] : undefined;
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't exactly know if you should be considering anything a "definition" location apart from the global ImportMeta interface.

Even that one is a little weird, because that's what you'd want if you requested "go-to-type", not necessarily "go-to-definition".

Do all responses require a definition? @jessetrinity and @andrewbranch might have more knowledge here.

Copy link
Member

Choose a reason for hiding this comment

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

The types imply that a definition isn’t required. My intuition is that this shouldn’t be 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's copied from getAllReferencesForKeyword, which searches primitive types like string/number/any. They should have no definition either?

function getAllReferencesForKeyword(sourceFiles: readonly SourceFile[], keywordKind: SyntaxKind, cancellationToken: CancellationToken, filter?: (node: Node) => boolean): SymbolAndEntries[] | undefined {
    // ...
    return references.length ? [{ definition: { type: DefinitionKind.Keyword, node: references[0].node }, references }] : undefined;
}

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 not 100% sure what the implications are for removing it—we should check with @mjbvz ad @uniqueiniquity.

Copy link
Member

Choose a reason for hiding this comment

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

Let’s get rid of the definition

Copy link
Contributor Author

@Zzzen Zzzen Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Only include referenced symbols that have a valid definition.

findReferencedSymbols won't work if definition is absent.

TypeScript/src/services/findAllReferences.ts

Lines 207 to 219 in 29c7ae2

export function findReferencedSymbols(program: Program, cancellationToken: CancellationToken, sourceFiles: readonly SourceFile[], sourceFile: SourceFile, position: number): ReferencedSymbol[] | undefined {
const node = getTouchingPropertyName(sourceFile, position);
const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, { use: FindReferencesUse.References });
const checker = program.getTypeChecker();
const symbol = checker.getSymbolAtLocation(node);
return !referencedSymbols || !referencedSymbols.length ? undefined : mapDefined<SymbolAndEntries, ReferencedSymbol>(referencedSymbols, ({ definition, references }) =>
// Only include referenced symbols that have a valid definition.
definition && {
definition: checker.runWithCancellationToken(cancellationToken, checker => definitionToReferencedSymbolDefinitionInfo(definition, checker, node)),
references: references.map(r => toReferenceEntry(r, symbol))
});
}

Copy link
Member

Choose a reason for hiding this comment

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

🤦 I swear all this stuff was just made up on the spot

}

function getAllReferencesForKeyword(sourceFiles: readonly SourceFile[], keywordKind: SyntaxKind, cancellationToken: CancellationToken, filter?: (node: Node) => boolean): SymbolAndEntries[] | undefined {
const references = flatMap(sourceFiles, sourceFile => {
cancellationToken.throwIfCancellationRequested();
Expand Down
4 changes: 4 additions & 0 deletions src/services/goToDefinition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ namespace ts.GoToDefinition {
return undefined;
}

if (isImportMeta(node.parent) && node.parent.name === node) {
return definitionFromType(typeChecker.getTypeAtLocation(node.parent), typeChecker, node.parent);
}

const symbol = getSymbol(node, typeChecker);
if (!symbol) return undefined;

Expand Down
5 changes: 5 additions & 0 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,9 @@ namespace ts {
if (isNamedTupleMember(node.parent) && node.pos === node.parent.pos) {
return node.parent;
}
if (isImportMeta(node.parent) && node.parent.name === node) {
return node.parent;
}
return node;
}

Expand All @@ -1740,6 +1743,8 @@ namespace ts {
case SyntaxKind.SuperKeyword:
case SyntaxKind.NamedTupleMember:
return true;
case SyntaxKind.MetaProperty:
return isImportMeta(node);
default:
return false;
}
Expand Down
61 changes: 61 additions & 0 deletions tests/baselines/reference/findAllRefsImportMeta.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// === /tests/cases/fourslash/foo.ts ===
// /// <reference no-default-lib="true"/>
// /// <reference path='./bar.d.ts' />
// import./*FIND ALL REFS*/[|meta|];
// import.[|meta|];

// === /tests/cases/fourslash/baz.ts ===
// /// <reference no-default-lib="true"/>
// /// <reference path='./bar.d.ts' />
// let x = import
// . // hai :)
// [|meta|];

[
{
"definition": {
"containerKind": "",
"containerName": "",
"fileName": "/tests/cases/fourslash/foo.ts",
"kind": "keyword",
"textSpan": {
"start": 82,
"length": 4
},
"displayParts": [
{
"kind": "keyword"
}
]
},
"references": [
{
"textSpan": {
"start": 82,
"length": 4
},
"fileName": "/tests/cases/fourslash/foo.ts",
"isWriteAccess": false,
"isDefinition": false
},
{
"textSpan": {
"start": 95,
"length": 4
},
"fileName": "/tests/cases/fourslash/foo.ts",
"isWriteAccess": false,
"isDefinition": false
},
{
"textSpan": {
"start": 109,
"length": 4
},
"fileName": "/tests/cases/fourslash/baz.ts",
"isWriteAccess": false,
"isDefinition": false
}
]
}
]
41 changes: 41 additions & 0 deletions tests/baselines/reference/quickInfoImportMeta.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
[
{
"marker": {
"fileName": "/tests/cases/fourslash/foo.ts",
"position": 77,
"name": "1"
},
"quickInfo": {
"kind": "",
"kindModifiers": "",
"textSpan": {
"start": 75,
"length": 6
},
"displayParts": [],
"documentation": []
}
},
{
"marker": {
"fileName": "/tests/cases/fourslash/foo.ts",
"position": 84,
"name": "2"
},
"quickInfo": {
"kind": "interface",
"kindModifiers": "declare",
"textSpan": {
"start": 75,
"length": 11
},
"displayParts": [],
"documentation": [
{
"text": "The type of `import.meta`.\n\nIf you need to declare that a given property exists on `import.meta`,\nthis type may be augmented via interface merging.",
"kind": "text"
}
]
}
}
]
26 changes: 26 additions & 0 deletions tests/cases/fourslash/findAllRefsImportMeta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// @noLib: true

/// <reference path='fourslash.ts'/>

// @module: esnext
// @Filename: foo.ts
/////// <reference no-default-lib="true"/>
/////// <reference path='./bar.d.ts' />
////import./**/meta;
////import.[|meta|];

//@Filename: bar.d.ts
////interface ImportMeta {
////}

// @Filename: baz.ts
/////// <reference no-default-lib="true"/>
/////// <reference path='./bar.d.ts' />
////let x = import
//// . // hai :)
//// meta;

verify.baselineFindAllReferences("");

goTo.rangeStart(test.ranges()[0]);
verify.renameInfoFailed();
13 changes: 13 additions & 0 deletions tests/cases/fourslash/goToDefinitionImportMeta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

// @module: esnext
// @Filename: foo.ts
/////// <reference no-default-lib="true"/>
/////// <reference path='./bar.d.ts' />
////import.me/*reference*/ta;

//@Filename: bar.d.ts
////interface ImportMeta {
////}

verify.goToDefinition("reference", []);
13 changes: 13 additions & 0 deletions tests/cases/fourslash/goToTypeDefinitionImportMeta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

// @module: esnext
// @Filename: foo.ts
/////// <reference no-default-lib="true"/>
/////// <reference path='./bar.d.ts' />
////import.me/*reference*/ta;

//@Filename: bar.d.ts
////interface /*definition*/ImportMeta {
////}

verify.goToType("reference", "definition");
19 changes: 19 additions & 0 deletions tests/cases/fourslash/quickInfoImportMeta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
///<reference path="fourslash.ts" />

// @module: esnext
// @Filename: foo.ts
/////// <reference no-default-lib="true"/>
/////// <reference path='./bar.d.ts' />
////im/*1*/port.me/*2*/ta;

//@Filename: bar.d.ts
/////**
//// * The type of `import.meta`.
//// *
//// * If you need to declare that a given property exists on `import.meta`,
//// * this type may be augmented via interface merging.
//// */
//// interface ImportMeta {
////}

verify.baselineQuickInfo()