Skip to content

Commit 2d6a490

Browse files
andrewbranchDanielRosenwassersheetalkamat
authored
Import statement completions (#43149)
* WIP * WIP * Get completion details working * Start unifying eager and lazy auto imports * Fix export= * Fix completion details for totally misspelled names * Almost fixed duplication... * Fix remaining completion tests * Refactor to support multiple origins for same symbol * Make import fixes make slightly more sense * Add cache back in * Set insertText based on import kind * Update API baselines * Add semicolons, snippet support, and sourceDisplay * Add some tests * Update baselines * Fix pattern ambient modules appearing in auto imports * Fix tests * Remove commented code * Switch to valueDeclaration for getting module source file * Small optimizations * Cache module specifiers / importableness and export map separately * Fix and test cache invalidation logic * Update API baselines * Add separate user preference for snippet-formatted completions * Require first character to match when resolving module specifiers * Fix AutoImportProvider export map cache invalidation * Really fix auto import provider export map invalidation * Update test added in master * Use logical or assignment Co-authored-by: Daniel Rosenwasser <[email protected]> * Simply conditional by reversing Co-authored-by: Daniel Rosenwasser <[email protected]> * When file is deleted need to marked correctly in the project as removed file * Simplify hasAddedOrRemovedSymlinks with cherry-picked fix * Ensure replacement range is on one line * Update baselines Co-authored-by: Daniel Rosenwasser <[email protected]> Co-authored-by: Sheetal Nandi <[email protected]>
1 parent a545ab1 commit 2d6a490

38 files changed

+1577
-845
lines changed

src/compiler/checker.ts

+13-6
Original file line numberDiff line numberDiff line change
@@ -3498,7 +3498,10 @@ namespace ts {
34983498
const exports = getExportsOfModuleAsArray(moduleSymbol);
34993499
const exportEquals = resolveExternalModuleSymbol(moduleSymbol);
35003500
if (exportEquals !== moduleSymbol) {
3501-
addRange(exports, getPropertiesOfType(getTypeOfSymbol(exportEquals)));
3501+
const type = getTypeOfSymbol(exportEquals);
3502+
if (shouldTreatPropertiesOfExternalModuleAsExports(type)) {
3503+
addRange(exports, getPropertiesOfType(type));
3504+
}
35023505
}
35033506
return exports;
35043507
}
@@ -3522,11 +3525,15 @@ namespace ts {
35223525
}
35233526

35243527
const type = getTypeOfSymbol(exportEquals);
3525-
return type.flags & TypeFlags.Primitive ||
3526-
getObjectFlags(type) & ObjectFlags.Class ||
3527-
isArrayOrTupleLikeType(type)
3528-
? undefined
3529-
: getPropertyOfType(type, memberName);
3528+
return shouldTreatPropertiesOfExternalModuleAsExports(type) ? getPropertyOfType(type, memberName) : undefined;
3529+
}
3530+
3531+
function shouldTreatPropertiesOfExternalModuleAsExports(resolvedExternalModuleType: Type) {
3532+
return !(resolvedExternalModuleType.flags & TypeFlags.Primitive ||
3533+
getObjectFlags(resolvedExternalModuleType) & ObjectFlags.Class ||
3534+
// `isArrayOrTupleLikeType` is too expensive to use in this auto-imports hot path
3535+
isArrayType(resolvedExternalModuleType) ||
3536+
isTupleType(resolvedExternalModuleType));
35303537
}
35313538

35323539
function getExportsOfSymbol(symbol: Symbol): SymbolTable {

src/compiler/moduleSpecifiers.ts

+11-9
Original file line numberDiff line numberDiff line change
@@ -327,19 +327,17 @@ namespace ts.moduleSpecifiers {
327327
: undefined);
328328
}
329329

