reportUnusedFunction on unused protected methods when the class is decorated with @final

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

@@ -211,6 +211,8 @@ interface TypeVarUsageInfo {
     nodes: NameNode[];
 }
 
+type SymbolVisibility = 'public' | 'protected' | 'private';
+
 // When enabled, this debug flag causes the code complexity of
 // functions to be emitted.
 const isPrintCodeComplexityEnabled = false;
@@ -2964,7 +2966,7 @@ export class Checker extends ParseTreeWalker {
 
             if (scope) {
                 scope.symbolTable.forEach((symbol, name) => {
-                    this._conditionallyReportUnusedSymbol(name, symbol, scope.type, dependentFileInfo);
+                    this._conditionallyReportUnusedSymbol(name, symbol, scope.type, dependentFileInfo, scopedNode);
 
                     this._reportIncompatibleDeclarations(name, symbol);
 
@@ -2996,7 +2998,7 @@ export class Checker extends ParseTreeWalker {
                 if (!accessedSymbolSet.has(symbol.id)) {
                     const decls = symbol.getDeclarations();
                     decls.forEach((decl) => {
-                        this._conditionallyReportUnusedDeclaration(decl, /* isPrivate */ false);
+                        this._conditionallyReportUnusedDeclaration(decl, 'public');
                     });
                 }
             }
@@ -3554,7 +3556,8 @@ export class Checker extends ParseTreeWalker {
         name: string,
         symbol: Symbol,
         scopeType: ScopeType,
-        dependentFileInfo?: AnalyzerFileInfo[]
+        dependentFileInfo: AnalyzerFileInfo[] | undefined,
+        scopedNode: AnalyzerNodeInfo.ScopedNode
     ) {
         const accessedSymbolSet = this._fileInfo.accessedSymbolSet;
         if (symbol.isIgnoredForProtocolMatch() || accessedSymbolSet.has(symbol.id)) {
@@ -3579,11 +3582,15 @@ export class Checker extends ParseTreeWalker {
 
         const decls = symbol.getDeclarations();
         decls.forEach((decl) => {
-            this._conditionallyReportUnusedDeclaration(decl, this._isSymbolPrivate(name, scopeType));
+            this._conditionallyReportUnusedDeclaration(decl, this._symbolVisibility(name, scopeType), scopedNode);
         });
     }
 
-    private _conditionallyReportUnusedDeclaration(decl: Declaration, isPrivate: boolean) {
+    private _conditionallyReportUnusedDeclaration(
+        decl: Declaration,
+        symbolVisibility: SymbolVisibility,
+        scopedNode?: AnalyzerNodeInfo.ScopedNode
+    ) {
         let nameNode: NameNode | undefined;
         let message: string | undefined;
         let rule: DiagnosticRule | undefined;
@@ -3642,7 +3649,7 @@ export class Checker extends ParseTreeWalker {
             case DeclarationType.TypeAlias:
             case DeclarationType.Variable:
             case DeclarationType.Param:
-                if (!isPrivate) {
+                if (symbolVisibility !== 'private') {
                     return;
                 }
 
@@ -3711,7 +3718,7 @@ export class Checker extends ParseTreeWalker {
                 break;
 
             case DeclarationType.Class:
-                if (!isPrivate) {
+                if (symbolVisibility !== 'private') {
                     return;
                 }
 
@@ -3726,22 +3733,50 @@ export class Checker extends ParseTreeWalker {
                 message = LocMessage.unaccessedClass().format({ name: nameNode.d.value });
                 break;
 
-            case DeclarationType.Function:
-                if (!isPrivate) {
-                    return;
-                }
+            case DeclarationType.Function: {
+                const type =
+                    scopedNode?.nodeType === ParseNodeType.Class
+                        ? this._evaluator.getTypeOfClass(scopedNode)?.classType
+                        : undefined;
+                const functionType = this._evaluator.getTypeForDeclaration(decl).type;
+                if (
+                    symbolVisibility === 'private' ||
+                    // normally if it's a protected method we don't report it as unused because a subclass could use it,
+                    // but that's not possible in cases where the class is decorated with `@final`:
+                    (symbolVisibility === 'protected' &&
+                        (!type || ClassType.isFinal(type)) &&
+                        // unless it's overriding a method with the same name in a base class, in which case we also shouldn't
+                        // report it because it could have a usage in the base class
+                        !(
+                            // check if it's explicitly overridden
+                            (
+                                (functionType && isFunction(functionType) && FunctionType.isOverridden(functionType)) ||
+                                // if it's not explicitly overridden, check if it's implicitly overridden by checking for the
+                                // same method on a base class in case the user doesn't have reportImplicitOverride enabled
+                                type?.shared.baseClasses.find(
+                                    (baseClass) =>
+                                        isClass(baseClass) &&
+                                        lookUpClassMember(
+                                            baseClass,
+                                            decl.node.d.name.d.value,
+                                            MemberAccessFlags.Default
+                                        )
+                                )
+                            )
+                        ))
+                ) {
+                    // If a stub is exporting a private type, we'll assume that the author
+                    // knows what he or she is doing.
+                    if (this._fileInfo.isStubFile) {
+                        return;
+                    }
 
-                // If a stub is exporting a private type, we'll assume that the author
-                // knows what he or she is doing.
-                if (this._fileInfo.isStubFile) {
-                    return;
+                    nameNode = decl.node.d.name;
+                    rule = DiagnosticRule.reportUnusedFunction;
+                    message = LocMessage.unaccessedFunction().format({ name: nameNode.d.value });
                 }
-
-                nameNode = decl.node.d.name;
-                rule = DiagnosticRule.reportUnusedFunction;
-                message = LocMessage.unaccessedFunction().format({ name: nameNode.d.value });
                 break;
-
+            }
             case DeclarationType.TypeParam:
                 // Never report a diagnostic for an unused TypeParam.
                 nameNode = decl.node.d.name;
@@ -4054,25 +4089,25 @@ export class Checker extends ParseTreeWalker {
         }
     }
 
-    private _isSymbolPrivate(nameValue: string, scopeType: ScopeType) {
-        // All variables within the scope of a function or a list
-        // comprehension are considered private.
-        if (scopeType === ScopeType.Function || scopeType === ScopeType.Comprehension) {
-            return true;
-        }
-
-        // See if the symbol is private.
-        if (SymbolNameUtils.isPrivateName(nameValue)) {
-            return true;
+    private _symbolVisibility(nameValue: string, scopeType: ScopeType): SymbolVisibility {
+        if (
+            // All variables within the scope of a function or a list
+            // comprehension are considered private.
+            scopeType === ScopeType.Function ||
+            scopeType === ScopeType.Comprehension ||
+            // See if the symbol is private.
+            SymbolNameUtils.isPrivateName(nameValue)
+        ) {
+            return 'private';
         }
 
         if (SymbolNameUtils.isProtectedName(nameValue)) {
             // Protected names outside of a class scope are considered private.
             const isClassScope = scopeType === ScopeType.Class;
-            return !isClassScope;
+            return isClassScope ? 'protected' : 'private';
         }
 
-        return false;
+        return 'public';
     }
 
     private _reportDeprecatedClassProperty(node: FunctionNode, functionTypeResult: FunctionTypeResult) {

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

@@ -0,0 +1,22 @@
+from typing import final, override
+
+@final
+class Final:
+    def foo(self): ... # no error
+    def _foo(self): ... # error
+    def __bar(self): ... # error
+
+class NotFinal:
+    def foo(self): ... # no error
+    def _foo(self): ... # no error
+    def __bar(self): ... # error
+
+@final
+class FinalWithExplicitOverride(NotFinal):
+    @override
+    def _foo(self): ... # no error
+
+@final
+class FinalWithImplicitOverride(NotFinal):
+    def _foo(self): ... # no error
+    def _bar(self): ... # error

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

@@ -142,3 +142,17 @@ describe('narrowing type vars using their bounds', () => {
         });
     });
 });
+
+test('`reportUnusedFunction` on `@final` classes', () => {
+    const configOptions = new BasedConfigOptions(Uri.empty());
+    configOptions.diagnosticRuleSet.reportImplicitOverride = 'none';
+    const analysisResults = typeAnalyzeSampleFiles(['reportUnusedFunction_final.py'], configOptions);
+    validateResultsButBased(analysisResults, {
+        warnings: [
+            { line: 5, code: DiagnosticRule.reportUnusedFunction },
+            { line: 6, code: DiagnosticRule.reportUnusedFunction },
+            { line: 11, code: DiagnosticRule.reportUnusedFunction },
+            { line: 21, code: DiagnosticRule.reportUnusedFunction },
+        ],
+    });
+});