Skip to content

Improvements to find-all-references for import types #23998

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

Merged
3 commits merged into from
May 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3252,12 +3252,17 @@ namespace ts {
/* @internal */
export type AnyImportOrReExport = AnyImportSyntax | ExportDeclaration;

/* @internal */
export interface ValidImportTypeNode extends ImportTypeNode {
argument: LiteralTypeNode & { literal: StringLiteral };
}

/* @internal */
export type AnyValidImportOrReExport =
| (ImportDeclaration | ExportDeclaration) & { moduleSpecifier: StringLiteral }
| ImportEqualsDeclaration & { moduleReference: ExternalModuleReference & { expression: StringLiteral } }
| RequireOrImportCall
| ImportTypeNode & { argument: LiteralType };
| ValidImportTypeNode;

/* @internal */
export type RequireOrImportCall = CallExpression & { arguments: [StringLiteralLike] };
Expand Down
9 changes: 7 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,10 @@ namespace ts {
}

export function importFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport {
return tryGetImportFromModuleSpecifier(node) || Debug.fail(Debug.showSyntaxKind(node.parent));
}

export function tryGetImportFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport | undefined {
switch (node.parent.kind) {
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ExportDeclaration:
Expand All @@ -1737,9 +1741,10 @@ namespace ts {
case SyntaxKind.CallExpression:
return node.parent as AnyValidImportOrReExport;
case SyntaxKind.LiteralType:
return cast(node.parent.parent, isImportTypeNode) as ImportTypeNode & { argument: LiteralType };
Debug.assert(isStringLiteral(node));
return tryCast(node.parent.parent, isImportTypeNode) as ValidImportTypeNode | undefined;
default:
return Debug.fail(Debug.showSyntaxKind(node.parent));
return undefined;
}
}

Expand Down
84 changes: 45 additions & 39 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ namespace ts.FindAllReferences.Core {
export function getReferencedSymbolsForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}, sourceFilesSet: ReadonlyMap<true> = arrayToSet(sourceFiles, f => f.fileName)): SymbolAndEntries[] | undefined {
if (isSourceFile(node)) {
const reference = GoToDefinition.getReferenceAtPosition(node, position, program);
return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), sourceFiles, sourceFilesSet);
return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), /*excludeImportTypeOfExportEquals*/ false, sourceFiles, sourceFilesSet);
}

