Skip to content

Commit

Permalink
Enable method signature completion for object literals (#48168)
Browse files Browse the repository at this point in the history
* skeleton of new feature

* working prototype

* refactor print and format code into its own function

* minor changes; don't support overloads

* have two completion entries

* get rid of accessor support

* add snippet support

* add formatting

* add trailing comma

* add sourcedisplay

* support auto-imports via completion details

* add user preference option and fix ordering of entries

* cleanup

* don't return code actions for no import fixes

* make sortText lower priority for snippets

* get rid of flag

* use optional member sort text

* update baselines

* don't collect method symbols if insert text is not supported

* remove comment

* return undefined if type is not function type

* only slice if needed

* use union reduction; more test cases

* WIP: modify sort text system

* Improve new sort text system

* add signature and union type check

* re-add flag

* fix tests

* rename sort text helper

* fix test and code for union case

* add new flag to protocol type

* fix spaces

* CR: minor fixes

* CR: more fixes

* CR: restructure main flow

* minor fix
  • Loading branch information
gabritto committed Mar 30, 2022
1 parent 7ec7d6d commit e25f04a
Show file tree
Hide file tree
Showing 31 changed files with 928 additions and 243 deletions.
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8752,6 +8752,7 @@ namespace ts {
readonly includeAutomaticOptionalChainCompletions?: boolean;
readonly includeCompletionsWithInsertText?: boolean;
readonly includeCompletionsWithClassMemberSnippets?: boolean;
readonly includeCompletionsWithObjectLiteralMethodSnippets?: boolean;
readonly allowIncompleteCompletions?: boolean;
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
Expand Down
24 changes: 12 additions & 12 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,36 +960,36 @@ namespace FourSlash {
expected = typeof expected === "string" ? { name: expected } : expected;

if (actual.insertText !== expected.insertText) {
this.raiseError(`Completion insert text did not match: ${showTextDiff(expected.insertText || "", actual.insertText || "")}`);
this.raiseError(`At entry ${actual.name}: Completion insert text did not match: ${showTextDiff(expected.insertText || "", actual.insertText || "")}`);
}
const convertedReplacementSpan = expected.replacementSpan && ts.createTextSpanFromRange(expected.replacementSpan);
if (convertedReplacementSpan?.length) {
try {
assert.deepEqual(actual.replacementSpan, convertedReplacementSpan);
}
catch {
this.raiseError(`Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`);
this.raiseError(`At entry ${actual.name}: Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`);
}
}

if (expected.kind !== undefined || expected.kindModifiers !== undefined) {
assert.equal(actual.kind, expected.kind, `Expected 'kind' for ${actual.name} to match`);
assert.equal(actual.kindModifiers, expected.kindModifiers || "", `Expected 'kindModifiers' for ${actual.name} to match`);
assert.equal(actual.kind, expected.kind, `At entry ${actual.name}: Expected 'kind' for ${actual.name} to match`);
assert.equal(actual.kindModifiers, expected.kindModifiers || "", `At entry ${actual.name}: Expected 'kindModifiers' for ${actual.name} to match`);
}
if (expected.isFromUncheckedFile !== undefined) {
assert.equal<boolean | undefined>(actual.isFromUncheckedFile, expected.isFromUncheckedFile, "Expected 'isFromUncheckedFile' properties to match");
assert.equal<boolean | undefined>(actual.isFromUncheckedFile, expected.isFromUncheckedFile, `At entry ${actual.name}: Expected 'isFromUncheckedFile' properties to match`);
}
if (expected.isPackageJsonImport !== undefined) {
assert.equal<boolean | undefined>(actual.isPackageJsonImport, expected.isPackageJsonImport, "Expected 'isPackageJsonImport' properties to match");
assert.equal<boolean | undefined>(actual.isPackageJsonImport, expected.isPackageJsonImport, `At entry ${actual.name}: Expected 'isPackageJsonImport' properties to match`);
}

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, `Expected 'sortText' properties to match`);
assert.equal(actual.hasAction, expected.hasAction, `At entry ${actual.name}: Expected 'hasAction' properties to match`);
assert.equal(actual.isRecommended, expected.isRecommended, `At entry ${actual.name}: Expected 'isRecommended' properties to match'`);
assert.equal(actual.isSnippet, expected.isSnippet, `At entry ${actual.name}: Expected 'isSnippet' properties to match`);
assert.equal(actual.source, expected.source, `At entry ${actual.name}: Expected 'source' values to match`);
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, `At entry ${actual.name}: Expected 'sortText' properties to match`);
if (expected.sourceDisplay && actual.sourceDisplay) {
assert.equal(ts.displayPartsToString(actual.sourceDisplay), expected.sourceDisplay, `Expected 'sourceDisplay' properties to match`);
assert.equal(ts.displayPartsToString(actual.sourceDisplay), expected.sourceDisplay, `At entry ${actual.name}: Expected 'sourceDisplay' properties to match`);
}

if (expected.text !== undefined) {
Expand Down
33 changes: 30 additions & 3 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -950,9 +950,36 @@ namespace FourSlashInterface {
}

export namespace Completion {
export import SortText = ts.Completions.SortText;
import SortTextType = ts.Completions.SortText;
export type SortText = SortTextType;
export import CompletionSource = ts.Completions.CompletionSource;

export const SortText = {
// Presets
LocalDeclarationPriority: "10" as SortText,
LocationPriority: "11" as SortText,
OptionalMember: "12" as SortText,
MemberDeclaredBySpreadAssignment: "13" as SortText,
SuggestedClassMembers: "14" as SortText,
GlobalsOrKeywords: "15" as SortText,
AutoImportSuggestions: "16" as SortText,
ClassMemberSnippets: "17" as SortText,
JavascriptIdentifiers: "18" as SortText,

// Transformations
Deprecated(sortText: SortText): SortText {
return "z" + sortText as SortText;
},

ObjectLiteralProperty(presetSortText: SortText, symbolDisplayName: string): SortText {
return `${presetSortText}\0${symbolDisplayName}\0` as SortText;
},

SortBelow(sortText: SortText): SortText {
return sortText + "1" as SortText;
},
};

const functionEntry = (name: string): ExpectedCompletionEntryObject => ({
name,
kind: "function",
Expand All @@ -963,7 +990,7 @@ namespace FourSlashInterface {
name,
kind: "function",
kindModifiers: "deprecated,declare",
sortText: SortText.DeprecatedGlobalsOrKeywords
sortText: "z15" as SortText,
});
const varEntry = (name: string): ExpectedCompletionEntryObject => ({
name,
Expand Down Expand Up @@ -992,7 +1019,7 @@ namespace FourSlashInterface {
name,
kind: "method",
kindModifiers: "deprecated,declare",
sortText: SortText.DeprecatedLocationPriority
sortText: "z11" as SortText,
});
const propertyEntry = (name: string): ExpectedCompletionEntryObject => ({
name,
Expand Down
7 changes: 7 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3406,6 +3406,13 @@ namespace ts.server.protocol {
* `class A { foo }`.
*/
readonly includeCompletionsWithClassMemberSnippets?: boolean;
/**
* If enabled, object literal methods will have a method declaration completion entry in addition
* to the regular completion entry containing just the method name.
* E.g., `const objectLiteral: T = { f| }` could be completed to `const objectLiteral: T = { foo(): void {} }`,
* in addition to `const objectLiteral: T = { foo }`.
*/
readonly includeCompletionsWithObjectLiteralMethodSnippets?: boolean;
readonly allowIncompleteCompletions?: boolean;
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
Expand Down
Loading

0 comments on commit e25f04a

Please sign in to comment.