fix modifier inheritance for overloads

src/compiler/checker.ts

@@ -38517,7 +38517,7 @@ namespace ts {
             baseWithThis: Type | undefined,
             type: InterfaceType,
             typeWithThis: Type,
-            memberHasOverrideModifier: boolean, // >> Note: we need this because this is computed from a node, but we don't have any (it would be synthetic)
+            memberHasOverrideModifier: boolean,
             memberHasAbstractModifier: boolean,
             memberIsStatic: boolean,
             memberIsParameterProperty: boolean,

src/services/completions.ts

@@ -795,7 +795,7 @@ namespace ts.Completions {
         and `location.parent.parent` is a class-like declaration.
         In
         `abstract class C {
-            abstract 
+            abstract
             abstract m|
         }`
         `location` is a syntax list (with modifiers as children),
@@ -803,8 +803,8 @@ namespace ts.Completions {
         */
         return !!(symbol.flags & memberFlags) &&
             (isClassLike(location) ||
-                (isClassElement(location.parent) && isClassLike(location.parent.parent)) ||
-                (isSyntaxList(location) && isClassLike(location.parent)));
+                (location.parent && location.parent.parent && isClassElement(location.parent) && isClassLike(location.parent.parent)) ||
+                (location.parent && isSyntaxList(location) && isClassLike(location.parent)));
     }
 
     function getEntryForMemberCompletion(
@@ -849,6 +849,7 @@ namespace ts.Completions {
             body = factory.createBlock([], /* multiline */ true);
         }
 
+        let modifiers = ModifierFlags.None;
         const completionNodes: Node[] = [];
         codefix.addNewNodeForMemberSymbol(
             symbol,
@@ -896,12 +897,18 @@ namespace ts.Completions {
                 }
 
                 let presentModifiers = ModifierFlags.None;
-                // Omit already present modifiers from the first completion node.
-                if (!completionNodes.length && contextToken) {
-                    presentModifiers = getPresentModifiers(contextToken);
+                if (!completionNodes.length) {
+                    // Omit already present modifiers from the first completion node/signature.
+                    if (contextToken) {
+                        presentModifiers = getPresentModifiers(contextToken);
+                    }
+                    // Keep track of added missing required modifiers and modifiers already present.
+                    // This is needed when we have overloaded signatures,
+                    // so this callback will be called for multiple nodes/signatures,
+                    // and we need to make sure the modifiers are uniform for all nodes/signatures.
+                    modifiers = node.modifierFlagsCache | requiredModifiers | presentModifiers;
                 }
-                // Update modifiers to add missing required ones, and remove modifiers already present.
-                node = factory.updateModifiers(node, (node.modifierFlagsCache | requiredModifiers) & (~presentModifiers));
+                node = factory.updateModifiers(node, modifiers & (~presentModifiers));
 
                 completionNodes.push(node);
             },
@@ -944,7 +951,6 @@ namespace ts.Completions {
         }
         if (isPropertyDeclaration(contextToken.parent)) {
             modifiers |= modifiersToFlags(contextToken.parent.modifiers);
-            // >> TODO: does this work? is the node going to have modifiers already or should we call `.getChildren()`?
         }
         return modifiers;
     }

tests/cases/fourslash/completionsOverridingMethod6.ts

@@ -1,7 +1,7 @@
 /// <reference path="fourslash.ts" />
 
 // @Filename: a.ts
-// Case: modifier inheritance/duplication
+// Case: modifier inheritance/deduplication
 ////class A {
 ////    public method(): number {
 ////        return 0;
@@ -15,6 +15,16 @@
 ////class C extends A {
 ////    public override m/*a*/
 ////}
+////
+////interface D {
+////    fun(a: number): number;
+////    fun(a: undefined, b: string): number;
+////}
+////
+////class E implements D {
+////    public f/*c*/
+////}
+
 
 // format.setFormatOptions({
 //     newLineCharacter: "\n",
@@ -61,4 +71,28 @@ verify.completions({
             insertText: "method(): number;\r\n",
         },
     ],
+});
+
+verify.completions({
+    marker: "c",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: false,
+    },
+    includes: [
+        {
+            name: "fun",
+            sortText: completion.SortText.LocationPriority,
+            replacementSpan: {
+                fileName: "",
+                pos: 0,
+                end: 0,
+            },
+            insertText:
+                "fun(a: number): number;\r\n\
+public fun(a: undefined, b: string): number;\r\n\
+public fun(a: any, b?: any): number {\r\n}\r\n",
+        },
+    ],
 });