if (!options.implementations) {
Expand All @@ -247,45 +247,45 @@ namespace ts.FindAllReferences.Core {
}

const checker = program.getTypeChecker();
const symbol = checker.getSymbolAtLocation(node);
let symbol = checker.getSymbolAtLocation(node);

// Could not find a symbol e.g. unknown identifier
if (!symbol) {
// String literal might be a property (and thus have a symbol), so do this here rather than in getReferencedSymbolsSpecial.
return !options.implementations && isStringLiteral(node) ? getReferencesForStringLiteral(node, sourceFiles, cancellationToken) : undefined;
}

if (symbol.flags & SymbolFlags.Module && isModuleReferenceLocation(node)) {
return getReferencedSymbolsForModule(program, symbol, sourceFiles, sourceFilesSet);
let moduleReferences: SymbolAndEntries[] = emptyArray;
const moduleSourceFile = isModuleSymbol(symbol);
if (moduleSourceFile) {
const exportEquals = symbol.exports.get(InternalSymbolName.ExportEquals);
// If !!exportEquals, we're about to add references to `import("mod")` anyway, so don't double-count them.
moduleReferences = getReferencedSymbolsForModule(program, symbol, !!exportEquals, sourceFiles, sourceFilesSet);
if (!exportEquals || !sourceFilesSet.has(moduleSourceFile.fileName)) return moduleReferences;
// Continue to get references to 'export ='.
symbol = skipAlias(exportEquals, checker);
node = undefined;
}

return getReferencedSymbolsForSymbol(symbol, node, sourceFiles, sourceFilesSet, checker, cancellationToken, options);
return concatenate(moduleReferences, getReferencedSymbolsForSymbol(symbol, node, sourceFiles, sourceFilesSet, checker, cancellationToken, options));
}

function isModuleReferenceLocation(node: Node): boolean {
if (!isStringLiteralLike(node)) {
return false;
}
switch (node.parent.kind) {
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.ExternalModuleReference:
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ExportDeclaration:
return true;
case SyntaxKind.LiteralType:
return isImportTypeNode(node.parent.parent);
case SyntaxKind.CallExpression:
return isRequireCall(node.parent as CallExpression, /*checkArgumentIsStringLiteralLike*/ false) || isImportCall(node.parent as CallExpression);
default:
return false;
}
function isModuleSymbol(symbol: Symbol): SourceFile | undefined {
return symbol.flags & SymbolFlags.Module && find(symbol.declarations, isSourceFile);
}

function getReferencedSymbolsForModule(program: Program, symbol: Symbol, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>): SymbolAndEntries[] {
function getReferencedSymbolsForModule(program: Program, symbol: Symbol, excludeImportTypeOfExportEquals: boolean, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>): SymbolAndEntries[] {
Debug.assert(!!symbol.valueDeclaration);

const references = findModuleReferences(program, sourceFiles, symbol).map<Entry>(reference => {
const references = mapDefined<ModuleReference, Entry>(findModuleReferences(program, sourceFiles, symbol), reference => {
if (reference.kind === "import") {
const parent = reference.literal.parent;
Copy link
Member

Choose a reason for hiding this comment

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

Uhhhh, isn't reference.literal.parent just reference?

Copy link
Author

Choose a reason for hiding this comment

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

reference isn't a node, it's a ModuleReference.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhhhh, ok

if (isLiteralTypeNode(parent)) {
const importType = cast(parent.parent, isImportTypeNode);
if (excludeImportTypeOfExportEquals && !importType.qualifier) {
return undefined;
}
}
// import("foo") with no qualifier will reference the `export =` of the module, which may be referenced anyway.
return { type: "node", node: reference.literal };
}
else {
Expand All @@ -308,11 +308,12 @@ namespace ts.FindAllReferences.Core {
}
break;
default:
// This may be merged with something.
Debug.fail("Expected a module symbol to be declared by a SourceFile or ModuleDeclaration.");
}
}

return [{ definition: { type: "symbol", symbol }, references }];
return references.length ? [{ definition: { type: "symbol", symbol }, references }] : emptyArray;
}

/** getReferencedSymbols for special node kinds. */
Expand Down Expand Up @@ -345,21 +346,21 @@ namespace ts.FindAllReferences.Core {
}

/** Core find-all-references algorithm for a normal symbol. */
function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
symbol = skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;
function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node | undefined, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
symbol = node && skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;

// Compute the meaning from the location and the symbol it references
const searchMeaning = getIntersectingMeaningFromDeclarations(node, symbol);
const searchMeaning = node ? getIntersectingMeaningFromDeclarations(node, symbol) : SemanticMeaning.All;

const result: SymbolAndEntries[] = [];
const state = new State(sourceFiles, sourceFilesSet, getSpecialSearchKind(node), checker, cancellationToken, searchMeaning, options, result);
const state = new State(sourceFiles, sourceFilesSet, node ? getSpecialSearchKind(node) : SpecialSearchKind.None, checker, cancellationToken, searchMeaning, options, result);

if (node.kind === SyntaxKind.DefaultKeyword) {
if (node && node.kind === SyntaxKind.DefaultKeyword) {
addReference(node, symbol, state);
searchForImportsOfExport(node, symbol, { exportingModuleSymbol: Debug.assertDefined(symbol.parent, "Expected export symbol to have a parent"), exportKind: ExportKind.Default }, state);
}
else {
const search = state.createSearch(node, symbol, /*comingFrom*/ undefined, { allSearchSymbols: populateSearchSymbolSet(symbol, node, checker, options.implementations) });
const search = state.createSearch(node, symbol, /*comingFrom*/ undefined, { allSearchSymbols: node ? populateSearchSymbolSet(symbol, node, checker, options.implementations) : [symbol] });

// Try to get the smallest valid scope that we can limit our search to;
// otherwise we'll need to search globally (i.e. include each file).
Expand Down Expand Up @@ -499,7 +500,7 @@ namespace ts.FindAllReferences.Core {
}

/** @param allSearchSymbols set of additinal symbols for use by `includes`. */
createSearch(location: Node, symbol: Symbol, comingFrom: ImportExport | undefined, searchOptions: { text?: string, allSearchSymbols?: Symbol[] } = {}): Search {
createSearch(location: Node | undefined, symbol: Symbol, comingFrom: ImportExport | undefined, searchOptions: { text?: string, allSearchSymbols?: Symbol[] } = {}): Search {
// Note: if this is an external module symbol, the name doesn't include quotes.
// Note: getLocalSymbolForExportDefault handles `export default class C {}`, but not `export default C` or `export { C as default }`.
// The other two forms seem to be handled downstream (e.g. in `skipPastExportOrImportSpecifier`), so special-casing the first form
Expand All @@ -509,7 +510,7 @@ namespace ts.FindAllReferences.Core {
allSearchSymbols = [symbol],
} = searchOptions;
const escapedText = escapeLeadingUnderscores(text);
const parents = this.options.implementations && getParentSymbolsOfPropertyAccess(location, symbol, this.checker);
const parents = this.options.implementations && location && getParentSymbolsOfPropertyAccess(location, symbol, this.checker);
return { symbol, comingFrom, text, escapedText, parents, allSearchSymbols, includes: sym => contains(allSearchSymbols, sym) };
}

Expand Down Expand Up @@ -559,11 +560,7 @@ namespace ts.FindAllReferences.Core {
if (singleReferences.length) {
const addRef = state.referenceAdder(exportSymbol);
for (const singleRef of singleReferences) {
// At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
if (hasMatchingMeaning(singleRef, state) &&
!(state.options.isForRename && (isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default)) {
addRef(singleRef);
}
if (shouldAddSingleReference(singleRef, state)) addRef(singleRef);
}
}

Expand Down Expand Up @@ -593,6 +590,15 @@ namespace ts.FindAllReferences.Core {
}
}

function shouldAddSingleReference(singleRef: Identifier | StringLiteral, state: State): boolean {
if (!hasMatchingMeaning(singleRef, state)) return false;
if (!state.options.isForRename) return true;
// Don't rename an import type `import("./module-name")` when renaming `name` in `export = name;`
if (!isIdentifier(singleRef)) return false;
// At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
return !((isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default);
}

// Go to the symbol we imported from and find references for it.
function searchForImportedSymbol(symbol: Symbol, state: State): void {
for (const declaration of symbol.declarations) {
Expand Down
25 changes: 12 additions & 13 deletions src/services/importTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
namespace ts.FindAllReferences {
export interface ImportsResult {
/** For every import of the symbol, the location and local symbol for the import. */
importSearches: [Identifier, Symbol][];
importSearches: ReadonlyArray<[Identifier, Symbol]>;
/** For rename imports/exports `{ foo as bar }`, `foo` is not a local, so it may be added as a reference immediately without further searching. */
singleReferences: Identifier[];
singleReferences: ReadonlyArray<Identifier | StringLiteral>;
/** List of source files that may (or may not) use the symbol via a namespace. (For UMD modules this is every file.) */
indirectUsers: ReadonlyArray<SourceFile>;
}
Expand Down Expand Up @@ -33,7 +33,7 @@ namespace ts.FindAllReferences {
interface AmbientModuleDeclaration extends ModuleDeclaration { body?: ModuleBlock; }
type SourceFileLike = SourceFile | AmbientModuleDeclaration;
// Identifier for the case of `const x = require("y")`.
type Importer = AnyImportOrReExport | ImportTypeNode | Identifier;
type Importer = AnyImportOrReExport | ValidImportTypeNode | Identifier;
type ImporterOrCallExpression = Importer | CallExpression;

/** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
Expand Down Expand Up @@ -135,13 +135,7 @@ namespace ts.FindAllReferences {
break;

case SyntaxKind.ImportType:
if (direct.qualifier) {
// `import("foo").x` named import
directImports.push(direct);
}
else {
// TODO: GH#23879
}
directImports.push(direct);
break;

default:
Expand Down Expand Up @@ -205,7 +199,7 @@ namespace ts.FindAllReferences {
*/
function getSearchesFromDirectImports(directImports: Importer[], exportSymbol: Symbol, exportKind: ExportKind, checker: TypeChecker, isForRename: boolean): Pick<ImportsResult, "importSearches" | "singleReferences"> {
const importSearches: [Identifier, Symbol][] = [];
const singleReferences: Identifier[] = [];
const singleReferences: (Identifier | StringLiteral)[] = [];
function addSearch(location: Identifier, symbol: Symbol): void {
importSearches.push([location, symbol]);
}
Expand All @@ -232,8 +226,13 @@ namespace ts.FindAllReferences {
}

if (decl.kind === SyntaxKind.ImportType) {
if (decl.qualifier) { // TODO: GH#23879
singleReferences.push(decl.qualifier.kind === SyntaxKind.Identifier ? decl.qualifier : decl.qualifier.right);
if (decl.qualifier) {
if (isIdentifier(decl.qualifier) && decl.qualifier.escapedText === symbolName(exportSymbol)) {
singleReferences.push(decl.qualifier);
}
}
else if (exportKind === ExportKind.ExportEquals) {
singleReferences.push(decl.argument.literal);
}
return;
}
Expand Down
3 changes: 3 additions & 0 deletions src/services/rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ namespace ts.Rename {
return undefined;
}

// Can't rename a module name.
if (isStringLiteralLike(node) && tryGetImportFromModuleSpecifier(node)) return undefined;

const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
const specifierName = (isImportOrExportSpecifierName(node) || isStringOrNumericLiteral(node) && node.parent.kind === SyntaxKind.ComputedPropertyName)
? stripQuotes(getTextOfIdentifierOrLiteral(node))
Expand Down
4 changes: 4 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ namespace ts {
Debug.assert(isJSDocTemplateTag(node.parent.parent)); // Else would be handled by isDeclarationName
return SemanticMeaning.Type;
}
else if (isLiteralTypeNode(node.parent)) {
// This might be T["name"], which is actually referencing a property and not a type. So allow both meanings.
return SemanticMeaning.Type | SemanticMeaning.Value;
}
else {
return SemanticMeaning.Value;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/cases/fourslash/findAllRefsExportEquals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////type [|{| "isWriteAccess": true, "isDefinition": true |}T|] = number;
////export = [|T|];

// @Filename: /b.ts
////import [|{| "isWriteAccess": true, "isDefinition": true |}T|] = require("[|./a|]");

const [r0, r1, r2, r3] = test.ranges();
const mod = { definition: 'module "/a"', ranges: [r3] };
const a = { definition: "type T = number", ranges: [r0, r1] };
const b = { definition: '(alias) type T = number\nimport T = require("./a")', ranges: [r2] };
verify.referenceGroups([r0, r1], [a, b]);
verify.referenceGroups(r2, [b, a]);
verify.referenceGroups(r3, [mod, a, b]);
3 changes: 0 additions & 3 deletions tests/cases/fourslash/findAllRefsForModuleGlobal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,4 @@
////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}foo|]" {}

verify.noErrors();

const ranges = test.ranges();
const [r0, r1, r2] = ranges;
verify.singleReferenceGroup('module "/node_modules/foo/index"');
13 changes: 0 additions & 13 deletions tests/cases/fourslash/findAllRefsImportTypeOfModule.ts

This file was deleted.

Loading