Added missing check for inconsistent use of @final in an overloaded…

… function. Added missing check for override of an overloaded method marked `@final`. This addresses #6860 and #6866.

packages/pyright-internal/src/analyzer/checker.ts

@@ -345,6 +345,8 @@ export class Checker extends ParseTreeWalker {
 
             this._validateBaseClassOverrides(classTypeResult.classType);
 
+            this._validateOverloadDecoratorConsistency(classTypeResult.classType);
+
             this._validateMultipleInheritanceBaseClasses(classTypeResult.classType, node.name);
 
             this._validateMultipleInheritanceCompatibility(classTypeResult.classType, node.name);
@@ -5818,6 +5820,67 @@ export class Checker extends ParseTreeWalker {
         }
     }
 
+    // Validates that any overloaded methods are consistent in how they
+    // are decorated. For example, if the first overload is not marked @final
+    // but subsequent ones are, an error should be reported.
+    private _validateOverloadDecoratorConsistency(classType: ClassType) {
+        classType.details.fields.forEach((symbol, name) => {
+            const primaryDecl = getLastTypedDeclaredForSymbol(symbol);
+
+            if (!primaryDecl || primaryDecl.type !== DeclarationType.Function) {
+                return;
+            }
+
+            const typeOfSymbol = this._evaluator.getEffectiveTypeOfSymbol(symbol);
+
+            if (!isOverloadedFunction(typeOfSymbol)) {
+                return;
+            }
+
+            const overloads = OverloadedFunctionType.getOverloads(typeOfSymbol);
+
+            // If there's an implementation, it will determine whether the
+            // function is @final.
+            const implementation = OverloadedFunctionType.getImplementation(typeOfSymbol);
+            if (implementation) {
+                // If one or more of the overloads is marked @final but the
+                // implementation is not, report an error.
+                if (!FunctionType.isFinal(implementation)) {
+                    overloads.forEach((overload) => {
+                        if (FunctionType.isFinal(overload) && overload.details.declaration?.node) {
+                            this._evaluator.addDiagnostic(
+                                this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
+                                DiagnosticRule.reportGeneralTypeIssues,
+                                Localizer.Diagnostic.overloadFinalInconsistencyImpl().format({
+                                    name: overload.details.name,
+                                }),
+                                getNameNodeForDeclaration(overload.details.declaration) ??
+                                    overload.details.declaration.node
+                            );
+                        }
+                    });
+                }
+                return;
+            }
+
+            if (!FunctionType.isFinal(overloads[0])) {
+                overloads.slice(1).forEach((overload, index) => {
+                    if (FunctionType.isFinal(overload) && overload.details.declaration?.node) {
+                        this._evaluator.addDiagnostic(
+                            this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
+                            DiagnosticRule.reportGeneralTypeIssues,
+                            Localizer.Diagnostic.overloadFinalInconsistencyNoImpl().format({
+                                name: overload.details.name,
+                                index: index + 2,
+                            }),
+                            getNameNodeForDeclaration(overload.details.declaration) ?? overload.details.declaration.node
+                        );
+                    }
+                });
+            }
+        });
+    }
+
     // Validates that any overridden methods or variables contain the same
     // types as the original method. Also marks the class as abstract if one
     // or more abstract methods are not overridden.
