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

Import statement completions #43149

Merged
merged 39 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
60422cb
WIP
andrewbranch Feb 17, 2021
081f564
WIP
andrewbranch Feb 18, 2021
48bfb22
Get completion details working
andrewbranch Mar 3, 2021
9e456b5
Start unifying eager and lazy auto imports
andrewbranch Mar 4, 2021
89dba08
Fix export=
andrewbranch Mar 4, 2021
3a4e275
Fix completion details for totally misspelled names
andrewbranch Mar 4, 2021
12bb1fe
Almost fixed duplication...
andrewbranch Mar 5, 2021
0ae905c
Fix remaining completion tests
andrewbranch Mar 5, 2021
e9f3ec1
Refactor to support multiple origins for same symbol
andrewbranch Mar 5, 2021
b5b58f8
Make import fixes make slightly more sense
andrewbranch Mar 5, 2021
c7e9bc7
Add cache back in
andrewbranch Mar 8, 2021
393fce5
Set insertText based on import kind
andrewbranch Mar 8, 2021
8999db9
Update API baselines
andrewbranch Mar 8, 2021
6c8457b
Add semicolons, snippet support, and sourceDisplay
andrewbranch Mar 11, 2021
db75d56
Add some tests
andrewbranch Mar 12, 2021
6e6c8ff
Update baselines
andrewbranch Mar 12, 2021
6382ece
Fix pattern ambient modules appearing in auto imports
andrewbranch Mar 12, 2021
409fa90
Fix tests
andrewbranch Mar 13, 2021
2ae85af
Remove commented code
andrewbranch Mar 15, 2021
77b6e3d
Switch to valueDeclaration for getting module source file
andrewbranch Mar 15, 2021
5388b30
Small optimizations
andrewbranch Mar 16, 2021
99ec664
Cache module specifiers / importableness and export map separately
andrewbranch Mar 19, 2021
630f200
Fix and test cache invalidation logic
andrewbranch Mar 21, 2021
f60ac89
Update API baselines
andrewbranch Mar 21, 2021
32fccbb
Add separate user preference for snippet-formatted completions
andrewbranch Mar 21, 2021
eb421c0
Require first character to match when resolving module specifiers
andrewbranch Mar 22, 2021
2506173
Merge branch 'master' into feature/31658
andrewbranch Mar 22, 2021
cdb7efd
Fix AutoImportProvider export map cache invalidation
andrewbranch Mar 22, 2021
9976d3f
Really fix auto import provider export map invalidation
andrewbranch Mar 22, 2021
d1afa7a
Merge branch 'master' into feature/31658
andrewbranch Mar 23, 2021
8550be7
Update test added in master
andrewbranch Mar 26, 2021
66caaf2
Use logical or assignment
andrewbranch Mar 26, 2021
d10aa0c
Simply conditional by reversing
andrewbranch Mar 26, 2021
5e868e3
Merge branch 'master' into feature/31658
andrewbranch Mar 26, 2021
9ea1bd4
When file is deleted need to marked correctly in the project as remov…
sheetalkamat Oct 9, 2020
eb52faa
Simplify hasAddedOrRemovedSymlinks with cherry-picked fix
andrewbranch Mar 26, 2021
f7028f6
Merge branch 'master' into feature/31658
andrewbranch Mar 26, 2021
f5df94b
Ensure replacement range is on one line
andrewbranch Mar 26, 2021
badcd83
Update baselines
andrewbranch Mar 26, 2021
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
19 changes: 13 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3498,7 +3498,10 @@ namespace ts {
const exports = getExportsOfModuleAsArray(moduleSymbol);
const exportEquals = resolveExternalModuleSymbol(moduleSymbol);
if (exportEquals !== moduleSymbol) {
addRange(exports, getPropertiesOfType(getTypeOfSymbol(exportEquals)));
const type = getTypeOfSymbol(exportEquals);
if (shouldTreatPropertiesOfExternalModuleAsExports(type)) {
addRange(exports, getPropertiesOfType(type));
}
}
return exports;
}
Expand All @@ -3522,11 +3525,15 @@ namespace ts {
}

const type = getTypeOfSymbol(exportEquals);
return type.flags & TypeFlags.Primitive ||
getObjectFlags(type) & ObjectFlags.Class ||
isArrayOrTupleLikeType(type)
? undefined
: getPropertyOfType(type, memberName);
return shouldTreatPropertiesOfExternalModuleAsExports(type) ? getPropertyOfType(type, memberName) : undefined;
}