330-
interface ModulePath {
331-
path: string;
332-
isInNodeModules: boolean;
333-
isRedirect: boolean;
334-
}
335-
336330
/**
337331
* Looks for existing imports that use symlinks to this module.
338332
* Symlinks will be returned first so they are preferred over the real path.
339333
*/
340-
function getAllModulePaths(importingFileName: string, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
341-
const cwd = host.getCurrentDirectory();
334+
function getAllModulePaths(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
335+
const cache = host.getModuleSpecifierCache?.();
342336
const getCanonicalFileName = hostGetCanonicalFileName(host);
337+
if (cache) {
338+
const cached = cache.get(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName));
339+
if (typeof cached === "object") return cached;
340+
}
343341
const allFileNames = new Map<string, { path: string, isRedirect: boolean, isInNodeModules: boolean }>();
344342
let importedFileFromNodeModules = false;
345343
forEachFileNameOfModule(
@@ -358,7 +356,7 @@ namespace ts.moduleSpecifiers {
358356
// Sort by paths closest to importing file Name directory
359357
const sortedPaths: ModulePath[] = [];
360358
for (
361-
let directory = getDirectoryPath(toPath(importingFileName, cwd, getCanonicalFileName));
359+
let directory = getDirectoryPath(importingFileName);
362360
allFileNames.size !== 0;
363361
) {
364362
const directoryStart = ensureTrailingDirectorySeparator(directory);
@@ -384,6 +382,10 @@ namespace ts.moduleSpecifiers {
384382
if (remainingPaths.length > 1) remainingPaths.sort(comparePathsByRedirectAndNumberOfDirectorySeparators);
385383
sortedPaths.push(...remainingPaths);
386384
}
385+
386+
if (cache) {
387+
cache.set(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName), sortedPaths);
388+
}
387389
return sortedPaths;
388390
}
389391

src/compiler/types.ts

+18
Original file line numberDiff line numberDiff line change
@@ -8015,6 +8015,7 @@ namespace ts {
80158015
readFile?(path: string): string | undefined;
80168016
realpath?(path: string): string;
80178017
getSymlinkCache?(): SymlinkCache;
8018+
getModuleSpecifierCache?(): ModuleSpecifierCache;
80188019
getGlobalTypingsCacheLocation?(): string | undefined;
80198020
getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;
80208021

@@ -8025,6 +8026,21 @@ namespace ts {
80258026
getFileIncludeReasons(): MultiMap<Path, FileIncludeReason>;
80268027
}
80278028

8029+
/* @internal */
8030+
export interface ModulePath {
8031+
path: string;
8032+
isInNodeModules: boolean;
8033+
isRedirect: boolean;
8034+
}
8035+
8036+
/* @internal */
8037+
export interface ModuleSpecifierCache {
8038+
get(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined;
8039+
set(fromFileName: Path, toFileName: Path, moduleSpecifiers: boolean | readonly ModulePath[]): void;
8040+
clear(): void;
8041+
count(): number;
8042+
}
8043+
80288044
// Note: this used to be deprecated in our public API, but is still used internally
80298045
/* @internal */
80308046
export interface SymbolTracker {
@@ -8314,6 +8330,8 @@ namespace ts {
83148330
readonly disableSuggestions?: boolean;
83158331
readonly quotePreference?: "auto" | "double" | "single";
83168332
readonly includeCompletionsForModuleExports?: boolean;
8333+
readonly includeCompletionsForImportStatements?: boolean;
8334+
readonly includeCompletionsWithSnippetText?: boolean;
83178335
readonly includeAutomaticOptionalChainCompletions?: boolean;
83188336
readonly includeCompletionsWithInsertText?: boolean;
83198337
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";

src/compiler/utilities.ts

+13
Original file line numberDiff line numberDiff line change
@@ -2412,6 +2412,19 @@ namespace ts {
24122412
return decl.kind === SyntaxKind.FunctionDeclaration || isVariableDeclaration(decl) && decl.initializer && isFunctionLike(decl.initializer);
24132413
}
24142414

2415+
export function tryGetModuleSpecifierFromDeclaration(node: AnyImportOrRequire): string | undefined {
2416+
switch (node.kind) {
2417+
case SyntaxKind.VariableDeclaration:
2418+
return node.initializer.arguments[0].text;
2419+
case SyntaxKind.ImportDeclaration:
2420+
return tryCast(node.moduleSpecifier, isStringLiteralLike)?.text;
2421+
case SyntaxKind.ImportEqualsDeclaration:
2422+
return tryCast(tryCast(node.moduleReference, isExternalModuleReference)?.expression, isStringLiteralLike)?.text;
2423+
default:
2424+
Debug.assertNever(node);
2425+
}
2426+
}
2427+
24152428
export function importFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport {
24162429
return tryGetImportFromModuleSpecifier(node) || Debug.failBadSyntaxKind(node.parent);
24172430
}

src/harness/fourslashImpl.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -930,8 +930,12 @@ namespace FourSlash {
930930

931931
assert.equal(actual.hasAction, expected.hasAction, `Expected 'hasAction' properties to match`);
932932
assert.equal(actual.isRecommended, expected.isRecommended, `Expected 'isRecommended' properties to match'`);
933+
assert.equal(actual.isSnippet, expected.isSnippet, `Expected 'isSnippet' properties to match`);
933934
assert.equal(actual.source, expected.source, `Expected 'source' values to match`);
934-
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`));
935+
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, `Expected 'sortText' properties to match`);
936+
if (expected.sourceDisplay && actual.sourceDisplay) {
937+
assert.equal(ts.displayPartsToString(actual.sourceDisplay), expected.sourceDisplay, `Expected 'sourceDisplay' properties to match`);
938+
}
935939

936940
if (expected.text !== undefined) {
937941
const actualDetails = ts.Debug.checkDefined(this.getCompletionEntryDetails(actual.name, actual.source, actual.data), `No completion details available for name '${actual.name}' and source '${actual.source}'`);
@@ -941,10 +945,13 @@ namespace FourSlash {
941945
// assert.equal(actualDetails.kind, actual.kind);
942946
assert.equal(actualDetails.kindModifiers, actual.kindModifiers, "Expected 'kindModifiers' properties to match");
943947
assert.equal(actualDetails.source && ts.displayPartsToString(actualDetails.source), expected.sourceDisplay, "Expected 'sourceDisplay' property to match 'source' display parts string");
948+
if (!actual.sourceDisplay) {
949+
assert.equal(actualDetails.sourceDisplay && ts.displayPartsToString(actualDetails.sourceDisplay), expected.sourceDisplay, "Expected 'sourceDisplay' property to match 'sourceDisplay' display parts string");
950+
}
944951
assert.deepEqual(actualDetails.tags, expected.tags);
945952
}
946953
else {
947-
assert(expected.documentation === undefined && expected.tags === undefined && expected.sourceDisplay === undefined, "If specifying completion details, should specify 'text'");
954+
assert(expected.documentation === undefined && expected.tags === undefined, "If specifying completion details, should specify 'text'");
948955
}
949956
}
950957

src/harness/fourslashInterfaceImpl.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1599,6 +1599,7 @@ namespace FourSlashInterface {
15991599
readonly isFromUncheckedFile?: boolean; // If not specified, won't assert about this
16001600
readonly kind?: string; // If not specified, won't assert about this
16011601
readonly isPackageJsonImport?: boolean; // If not specified, won't assert about this
1602+
readonly isSnippet?: boolean;
16021603
readonly kindModifiers?: string; // Must be paired with 'kind'
16031604
readonly text?: string;
16041605
readonly documentation?: string;

src/server/editorServices.ts

+13-12
Original file line numberDiff line numberDiff line change
@@ -2958,7 +2958,7 @@ namespace ts.server {
29582958
});
29592959
}
29602960
if (includePackageJsonAutoImports !== args.preferences.includePackageJsonAutoImports) {
2961-
this.invalidateProjectAutoImports(/*packageJsonPath*/ undefined);
2961+
this.invalidateProjectPackageJson(/*packageJsonPath*/ undefined);
29622962
}
29632963
}
29642964
if (args.extraFileExtensions) {
@@ -4041,7 +4041,7 @@ namespace ts.server {
40414041
private watchPackageJsonFile(path: Path) {
40424042
const watchers = this.packageJsonFilesMap || (this.packageJsonFilesMap = new Map());
40434043
if (!watchers.has(path)) {
4044-
this.invalidateProjectAutoImports(path);
4044+
this.invalidateProjectPackageJson(path);
40454045
watchers.set(path, this.watchFactory.watchFile(
40464046
path,
40474047
(fileName, eventKind) => {
@@ -4051,11 +4051,11 @@ namespace ts.server {
40514051
return Debug.fail();
40524052
case FileWatcherEventKind.Changed:
40534053
this.packageJsonCache.addOrUpdate(path);
4054-
this.invalidateProjectAutoImports(path);
4054+
this.invalidateProjectPackageJson(path);
40554055
break;
40564056
case FileWatcherEventKind.Deleted:
40574057
this.packageJsonCache.delete(path);
4058-
this.invalidateProjectAutoImports(path);
4058+
this.invalidateProjectPackageJson(path);
40594059
watchers.get(path)!.close();
40604060
watchers.delete(path);
40614061
}
@@ -4083,15 +4083,16 @@ namespace ts.server {
40834083
}
40844084

40854085
/*@internal*/
4086-
private invalidateProjectAutoImports(packageJsonPath: Path | undefined) {
4087-
if (this.includePackageJsonAutoImports()) {
4088-
this.configuredProjects.forEach(invalidate);
4089-
this.inferredProjects.forEach(invalidate);
4090-
this.externalProjects.forEach(invalidate);
4091-
}
4086+
private invalidateProjectPackageJson(packageJsonPath: Path | undefined) {
4087+
this.configuredProjects.forEach(invalidate);
4088+
this.inferredProjects.forEach(invalidate);
4089+
this.externalProjects.forEach(invalidate);
40924090
function invalidate(project: Project) {
4093-
if (!packageJsonPath || project.packageJsonsForAutoImport?.has(packageJsonPath)) {
4094-
project.markAutoImportProviderAsDirty();
4091+
if (packageJsonPath) {
4092+
project.onPackageJsonChange(packageJsonPath);
4093+
}
4094+
else {
4095+
project.onAutoImportProviderSettingsChanged();
40954096
}
40964097
}
40974098
}

0 commit comments

Comments
 (0)