@@ -6009,6 +6072,39 @@ export class Checker extends ParseTreeWalker {
         if (isFunction(baseType) || isOverloadedFunction(baseType)) {
             const diagAddendum = new DiagnosticAddendum();
 
+            // Determine whether this is an attempt to override a method marked @final.
+            let reportFinalMethodOverride = false;
+
+            // Private names (starting with double underscore) are exempt from this check.
+            if (!SymbolNameUtils.isPrivateName(memberName)) {
+                if (isFunction(baseType) && FunctionType.isFinal(baseType)) {
+                    reportFinalMethodOverride = true;
+                } else if (
+                    isOverloadedFunction(baseType) &&
+                    baseType.overloads.some((overload) => FunctionType.isFinal(overload))
+                ) {
+                    reportFinalMethodOverride = true;
+                }
+            }
+
+            if (reportFinalMethodOverride) {
+                const decl = getLastTypedDeclaredForSymbol(overrideSymbol);
+                if (decl && decl.type === DeclarationType.Function) {
+                    const diag = this._evaluator.addError(
+                        Localizer.Diagnostic.finalMethodOverride().format({
+                            name: memberName,
+                            className: baseClass.details.name,
+                        }),
+                        decl.node.name
+                    );
+
+                    const origDecl = getLastTypedDeclaredForSymbol(baseClassAndSymbol.symbol);
+                    if (diag && origDecl) {
+                        diag.addRelatedInfo(Localizer.DiagnosticAddendum.finalMethod(), origDecl.uri, origDecl.range);
+                    }
+                }
+            }
+
             if (isFunction(overrideType) || isOverloadedFunction(overrideType)) {
                 // Don't enforce parameter names for dundered methods. Many of them
                 // are misnamed in typeshed stubs, so this would result in many
@@ -6059,31 +6155,6 @@ export class Checker extends ParseTreeWalker {
                         }
                     }
                 }
-
-                if (isFunction(baseType)) {
-                    // Private names (starting with double underscore) are exempt from this check.
-                    if (!SymbolNameUtils.isPrivateName(memberName) && FunctionType.isFinal(baseType)) {
-                        const decl = getLastTypedDeclaredForSymbol(overrideSymbol);
-                        if (decl && decl.type === DeclarationType.Function) {
-                            const diag = this._evaluator.addError(
-                                Localizer.Diagnostic.finalMethodOverride().format({
-                                    name: memberName,
-                                    className: baseClass.details.name,
-                                }),
-                                decl.node.name
-                            );
-
-                            const origDecl = getLastTypedDeclaredForSymbol(baseClassAndSymbol.symbol);
-                            if (diag && origDecl) {
-                                diag.addRelatedInfo(
-                                    Localizer.DiagnosticAddendum.finalMethod(),
-                                    origDecl.uri,
-                                    origDecl.range
-                                );
-                            }
-                        }
-                    }
-                }
             } else if (!isAnyOrUnknown(overrideType)) {
                 // Special-case overrides of methods in '_TypedDict', since
                 // TypedDict attributes aren't manifest as attributes but rather

packages/pyright-internal/src/localization/localize.ts

@@ -708,6 +708,12 @@ export namespace Localizer {
             new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overloadAbstractMismatch'));
         export const overloadClassMethodInconsistent = () =>
             new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overloadClassMethodInconsistent'));
+        export const overloadFinalInconsistencyImpl = () =>
+            new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overloadFinalInconsistencyImpl'));
+        export const overloadFinalInconsistencyNoImpl = () =>
+            new ParameterizedString<{ name: string; index: number }>(
+                getRawString('Diagnostic.overloadFinalInconsistencyNoImpl')
+            );
         export const overloadImplementationMismatch = () =>
             new ParameterizedString<{ name: string; index: number }>(
                 getRawString('Diagnostic.overloadImplementationMismatch')

packages/pyright-internal/src/localization/package.nls.en-us.json

@@ -330,6 +330,8 @@
         "overlappingOverload": "Overload {obscured} for \"{name}\" will never be used because its parameters overlap overload {obscuredBy}",
         "overloadAbstractMismatch": "Overloaded methods must all be abstract or not",
         "overloadClassMethodInconsistent": "Overloads for \"{name}\" use @classmethod inconsistently",
+        "overloadFinalInconsistencyImpl": "Overload for \"{name}\" is marked @final but implementation is not",
+        "overloadFinalInconsistencyNoImpl": "Overload {index} for \"{name}\" is marked @final but overload 1 is not",
         "overloadImplementationMismatch": "Overloaded implementation is not consistent with signature of overload {index}",
         "overloadReturnTypeMismatch": "Overload {prevIndex} for \"{name}\" overlaps overload {newIndex} and returns an incompatible type",
         "overloadStaticMethodInconsistent": "Overloads for \"{name}\" use @staticmethod inconsistently",

packages/pyright-internal/src/tests/samples/final2.py

@@ -1,6 +1,6 @@
 # This sample tests the handling of the @final method decorator.
 
-from typing import final
+from typing import Any, cast, final, overload
 
 
 class ClassA:
@@ -28,6 +28,32 @@ def _func5(self):
     def __func6(self):
         pass
 
+    @overload
+    def func7(self, x: int) -> int:
+        ...
+
+    @overload
+    def func7(self, x: str) -> str:
+        ...
+
+    @final
+    def func7(self, x: int | str) -> int | str:
+        ...
+
+    # This should generate an error because the implementation
+    # of func8 is marked as not final but this overload is.
+    @overload
+    @final
+    def func8(self, x: int) -> int:
+        ...
+
+    @overload
+    def func8(self, x: str) -> str:
+        ...
+
+    def func8(self, x: int | str) -> int | str:
+        ...
+
 
 # This should generate an error because func3 is final.
 ClassA.func3 = lambda self: None
@@ -38,6 +64,9 @@ def __func6(self):
 # This should generate an error because _func5 is final.
 ClassA._func5 = lambda self: None
 
+# This should generate an error because func7 is final.
+ClassA.func7 = cast(Any, lambda self, x: "")
+
 
 class ClassB(ClassA):
     def func1(self):
@@ -46,7 +75,6 @@ def func1(self):
         @final
         def func1_inner():
             pass
-
 
     @classmethod
     def func2(cls):
@@ -73,6 +101,20 @@ def _func5(self):
     def __func6(self):
         pass
 
+    @overload
+    def func7(self, x: int) -> int:
+        ...
+
+    @overload
+    def func7(self, x: str) -> str:
+        ...
+
+    @final
+    # This should generate an error because func7 is
+    # defined as final.
+    def func7(self, x: int | str) -> int | str:
+        ...
+
 
 class Base4:
     ...

packages/pyright-internal/src/tests/samples/final6.pyi

@@ -0,0 +1,25 @@
+# This sample tests that an overloaded method in a stub honors the
+# @final on the first overload.
+
+from typing import final, overload
+
+class ClassA:
+    @overload
+    @final
+    def method1(self, x: int) -> int: ...
+    @overload
+    def method1(self, x: str) -> str: ...
+    @overload
+    def method2(self, x: int) -> int: ...
+    @overload
+    @final
+    # This should generate an error because the first overload
+    # is not marked @final but this one is.
+    def method2(self, x: str) -> str: ...
+
+class ClassB(ClassA):
+    @overload
+    def method1(self, x: int) -> int: ...
+    @overload
+    # This should generate an error.
+    def method1(self, x: str) -> str: ...

packages/pyright-internal/src/tests/typeEvaluator4.test.ts

@@ -362,7 +362,7 @@ test('Final1', () => {
 
 test('Final2', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['final2.py']);
-    TestUtils.validateResults(analysisResults, 9);
+    TestUtils.validateResults(analysisResults, 12);
 });
 
 test('Final3', () => {
@@ -380,6 +380,11 @@ test('Final5', () => {
     TestUtils.validateResults(analysisResults, 0);
 });
 
+test('Final6', () => {
+    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['final6.pyi']);
+    TestUtils.validateResults(analysisResults, 2);
+});
+
 test('InferredTypes1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['inferredTypes1.py']);
     TestUtils.validateResults(analysisResults, 0);