-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Fix(52604): Provide Object member completions without comma; insert a comma #52899
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
Changes from 5 commits
7c72784
fa517aa
6613dca
4f88134
586a2e5
3ab385f
abb5c07
c8c1362
463a435
54456cc
87b09f8
2fff618
b33aef8
731222c
89d178b
790b30c
c477b17
7e861b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ import { | |
| Expression, | ||
| ExpressionWithTypeArguments, | ||
| factory, | ||
| FileTextChanges, | ||
| filter, | ||
| find, | ||
| findAncestor, | ||
|
|
@@ -293,6 +294,7 @@ import { | |
| Program, | ||
| programContainsModules, | ||
| PropertyAccessExpression, | ||
| PropertyAssignment, | ||
| PropertyDeclaration, | ||
| PropertyName, | ||
| PropertySignature, | ||
|
|
@@ -1346,7 +1348,7 @@ function createCompletionEntry( | |
| } | ||
| } | ||
|
|
||
| if (origin?.kind === SymbolOriginInfoKind.TypeOnlyAlias) { | ||
| if ((origin?.kind === SymbolOriginInfoKind.TypeOnlyAlias) || (contextToken && contextToken.kind !== SyntaxKind.OpenBraceToken && completionKind === CompletionKind.ObjectPropertyDeclaration && preferences.includeCompletionsWithInsertText)) { | ||
| hasAction = true; | ||
| } | ||
|
|
||
|
|
@@ -2464,6 +2466,29 @@ function getCompletionEntryCodeActionsAndSourceDisplay( | |
| return { codeActions: [codeAction], sourceDisplay: undefined }; | ||
| } | ||
|
|
||
| if (contextToken && isObjectLiteralExpression(contextToken.parent.parent) && previousToken?.kind !== SyntaxKind.ColonToken) { | ||
| let changes: FileTextChanges[]; | ||
| if (getLineAndCharacterOfPosition(sourceFile, contextToken.getEnd()).line !== getLineAndCharacterOfPosition(sourceFile, position).line) { | ||
| changes = textChanges.ChangeTracker.with( | ||
| { host, formatContext, preferences }, | ||
| tracker=>tracker.replacePropertyAssignment(sourceFile, contextToken.parent as PropertyAssignment ,contextToken.parent as PropertyAssignment)); | ||
| } | ||
| else { | ||
| changes = textChanges.ChangeTracker.with( | ||
| { host, formatContext, preferences }, | ||
| tracker=>tracker.replacePropertyAssignmentOnSameLine(sourceFile, contextToken.parent as PropertyAssignment ,contextToken.parent as PropertyAssignment)); | ||
| } | ||
|
andrewbranch marked this conversation as resolved.
Outdated
|
||
| if (changes) { | ||
| return { | ||
| sourceDisplay: undefined, | ||
| codeActions: [{ | ||
| changes, | ||
| description: diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]), | ||
|
andrewbranch marked this conversation as resolved.
Outdated
|
||
| }], | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (!origin || !(originIsExport(origin) || originIsResolvedExport(origin))) { | ||
| return { codeActions: undefined, sourceDisplay: undefined }; | ||
| } | ||
|
|
@@ -4500,6 +4525,10 @@ function tryGetObjectLikeCompletionContainer(contextToken: Node | undefined): Ob | |
| case SyntaxKind.Identifier: | ||
| return (contextToken as Identifier).text === "async" && isShorthandPropertyAssignment(contextToken.parent) | ||
| ? contextToken.parent.parent : undefined; | ||
| default: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, I've been thinking about the difference between interface A {
a: string;
b: number;
foo(): void;
}
const b = 3;
const a: A = {
foo() {
},
b: b
/**/
}I have a feeling that we might want to be using
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, this is not a new test case, it's the same as this one I previously commented: const k: I = {
["e"]: i
/**/
}The relevant factor seems to be that there is an identifier in the right side of the property assignment.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC, foo | // contextToken: foo, previousToken: foo
foo b| // contextToken: foo, previousToken: bThe idea is in most places, the completions should be the same whether or not you’ve already started typing something at the current location, so
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added test cases to check when the beginning of the expected completion is typed. |
||
| if (parent.parent && isObjectLiteralExpression(parent.parent) && contextToken.kind !== SyntaxKind.ColonToken) { | ||
| return parent.parent; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -592,6 +592,11 @@ export class ChangeTracker { | |
| this.replaceNode(sourceFile, oldNode, newNode, { suffix }); | ||
| } | ||
|
|
||
| public replacePropertyAssignmentOnSameLine(sourceFile: SourceFile, oldNode: PropertyAssignment, newNode: PropertyAssignment): void { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is no longer used. |
||
| const suffix = this.nextCommaToken(sourceFile, oldNode) ? "" : ","; | ||
| this.replaceNode(sourceFile, oldNode, newNode, { suffix }); | ||
| } | ||
|
|
||
| public insertNodeAt(sourceFile: SourceFile, pos: number, newNode: Node, options: InsertNodeOptions = {}): void { | ||
| this.replaceRange(sourceFile, createRange(pos), newNode, options); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /// <reference path="fourslash.ts" /> | ||
| //// interface ColorPalette { | ||
| //// primary?: string; | ||
| //// secondary?: string; | ||
| //// } | ||
|
|
||
| //// let colors: ColorPalette = { | ||
| //// primary: "red" | ||
| //// /**/ | ||
| //// }; | ||
|
|
||
| verify.completions({ | ||
| marker: "", | ||
| includes: [ | ||
| { | ||
| name: "secondary", | ||
| sortText: completion.SortText.OptionalMember, | ||
| hasAction: true, | ||
| }], | ||
| preferences: { | ||
| allowIncompleteCompletions: true, | ||
| includeInsertTextCompletions: true, | ||
| }, | ||
| }); | ||
|
|
||
| verify.applyCodeActionFromCompletion("", { | ||
| name: "secondary", | ||
| description: `Includes imports of types referenced by 'secondary'`, | ||
| newFileContent: | ||
| `interface ColorPalette { | ||
| primary?: string; | ||
| secondary?: string; | ||
| } | ||
| let colors: ColorPalette = { | ||
| primary: "red", | ||
|
|
||
| };`, | ||
| preferences: { | ||
| allowIncompleteCompletions: true, | ||
| includeInsertTextCompletions: true, | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /// <reference path="fourslash.ts" /> | ||
| //// interface ColorPalette { | ||
| //// primary?: string; | ||
| //// secondary?: string; | ||
| //// } | ||
|
|
||
| //// interface I { | ||
| //// color: ColorPalette; | ||
| //// } | ||
|
|
||
| //// const a: I = { | ||
| //// color: {primary: "red" /**/} | ||
| //// } | ||
|
|
||
| verify.completions({ | ||
| marker: "", | ||
| includes: [ | ||
| { | ||
| name: "secondary", | ||
| sortText: completion.SortText.OptionalMember, | ||
| hasAction: true, | ||
| }], | ||
| preferences: { | ||
| allowIncompleteCompletions: true, | ||
| includeInsertTextCompletions: true, | ||
| } | ||
| }); | ||
|
|
||
| verify.applyCodeActionFromCompletion("", { | ||
| name: "secondary", | ||
| description: `Includes imports of types referenced by 'secondary'`, | ||
| newFileContent: | ||
| `interface ColorPalette { | ||
| primary?: string; | ||
| secondary?: string; | ||
| } | ||
| interface I { | ||
| color: ColorPalette; | ||
| } | ||
| const a: I = { | ||
| color: {primary: "red", } | ||
| }`, | ||
| preferences: { | ||
| allowIncompleteCompletions: true, | ||
| includeInsertTextCompletions: true, | ||
| }, | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.