Adressing pr comments and adding test cases

Author: navya9singh

src/compiler/diagnosticMessages.json

@@ -7644,6 +7644,10 @@
         "category": "Message",
         "code": 95186
     },
+    "Add missing comma for an object member completion '{0}'.": {
+        "category": "Message",
+        "code": 95187
+    },
 
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",

src/services/completions.ts

@@ -446,7 +446,7 @@ export enum CompletionSource {
     /** Case completions for switch statements */
     SwitchCases = "SwitchCases/",
     /** Completions for an Object literal expression */
-    ObjectLiteralExpression = "ObjectLiteralExpression/",
+    ObjectLiteralMemberWithComma = "ObjectLiteralMemberWithComma/",
 }
 
 /** @internal */
@@ -1687,9 +1687,11 @@ function createCompletionEntry(
         hasAction = true;
     }
 
-    if ((contextToken && isPropertyAssignment(contextToken.parent) && findNextToken(contextToken, contextToken?.parent, sourceFile)?.kind !== SyntaxKind.CommaToken &&
-        completionKind === CompletionKind.ObjectPropertyDeclaration)) {
-        source = CompletionSource.ObjectLiteralExpression;
+    if (completionKind === CompletionKind.ObjectPropertyDeclaration && contextToken &&
+        findPrecedingToken(contextToken.pos, sourceFile, contextToken)?.kind !== SyntaxKind.CommaToken &&
+        findNextToken(contextToken, contextToken.parent, sourceFile)?.kind !== SyntaxKind.CommaToken &&
+        (findAncestor(contextToken.parent, (node: Node) => isPropertyAssignment(node))?.getLastToken() === contextToken || isMethodDeclaration(contextToken.parent.parent))) {
+        source = CompletionSource.ObjectLiteralMemberWithComma;
         hasAction = true;
     }
 
@@ -2675,7 +2677,7 @@ function getSymbolCompletionFromEntryId(
             entryId.source === CompletionSource.ClassMemberSnippet && symbol.flags & SymbolFlags.ClassMember
             || entryId.source === CompletionSource.ObjectLiteralMethodSnippet && symbol.flags & (SymbolFlags.Property | SymbolFlags.Method)
             || getSourceFromOrigin(origin) === entryId.source
-            || entryId.source === CompletionSource.ObjectLiteralExpression)
+            || entryId.source === CompletionSource.ObjectLiteralMemberWithComma)
             ? { type: "symbol" as const, symbol, location, origin, contextToken, previousToken, isJsxInitializer, isTypeOnlyLocation }
             : undefined;
     }) || { type: "none" };
@@ -2871,10 +2873,10 @@ function getCompletionEntryCodeActionsAndSourceDisplay(
         return { codeActions: [codeAction], sourceDisplay: undefined };
     }
 
-    if (source === CompletionSource.ObjectLiteralExpression && contextToken) {
+    if (source === CompletionSource.ObjectLiteralMemberWithComma && contextToken) {
         const changes = textChanges.ChangeTracker.with(
             { host, formatContext, preferences },
-            tracker=>tracker.insertText(sourceFile, contextToken.end,","));
+            tracker => tracker.insertText(sourceFile, contextToken.end,","));
         if (changes) {
             return {
                 sourceDisplay: undefined,
@@ -4920,6 +4922,11 @@ function tryGetObjectLikeCompletionContainer(contextToken: Node | undefined): Ob
                     return parent;
                 }
                 break;
+            case SyntaxKind.CloseBraceToken: // const x = { } |
+                if (parent.parent && parent.parent.parent && isMethodDeclaration(parent.parent) && isObjectLiteralExpression(parent.parent.parent)) {
+                    return parent.parent.parent;
+                }
+                break;
             case SyntaxKind.AsteriskToken:
                 return isMethodDeclaration(parent) ? tryCast(parent.parent, isObjectLiteralExpression) : undefined;
             case SyntaxKind.AsyncKeyword:
@@ -4928,8 +4935,10 @@ function tryGetObjectLikeCompletionContainer(contextToken: Node | undefined): Ob
                 return (contextToken as Identifier).text === "async" && isShorthandPropertyAssignment(contextToken.parent)
                     ? contextToken.parent.parent : undefined;
             default:
-                if (parent.parent && isObjectLiteralExpression(parent.parent) && contextToken.kind !== SyntaxKind.ColonToken) {
-                    return parent.parent;
+                const ancestorNode = findAncestor(parent, (node: Node) => isPropertyAssignment(node));
+                if (contextToken.kind !== SyntaxKind.ColonToken && ancestorNode && ancestorNode.getLastToken() === contextToken &&
+                    isObjectLiteralExpression(ancestorNode.parent)) {
+                    return ancestorNode.parent;
                 }
         }
     }

src/services/textChanges.ts

@@ -601,11 +601,6 @@ export class ChangeTracker {
         this.replaceNode(sourceFile, oldNode, newNode, { suffix });
     }
 
-    public replacePropertyAssignmentOnSameLine(sourceFile: SourceFile, oldNode: PropertyAssignment, newNode: PropertyAssignment): void {
-        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);
     }