function shouldTreatPropertiesOfExternalModuleAsExports(resolvedExternalModuleType: Type) {
return !(resolvedExternalModuleType.flags & TypeFlags.Primitive ||
getObjectFlags(resolvedExternalModuleType) & ObjectFlags.Class ||
// `isArrayOrTupleLikeType` is too expensive to use in this auto-imports hot path
isArrayType(resolvedExternalModuleType) ||
isTupleType(resolvedExternalModuleType));
}

function getExportsOfSymbol(symbol: Symbol): SymbolTable {
Expand Down
20 changes: 11 additions & 9 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,17 @@ namespace ts.moduleSpecifiers {
: undefined);
}

interface ModulePath {
path: string;
isInNodeModules: boolean;
isRedirect: boolean;
}

/**
* Looks for existing imports that use symlinks to this module.
* Symlinks will be returned first so they are preferred over the real path.
*/
function getAllModulePaths(importingFileName: string, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
const cwd = host.getCurrentDirectory();
function getAllModulePaths(importingFileName: Path, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly ModulePath[] {
const cache = host.getModuleSpecifierCache?.();
const getCanonicalFileName = hostGetCanonicalFileName(host);
if (cache) {
const cached = cache.get(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName));
if (typeof cached === "object") return cached;
}
const allFileNames = new Map<string, { path: string, isRedirect: boolean, isInNodeModules: boolean }>();
let importedFileFromNodeModules = false;
forEachFileNameOfModule(
Expand All @@ -358,7 +356,7 @@ namespace ts.moduleSpecifiers {
// Sort by paths closest to importing file Name directory
const sortedPaths: ModulePath[] = [];
for (
let directory = getDirectoryPath(toPath(importingFileName, cwd, getCanonicalFileName));
let directory = getDirectoryPath(importingFileName);
allFileNames.size !== 0;
) {
const directoryStart = ensureTrailingDirectorySeparator(directory);
Expand All @@ -384,6 +382,10 @@ namespace ts.moduleSpecifiers {
if (remainingPaths.length > 1) remainingPaths.sort(comparePathsByRedirectAndNumberOfDirectorySeparators);
sortedPaths.push(...remainingPaths);
}

if (cache) {
cache.set(importingFileName, toPath(importedFileName, host.getCurrentDirectory(), getCanonicalFileName), sortedPaths);
}
return sortedPaths;
}

Expand Down
18 changes: 18 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8014,6 +8014,7 @@ namespace ts {
readFile?(path: string): string | undefined;
realpath?(path: string): string;
getSymlinkCache?(): SymlinkCache;
getModuleSpecifierCache?(): ModuleSpecifierCache;
getGlobalTypingsCacheLocation?(): string | undefined;
getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;

Expand All @@ -8024,6 +8025,21 @@ namespace ts {
getFileIncludeReasons(): MultiMap<Path, FileIncludeReason>;
}

/* @internal */
export interface ModulePath {
path: string;
isInNodeModules: boolean;
isRedirect: boolean;
}

/* @internal */
export interface ModuleSpecifierCache {
get(fromFileName: Path, toFileName: Path): boolean | readonly ModulePath[] | undefined;
set(fromFileName: Path, toFileName: Path, moduleSpecifiers: boolean | readonly ModulePath[]): void;
clear(): void;
count(): number;
}

// Note: this used to be deprecated in our public API, but is still used internally
/* @internal */
export interface SymbolTracker {
Expand Down Expand Up @@ -8313,6 +8329,8 @@ namespace ts {
readonly disableSuggestions?: boolean;
readonly quotePreference?: "auto" | "double" | "single";
readonly includeCompletionsForModuleExports?: boolean;
readonly includeCompletionsForImportStatements?: boolean;
readonly includeCompletionsWithSnippetText?: boolean;
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly includeCompletionsWithInsertText?: boolean;
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
Expand Down
13 changes: 13 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2412,6 +2412,19 @@ namespace ts {
return decl.kind === SyntaxKind.FunctionDeclaration || isVariableDeclaration(decl) && decl.initializer && isFunctionLike(decl.initializer);
}

export function tryGetModuleSpecifierFromDeclaration(node: AnyImportOrRequire): string | undefined {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
return node.initializer.arguments[0].text;
case SyntaxKind.ImportDeclaration:
return tryCast(node.moduleSpecifier, isStringLiteralLike)?.text;
case SyntaxKind.ImportEqualsDeclaration:
return tryCast(tryCast(node.moduleReference, isExternalModuleReference)?.expression, isStringLiteralLike)?.text;
default:
Debug.assertNever(node);
}
}

export function importFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport {
return tryGetImportFromModuleSpecifier(node) || Debug.failBadSyntaxKind(node.parent);
}
Expand Down
11 changes: 9 additions & 2 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -930,8 +930,12 @@ namespace FourSlash {

assert.equal(actual.hasAction, expected.hasAction, `Expected 'hasAction' properties to match`);
assert.equal(actual.isRecommended, expected.isRecommended, `Expected 'isRecommended' properties to match'`);
assert.equal(actual.isSnippet, expected.isSnippet, `Expected 'isSnippet' properties to match`);
assert.equal(actual.source, expected.source, `Expected 'source' values to match`);
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`));
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, `Expected 'sortText' properties to match`);
if (expected.sourceDisplay && actual.sourceDisplay) {
assert.equal(ts.displayPartsToString(actual.sourceDisplay), expected.sourceDisplay, `Expected 'sourceDisplay' properties to match`);
}

if (expected.text !== undefined) {
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}'`);
Expand All @@ -941,10 +945,13 @@ namespace FourSlash {
// assert.equal(actualDetails.kind, actual.kind);
assert.equal(actualDetails.kindModifiers, actual.kindModifiers, "Expected 'kindModifiers' properties to match");
assert.equal(actualDetails.source && ts.displayPartsToString(actualDetails.source), expected.sourceDisplay, "Expected 'sourceDisplay' property to match 'source' display parts string");
if (!actual.sourceDisplay) {
assert.equal(actualDetails.sourceDisplay && ts.displayPartsToString(actualDetails.sourceDisplay), expected.sourceDisplay, "Expected 'sourceDisplay' property to match 'sourceDisplay' display parts string");
}
assert.deepEqual(actualDetails.tags, expected.tags);
}
else {
assert(expected.documentation === undefined && expected.tags === undefined && expected.sourceDisplay === undefined, "If specifying completion details, should specify 'text'");
assert(expected.documentation === undefined && expected.tags === undefined, "If specifying completion details, should specify 'text'");
}
}

Expand Down
1 change: 1 addition & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,7 @@ namespace FourSlashInterface {
readonly isFromUncheckedFile?: boolean; // If not specified, won't assert about this
readonly kind?: string; // If not specified, won't assert about this
readonly isPackageJsonImport?: boolean; // If not specified, won't assert about this
readonly isSnippet?: boolean;
readonly kindModifiers?: string; // Must be paired with 'kind'
readonly text?: string;
readonly documentation?: string;
Expand Down
25 changes: 13 additions & 12 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2845,7 +2845,7 @@ namespace ts.server {
});
}
if (includePackageJsonAutoImports !== args.preferences.includePackageJsonAutoImports) {
this.invalidateProjectAutoImports(/*packageJsonPath*/ undefined);
this.invalidateProjectPackageJson(/*packageJsonPath*/ undefined);
}
}
if (args.extraFileExtensions) {
Expand Down Expand Up @@ -3933,7 +3933,7 @@ namespace ts.server {
private watchPackageJsonFile(path: Path) {
const watchers = this.packageJsonFilesMap || (this.packageJsonFilesMap = new Map());
if (!watchers.has(path)) {
this.invalidateProjectAutoImports(path);
this.invalidateProjectPackageJson(path);
watchers.set(path, this.watchFactory.watchFile(
path,
(fileName, eventKind) => {
Expand All @@ -3943,11 +3943,11 @@ namespace ts.server {
return Debug.fail();
case FileWatcherEventKind.Changed:
this.packageJsonCache.addOrUpdate(path);
this.invalidateProjectAutoImports(path);
this.invalidateProjectPackageJson(path);
break;
case FileWatcherEventKind.Deleted:
this.packageJsonCache.delete(path);
this.invalidateProjectAutoImports(path);
this.invalidateProjectPackageJson(path);
watchers.get(path)!.close();
watchers.delete(path);
}
Expand Down Expand Up @@ -3975,15 +3975,16 @@ namespace ts.server {
}

/*@internal*/
private invalidateProjectAutoImports(packageJsonPath: Path | undefined) {
if (this.includePackageJsonAutoImports()) {
this.configuredProjects.forEach(invalidate);
this.inferredProjects.forEach(invalidate);
this.externalProjects.forEach(invalidate);
}
private invalidateProjectPackageJson(packageJsonPath: Path | undefined) {
this.configuredProjects.forEach(invalidate);
this.inferredProjects.forEach(invalidate);
this.externalProjects.forEach(invalidate);
function invalidate(project: Project) {
if (!packageJsonPath || project.packageJsonsForAutoImport?.has(packageJsonPath)) {
project.markAutoImportProviderAsDirty();
if (packageJsonPath) {
project.onPackageJsonChange(packageJsonPath);
}
else {
project.onAutoImportProviderSettingsChanged();
}
}
}
Expand Down
Loading