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 18 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
17 changes: 11 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3493,7 +3493,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 @@ -3517,11 +3520,13 @@ 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(resolvedExternalModuleType));
}

function getExportsOfSymbol(symbol: Symbol): SymbolTable {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8253,6 +8253,7 @@ namespace ts {
readonly disableSuggestions?: boolean;
readonly quotePreference?: "auto" | "double" | "single";
readonly includeCompletionsForModuleExports?: boolean;
readonly includeCompletionsForImportStatements?: 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 @@ -2401,6 +2401,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 @@ -1591,6 +1591,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
6 changes: 3 additions & 3 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ namespace ts.server {
public readonly getCanonicalFileName: GetCanonicalFileName;

/*@internal*/
private importSuggestionsCache = Completions.createImportSuggestionsForFileCache();
private importSuggestionsCache = codefix.createImportSuggestionsForFileCache();
/*@internal*/
private dirtyFilesForSuggestions: Set<Path> | undefined;
/*@internal*/
Expand Down Expand Up @@ -1168,8 +1168,8 @@ namespace ts.server {
}
else if (this.dirtyFilesForSuggestions && oldProgram && this.program) {
forEachKey(this.dirtyFilesForSuggestions, fileName => {
const oldSourceFile = oldProgram.getSourceFile(fileName);
const sourceFile = this.program!.getSourceFile(fileName);
const oldSourceFile = oldProgram.getSourceFileByPath(fileName);
const sourceFile = this.program!.getSourceFileByPath(fileName);
if (this.sourceFileHasChangedOwnImportSuggestions(oldSourceFile, sourceFile)) {
this.importSuggestionsCache.clear();
return true;
Expand Down
23 changes: 21 additions & 2 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2127,7 +2127,7 @@ namespace ts.server.protocol {
arguments: FormatOnKeyRequestArgs;
}

export type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#";
export type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#" | " ";
Copy link
Member Author

Choose a reason for hiding this comment

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

The space, in combination with the isIncomplete property added, is a targeted fix for the problem described at #31658 (comment).


/**
* Arguments for completions messages.
Expand Down Expand Up @@ -2232,6 +2232,10 @@ namespace ts.server.protocol {
* coupled with `replacementSpan` to replace a dotted access with a bracket access.
*/
insertText?: string;
/**
* `insertText` should be interpreted as a snippet if true.
*/
isSnippet?: true;
/**
* An optional span that indicates the text to be replaced by this completion item.
* If present, this span should be used instead of the default one.
Expand All @@ -2247,6 +2251,10 @@ namespace ts.server.protocol {
* Identifier (not necessarily human-readable) identifying where this completion came from.
*/
source?: string;
/**
* Human-readable description of the `source`.
*/
sourceDisplay?: SymbolDisplayPart[];
/**
* If true, this completion should be highlighted as recommended. There will only be one of these.
* This will be set when we know the user should write an expression with a certain type and that type is an enum or constructable class.
Expand Down Expand Up @@ -2308,9 +2316,14 @@ namespace ts.server.protocol {
codeActions?: CodeAction[];

/**
* Human-readable description of the `source` from the CompletionEntry.
* @deprecated Use `sourceDisplay` instead.
*/
source?: SymbolDisplayPart[];

/**
* Human-readable description of the `source` from the CompletionEntry.
*/
sourceDisplay?: SymbolDisplayPart[];
Comment on lines -2332 to +2347
Copy link
Member Author

Choose a reason for hiding this comment

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

Aiming to stop overloading the meaning of source between CompletionEntry and CompletionEntryDetails. In an ideal world, source is the machine-readable module name for the auto-import, and sourceDisplay is the human-readable module specifier for it, no matter whether you’re looking at CompletionEntry or CompletionEntryDetails. In reality, we will continue to set CompletionEntryDetails["source"] to the human-readable module specifier for backward compatibility, but I’ve marked it as deprecated.

Copy link
Member

Choose a reason for hiding this comment

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

The editor only pulls on these one at a time right?

}

/** @deprecated Prefer CompletionInfoResponse, which supports several top-level fields in addition to the array of entries. */
Expand All @@ -2332,6 +2345,7 @@ namespace ts.server.protocol {
* must be used to commit that completion entry.
*/
readonly optionalReplacementSpan?: TextSpan;
readonly isIncomplete?: boolean;
readonly entries: readonly CompletionEntry[];
}

Expand Down Expand Up @@ -3277,6 +3291,11 @@ namespace ts.server.protocol {
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
*/
readonly includeCompletionsForModuleExports?: boolean;
/**
* Enables auto-import-style completions on partially-typed import statements. E.g., allows
* `import write|` to be completed to `import { writeFile } from "fs"`.
*/
readonly includeCompletionsForImportStatements?: boolean;
/**
* If enabled, the completion list will include completions with invalid identifier names.
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
Expand Down
4 changes: 2 additions & 2 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1810,10 +1810,10 @@ namespace ts.server {
const prefix = args.prefix || "";
const entries = stableSort(mapDefined<CompletionEntry, protocol.CompletionEntry>(completions.entries, entry => {
if (completions.isMemberCompletion || startsWith(entry.name.toLowerCase(), prefix.toLowerCase())) {
const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, isRecommended, isPackageJsonImport, data } = entry;
const { name, kind, kindModifiers, sortText, insertText, replacementSpan, hasAction, source, sourceDisplay, isSnippet, isRecommended, isPackageJsonImport, data } = entry;
const convertedSpan = replacementSpan ? toProtocolTextSpan(replacementSpan, scriptInfo) : undefined;
// Use `hasAction || undefined` to avoid serializing `false`.
return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended, isPackageJsonImport, data };
return { name, kind, kindModifiers, sortText, insertText, replacementSpan: convertedSpan, isSnippet, hasAction: hasAction || undefined, source, sourceDisplay, isRecommended, isPackageJsonImport, data };
}
}), (a, b) => compareStringsCaseSensitiveUI(a.name, b.name));

Expand Down
Loading