tests/cases/fourslash/completionsObjectLiteralExpressions1.ts

@@ -15,7 +15,7 @@ verify.completions({
     name: "secondary",
     sortText: completion.SortText.OptionalMember,
     hasAction: true,
-    source: completion.CompletionSource.ObjectLiteralExpression,
+    source: completion.CompletionSource.ObjectLiteralMemberWithComma,
   }],
   preferences: {
     allowIncompleteCompletions: true,
@@ -26,7 +26,7 @@ verify.completions({
 verify.applyCodeActionFromCompletion("", {
   name: "secondary",
   description: `Add missing comma for an object member completion 'secondary'.`,
-  source: completion.CompletionSource.ObjectLiteralExpression,
+  source: completion.CompletionSource.ObjectLiteralMemberWithComma,
   newFileContent:
   `interface ColorPalette {
     primary?: string;

tests/cases/fourslash/completionsObjectLiteralExpressions2.ts

@@ -18,7 +18,7 @@ verify.completions({
     name: "secondary",
     sortText: completion.SortText.OptionalMember,
     hasAction: true,
-    source: completion.CompletionSource.ObjectLiteralExpression,
+    source: completion.CompletionSource.ObjectLiteralMemberWithComma,
    }],
   preferences: {
     allowIncompleteCompletions: true,
@@ -29,7 +29,7 @@ verify.completions({
 verify.applyCodeActionFromCompletion("", {
   name: "secondary",
   description: `Add missing comma for an object member completion 'secondary'.`,
-  source: completion.CompletionSource.ObjectLiteralExpression,
+  source: completion.CompletionSource.ObjectLiteralMemberWithComma,
   newFileContent:
 `interface ColorPalette {
   primary?: string;

tests/cases/fourslash/completionsObjectLiteralExpressions3.ts

@@ -0,0 +1,47 @@
+/// <reference path="fourslash.ts" />
+////interface T {
+////     aaa?: string;
+////     foo(): void;
+//// }
+//// const obj: T = {
+////     foo() {
+//        
+////     }
+////     /**/
+//// }
+
+
+
+verify.completions({
+    marker: "",
+    includes: [{
+      name: "aaa",
+      sortText: completion.SortText.OptionalMember,
+      hasAction: true,
+      source: completion.CompletionSource.ObjectLiteralMemberWithComma,
+    }],
+    preferences: {
+      allowIncompleteCompletions: true,
+      includeInsertTextCompletions: true,
+    },
+  });
+  
+  verify.applyCodeActionFromCompletion("", {
+    name: "aaa",
+    description: `Add missing comma for an object member completion 'aaa'.`,
+    source: completion.CompletionSource.ObjectLiteralMemberWithComma,
+    newFileContent:
+    `interface T {
+     aaa?: string;
+     foo(): void;
+ }
+ const obj: T = {
+     foo() {
+     },
+     
+ }`,
+    preferences: {
+      allowIncompleteCompletions: true,
+      includeInsertTextCompletions: true,
+    },
+  });

tests/cases/fourslash/completionsObjectLiteralExpressions4.ts

@@ -0,0 +1,44 @@
+/// <reference path="fourslash.ts" />
+////interface T {
+////     aaa: number;
+////     bbb?: number;
+//// }
+//// const obj: T = {
+////     aaa: 1 * (2 + 3)
+////     /**/
+//// }
+
+
+
+verify.completions({
+    marker: "",
+    includes: [{
+      name: "bbb",
+      sortText: completion.SortText.OptionalMember,
+      hasAction: true,
+      source: completion.CompletionSource.ObjectLiteralMemberWithComma,
+    }],
+    preferences: {
+      allowIncompleteCompletions: true,
+      includeInsertTextCompletions: true,
+    },
+  });
+  
+  verify.applyCodeActionFromCompletion("", {
+    name: "bbb",
+    description: `Add missing comma for an object member completion 'bbb'.`,
+    source: completion.CompletionSource.ObjectLiteralMemberWithComma,
+    newFileContent:
+    `interface T {
+     aaa: number;
+     bbb?: number;
+ }
+ const obj: T = {
+     aaa: 1 * (2 + 3),
+     
+ }`,
+    preferences: {
+      allowIncompleteCompletions: true,
+      includeInsertTextCompletions: true,
+    },
+  });

tests/cases/fourslash/fourslash.ts

@@ -914,7 +914,7 @@ declare namespace completion {
         TypeOnlyAlias = "TypeOnlyAlias/",
         ObjectLiteralMethodSnippet = "ObjectLiteralMethodSnippet/",
         SwitchCases = "SwitchCases/",
-        ObjectLiteralExpression = "ObjectLiteralExpression/",
+        ObjectLiteralMemberWithComma = "ObjectLiteralMemberWithComma/",
     }
     export const globalThisEntry: Entry;
     export const undefinedVarEntry: Entry;