expose check override modifier in checker

src/compiler/checker.ts

@@ -720,6 +720,7 @@ namespace ts {
             getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
             isDeclarationVisible,
             isPropertyAccessible,
+            getMemberOverrideModifierDiagnostic,
         };
 
         function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
@@ -37781,7 +37782,7 @@ namespace ts {
                 }
             }
 
-            checkMembersForMissingOverrideModifier(node, type, typeWithThis, staticType);
+            checkMembersForOverrideModifier(node, type, typeWithThis, staticType);
 
             const implementedTypeNodes = getEffectiveImplementsTypeNodes(node);
             if (implementedTypeNodes) {
@@ -37818,8 +37819,7 @@ namespace ts {
             }
         }
 
-        function checkMembersForMissingOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type, staticType: ObjectType) {
-            const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
+        function checkMembersForOverrideModifier(node: ClassLikeDeclaration, type: InterfaceType, typeWithThis: Type, staticType: ObjectType) {
             const baseTypeNode = getEffectiveBaseTypeNode(node);
             const baseTypes = baseTypeNode && getBaseTypes(type);
             const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;
@@ -37833,56 +37833,142 @@ namespace ts {
                 if (isConstructorDeclaration(member)) {
                     forEach(member.parameters, param => {
                         if (isParameterPropertyDeclaration(param, member)) {
-                            checkClassMember(param, /*memberIsParameterProperty*/ true);
+                            checkExistingMemberForOverrideModifier(
+                                node,
+                                staticType,
+                                baseStaticType,
+                                baseWithThis,
+                                type,
+                                typeWithThis,
+                                param,
+                                /* memberIsParameterProperty */ true
+                            );
                         }
                     });
                 }
-                checkClassMember(member);
+                checkExistingMemberForOverrideModifier(
+                    node,
+                    staticType,
+                    baseStaticType,
+                    baseWithThis,
+                    type,
+                    typeWithThis,
+                    member,
+                    /* memberIsParameterProperty */ false,
+                );
+            }
+        }
+
+        /**
+         * @param member Existing member node to be checked.
+         * Note: `member` cannot be a synthetic node.
+        */
+        function checkExistingMemberForOverrideModifier(
+            node: ClassLikeDeclaration,
+            staticType: ObjectType,
+            baseStaticType: Type,
+            baseWithThis: Type | undefined,
+            type: InterfaceType,
+            typeWithThis: Type,
+            member: ClassElement | ParameterPropertyDeclaration,
+            memberIsParameterProperty: boolean,
+            reportErrors = true,
+        ): MemberOverrideDiagnostic {
+            const declaredProp = member.name
+                && getSymbolAtLocation(member.name)
+                || getSymbolAtLocation(member);
+            if (!declaredProp) {
+                return MemberOverrideDiagnostic.Ok;
             }
 
-            function checkClassMember(member: ClassElement | ParameterPropertyDeclaration, memberIsParameterProperty?: boolean) {
-                const hasOverride = hasOverrideModifier(member);
-                const hasStatic = isStatic(member);
-                if (baseWithThis && (hasOverride || compilerOptions.noImplicitOverride)) {
-                    const declaredProp = member.name && getSymbolAtLocation(member.name) || getSymbolAtLocation(member);
-                    if (!declaredProp) {
-                        return;
-                    }
+            return checkMemberForOverrideModifier(
+                node,
+                staticType,
+                baseStaticType,
+                baseWithThis,
+                type,
+                typeWithThis,
+                hasOverrideModifier(member),
+                hasAbstractModifier(member),
+                isStatic(member),
+                memberIsParameterProperty,
+                symbolName(declaredProp),
+                reportErrors ? member : undefined,
+            );
+        }
 
-                    const thisType = hasStatic ? staticType : typeWithThis;
-                    const baseType = hasStatic ? baseStaticType : baseWithThis;
-                    const prop = getPropertyOfType(thisType, declaredProp.escapedName);
-                    const baseProp = getPropertyOfType(baseType, declaredProp.escapedName);
+        /**
+         * Checks a class member declaration for either a missing or an invalid `override` modifier.
+         * Note: this function can be used for speculative checking,
+         * i.e. checking a member that does not yet exist in the program.
+         * An example of that would be to call this function in a completions scenario,
+         * when offering a method declaration as completion.
+         * @param errorNode The node where we should report an error, or undefined if we should not report errors.
+         */
+        function checkMemberForOverrideModifier(
+            node: ClassLikeDeclaration,
+            staticType: ObjectType,
+            baseStaticType: Type,
+            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)
+            memberHasAbstractModifier: boolean,
+            memberIsStatic: boolean,
+            memberIsParameterProperty: boolean,
+            memberName: string,
+            errorNode?: Node,
+        ): MemberOverrideDiagnostic {
+            const nodeInAmbientContext = !!(node.flags & NodeFlags.Ambient);
+            if (baseWithThis && (memberHasOverrideModifier || compilerOptions.noImplicitOverride)) {
+                const memberEscapedName = escapeLeadingUnderscores(memberName);
+                const thisType = memberIsStatic ? staticType : typeWithThis;
+                const baseType = memberIsStatic ? baseStaticType : baseWithThis;
+                const prop = getPropertyOfType(thisType, memberEscapedName);
+                const baseProp = getPropertyOfType(baseType, memberEscapedName);
 
-                    const baseClassName = typeToString(baseWithThis);
-                    if (prop && !baseProp && hasOverride) {
-                        const suggestion = getSuggestedSymbolForNonexistentClassMember(symbolName(declaredProp), baseType);
+                const baseClassName = typeToString(baseWithThis);
+                if (prop && !baseProp && memberHasOverrideModifier) {
+                    if (errorNode) {
+                        const suggestion = getSuggestedSymbolForNonexistentClassMember(memberName, baseType); // Again, using symbol name: note that's different from `symbol.escapedName`
                         suggestion ?
-                            error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0_Did_you_mean_1, baseClassName, symbolToString(suggestion)) :
-                            error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0, baseClassName);
+                            error(errorNode, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0_Did_you_mean_1, baseClassName, symbolToString(suggestion)) :
+                            error(errorNode, Diagnostics.This_member_cannot_have_an_override_modifier_because_it_is_not_declared_in_the_base_class_0, baseClassName);
+                    }
+                    return MemberOverrideDiagnostic.HasInvalidOverride;
+                }
+                else if (prop && baseProp?.declarations && compilerOptions.noImplicitOverride && !nodeInAmbientContext) {
+                    const baseHasAbstract = some(baseProp.declarations, hasAbstractModifier);
+                    if (memberHasOverrideModifier) {
+                        return MemberOverrideDiagnostic.Ok;
                     }
-                    else if (prop && baseProp?.declarations && compilerOptions.noImplicitOverride && !nodeInAmbientContext) {
-                        const baseHasAbstract = some(baseProp.declarations, hasAbstractModifier);
-                        if (hasOverride) {
-                            return;
-                        }
 
-                        if (!baseHasAbstract) {
+                    if (!baseHasAbstract) {
+                        if (errorNode) {
                             const diag = memberIsParameterProperty ?
                                 Diagnostics.This_parameter_property_must_have_an_override_modifier_because_it_overrides_a_member_in_base_class_0 :
                                 Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_a_member_in_the_base_class_0;
-                            error(member, diag, baseClassName);
+                            error(errorNode, diag, baseClassName);
                         }
-                        else if (hasAbstractModifier(member) && baseHasAbstract) {
-                            error(member, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0, baseClassName);
+                        return MemberOverrideDiagnostic.NeedsOverride;
+                    }
+                    else if (memberHasAbstractModifier && baseHasAbstract) {
+                        if (errorNode) {
+                            error(errorNode, Diagnostics.This_member_must_have_an_override_modifier_because_it_overrides_an_abstract_method_that_is_declared_in_the_base_class_0, baseClassName);
                         }
+                        return MemberOverrideDiagnostic.NeedsOverride;
                     }
                 }
-                else if (hasOverride) {
-                    const className = typeToString(type);
-                    error(member, Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class, className);
-                }
             }
+            else if (memberHasOverrideModifier) {
+                if (errorNode) {
+                    const className = typeToString(type);
+                    error(errorNode, Diagnostics.This_member_cannot_have_an_override_modifier_because_its_containing_class_0_does_not_extend_another_class, className);
+                }
+                return MemberOverrideDiagnostic.HasInvalidOverride;
+            }
+
+            return MemberOverrideDiagnostic.Ok;
         }
 
         function issueMemberSpecificError(node: ClassLikeDeclaration, typeWithThis: Type, baseWithThis: Type, broadDiag: DiagnosticMessage) {
@@ -37929,6 +38015,48 @@ namespace ts {
             }
         }
 
+        /**
+         * Checks a member declaration node to see if has a missing or invalid `override` modifier.
+         * @param node Class-like node where the member is declared.
+         * @param member Member declaration node.
+         * Note: `member` can be a synthetic node without a parent.
+         */
+        function getMemberOverrideModifierDiagnostic(node: ClassLikeDeclaration, member: ClassElement): MemberOverrideDiagnostic {
+            if (!member.name) {
+                return MemberOverrideDiagnostic.Ok;
+            }
+
+            const symbol = getSymbolOfNode(node);
+            const type = getDeclaredTypeOfSymbol(symbol) as InterfaceType;
+            const typeWithThis = getTypeWithThisArgument(type);
+            const staticType = getTypeOfSymbol(symbol) as ObjectType;
+
+            const baseTypeNode = getEffectiveBaseTypeNode(node);
+            const baseTypes = baseTypeNode && getBaseTypes(type);
+            const baseWithThis = baseTypes?.length ? getTypeWithThisArgument(first(baseTypes), type.thisType) : undefined;
+            const baseStaticType = getBaseConstructorTypeOfClass(type);
+
+            const memberHasOverrideModifier = member.parent
+                ? hasOverrideModifier(member)
+                : hasSyntacticModifier(member, ModifierFlags.Override);
+
+            const memberName = unescapeLeadingUnderscores(getTextOfPropertyName(member.name));
+
+            return checkMemberForOverrideModifier(
+                node,
+                staticType,
+                baseStaticType,
+                baseWithThis,
+                type,
+                typeWithThis,
+                memberHasOverrideModifier,
+                hasAbstractModifier(member),
+                isStatic(member),
+                /* memberIsParameterProperty */ false,
+                memberName,
+            );
+        }
+
         function getTargetSymbol(s: Symbol) {
             // if symbol is instantiated its flags are not copied from the 'target'
             // so we'll need to get back original 'target' symbol to work with correct set of flags

src/compiler/types.ts

@@ -4363,6 +4363,14 @@ namespace ts {
         /* @internal */ getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): readonly TypeParameter[] | undefined;
         /* @internal */ isDeclarationVisible(node: Declaration | AnyImportSyntax): boolean;
         /* @internal */ isPropertyAccessible(node: Node, isSuper: boolean, isWrite: boolean, containingType: Type, property: Symbol): boolean;
+        /* @internal */ getMemberOverrideModifierDiagnostic(node: ClassLikeDeclaration, member: ClassElement): MemberOverrideDiagnostic;
+    }
+
+    /* @internal */
+    export const enum MemberOverrideDiagnostic {
+        Ok,
+        NeedsOverride,
+        HasInvalidOverride
     }
 
     /* @internal */

src/services/completions.ts

@@ -746,6 +746,8 @@ namespace ts.Completions {
 
         let isSnippet: true | undefined;
         let insertText: string = name;
+
+        const checker = program.getTypeChecker();
         const sourceFile = location.getSourceFile();
         const printer = createPrinter({
             removeComments: true,
@@ -754,6 +756,7 @@ namespace ts.Completions {
             omitTrailingSemicolon: true,
         });
         const importAdder = codefix.createImportAdder(sourceFile, program, preferences, host);
+
         let body;
         if (preferences.includeCompletionsWithSnippetText) {
             isSnippet = true;
@@ -803,7 +806,8 @@ namespace ts.Completions {
                     //     );
                     // }
                 // }
-                if (options.noImplicitOverride && /* TODO: isOverride(node) */ undefined) {
+                if (isClassElement(node)
+                    && checker.getMemberOverrideModifierDiagnostic(classLikeDeclaration, node) === MemberOverrideDiagnostic.NeedsOverride) {
                     node = factory.updateModifiers(
                         node,
                         concatenate([factory.createModifier(SyntaxKind.OverrideKeyword)], node.modifiers),

tests/cases/fourslash/completionsOverridingMethod.ts

@@ -81,6 +81,17 @@
 ////    f/*g*/
 ////}
 
+// @Filename: h.ts
+// @noImplicitOverride: true // >> TODO: move this to a new test file, because this option is global
+// Case: Suggested method needs `override` modifier
+////class HBase {
+////    foo(a: string): void {}
+////}
+////
+////class HSub extends HBase {
+////    f/*h*/
+////}
+
 // format.setFormatOptions({
 //     newLineCharacter: "\n",
 // });
@@ -249,3 +260,25 @@ foo(a: any, b?: any): string {\r\n    $1;\r\n}\r\n",
     ],
 });
 
+verify.completions({
+    marker: "h",
+    isNewIdentifierLocation: true,
+    preferences: {
+        includeCompletionsWithInsertText: true,
+        includeCompletionsWithSnippetText: true,
+    },
+    includes: [
+        {
+            name: "foo",
+            sortText: completion.SortText.LocationPriority,
+            replacementSpan: {
+                fileName: "",
+                pos: 0,
+                end: 0,
+            },
+            isSnippet: true,
+            insertText:
+"override foo(): void {\r\n    $1;\r\n}\r\n",
+        }
+    ],
+});