Adressing pr comments

Author: navya9singh

src/compiler/diagnosticMessages.json

@@ -7720,5 +7720,9 @@
     "Compiler option '{0}' cannot be given an empty string.": {
         "category": "Error",
         "code": 18051
+    },
+    "Add missing comma for an object member completion '{0}'.": {
+        "category": "Message",
+        "code": 18052
     }
 }

src/services/completions.ts

@@ -54,7 +54,6 @@ import {
     Expression,
     ExpressionWithTypeArguments,
     factory,
-    FileTextChanges,
     filter,
     find,
     findAncestor,
@@ -294,7 +293,6 @@ import {
     Program,
     programContainsModules,
     PropertyAccessExpression,
-    PropertyAssignment,
     PropertyDeclaration,
     PropertyName,
     PropertySignature,
@@ -360,6 +358,8 @@ import {
     UserPreferences,
     VariableDeclaration,
     walkUpParenthesizedExpressions,
+    isPropertyAssignment,
+    findNextToken,
 } from "./_namespaces/ts";
 import { StringCompletions } from "./_namespaces/ts.Completions";
 
@@ -425,6 +425,8 @@ export enum CompletionSource {
     ObjectLiteralMethodSnippet = "ObjectLiteralMethodSnippet/",
     /** Case completions for switch statements */
     SwitchCases = "SwitchCases/",
+    /** Completions for an Object literal expression */
+    ObjectLiteralExpression = "ObjectLiteralExpression/",
 }
 
 /** @internal */
@@ -1348,7 +1350,13 @@ function createCompletionEntry(
         }
     }
 
-    if ((origin?.kind === SymbolOriginInfoKind.TypeOnlyAlias) || (contextToken && contextToken.kind !== SyntaxKind.OpenBraceToken && completionKind === CompletionKind.ObjectPropertyDeclaration && preferences.includeCompletionsWithInsertText)) {
+    if ((origin?.kind === SymbolOriginInfoKind.TypeOnlyAlias)) {
+        hasAction = true;
+    }
+
+    if ((contextToken && isPropertyAssignment(contextToken.parent) && findNextToken(contextToken, contextToken?.parent, sourceFile)?.kind !== SyntaxKind.CommaToken &&
+        completionKind === CompletionKind.ObjectPropertyDeclaration)) {
+        source = CompletionSource.ObjectLiteralExpression;
         hasAction = true;
     }
 
@@ -2277,7 +2285,8 @@ function getSymbolCompletionFromEntryId(
         return info && info.name === entryId.name && (
             entryId.source === CompletionSource.ClassMemberSnippet && symbol.flags & SymbolFlags.ClassMember
             || entryId.source === CompletionSource.ObjectLiteralMethodSnippet && symbol.flags & (SymbolFlags.Property | SymbolFlags.Method)
-            || getSourceFromOrigin(origin) === entryId.source)
+            || getSourceFromOrigin(origin) === entryId.source
+            || entryId.source === CompletionSource.ObjectLiteralExpression)
             ? { type: "symbol" as const, symbol, location, origin, contextToken, previousToken, isJsxInitializer, isTypeOnlyLocation }
             : undefined;
     }) || { type: "none" };
@@ -2466,24 +2475,16 @@ 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));
-        }
+    if (source === CompletionSource.ObjectLiteralExpression && contextToken) {
+        const changes = textChanges.ChangeTracker.with(
+            { host, formatContext, preferences },
+            tracker=>tracker.insertText(sourceFile, contextToken.end,","));
         if (changes) {
             return {
                 sourceDisplay: undefined,
                 codeActions: [{
                     changes,
-                    description: diagnosticToString([Diagnostics.Includes_imports_of_types_referenced_by_0, name]),
+                    description: diagnosticToString([Diagnostics.Add_missing_comma_for_an_object_member_completion_0, name]),
                 }],
             };
         }

tests/cases/fourslash/completionsObjectLiteralExpressions1.ts

@@ -10,23 +10,24 @@
 //// };
 
 verify.completions({
-    marker: "",
-    includes: [
-        {
-            name: "secondary",
-            sortText: completion.SortText.OptionalMember,
-            hasAction: true,
-        }],
-    preferences: {
-        allowIncompleteCompletions: true,
-        includeInsertTextCompletions: true,
-      },
+  marker: "",
+  includes: [{
+    name: "secondary",
+    sortText: completion.SortText.OptionalMember,
+    hasAction: true,
+    source: completion.CompletionSource.ObjectLiteralExpression,
+  }],
+  preferences: {
+    allowIncompleteCompletions: true,
+    includeInsertTextCompletions: true,
+  },
 });
 
 verify.applyCodeActionFromCompletion("", {
-    name: "secondary",
-    description: `Includes imports of types referenced by 'secondary'`,
-    newFileContent:
+  name: "secondary",
+  description: `Add missing comma for an object member completion 'secondary'.`,
+  source: completion.CompletionSource.ObjectLiteralExpression,
+  newFileContent:
   `interface ColorPalette {
     primary?: string;
     secondary?: string;
@@ -35,8 +36,8 @@ let colors: ColorPalette = {
     primary: "red",
     
 };`,
-    preferences: {
-      allowIncompleteCompletions: true,
-      includeInsertTextCompletions: true,
-    },
-  });
+  preferences: {
+    allowIncompleteCompletions: true,
+    includeInsertTextCompletions: true,
+  },
+});

tests/cases/fourslash/completionsObjectLiteralExpressions2.ts

@@ -14,21 +14,22 @@
 
 verify.completions({
   marker: "",
-  includes: [
-      {
-          name: "secondary",
-          sortText: completion.SortText.OptionalMember,
-          hasAction: true,
-      }],
+  includes: [{
+    name: "secondary",
+    sortText: completion.SortText.OptionalMember,
+    hasAction: true,
+    source: completion.CompletionSource.ObjectLiteralExpression,
+   }],
   preferences: {
-      allowIncompleteCompletions: true,
-      includeInsertTextCompletions: true,
+    allowIncompleteCompletions: true,
+    includeInsertTextCompletions: true,
   }
 });
 
 verify.applyCodeActionFromCompletion("", {
   name: "secondary",
-  description: `Includes imports of types referenced by 'secondary'`,
+  description: `Add missing comma for an object member completion 'secondary'.`,
+  source: completion.CompletionSource.ObjectLiteralExpression,
   newFileContent:
 `interface ColorPalette {
   primary?: string;

tests/cases/fourslash/fourslash.ts

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