don't reportUnusedParameter on dunders and do report them on old po…

…sitional arguments
DetachHead · Aug 17, 2024 · b245efd · b245efd
1 parent 4a4dfc3
commit b245efd
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -3673,24 +3673,35 @@ export class Checker extends ParseTreeWalker {
                     // check if it's an overridden method, in which case don't report unused parameters because the user has no choice
                     if (
                         nameNode &&
-                        !nameNode.d.value.startsWith('_') &&
+                        // parameters preficed with a single underscore mean intentionally unused, but parameters prefixed with a
+                        // double underscore are the old way of defining positional-only parameters, in which case we still want to
+                        // report an error if it's unused
+                        !SymbolNameUtils.isProtectedName(nameNode.d.value) &&
                         decl.node.parent?.nodeType === ParseNodeType.Function
                     ) {
                         const methodName = decl.node.parent.d.name;
-                        const functionType = this._evaluator.getType(methodName);
-                        if (functionType?.category === TypeCategory.Function && functionType.shared.methodClass) {
-                            const classType = functionType.shared.methodClass;
-                            if (
-                                !classType.shared.baseClasses
-                                    .filter(isClass)
-                                    .some((mroBaseClass) =>
-                                        lookUpClassMember(mroBaseClass, methodName.d.value, MemberAccessFlags.Default)
-                                    )
-                            ) {
+                        // dunders typically need to be treated the same as overridden methods, which is unsafe and cringe, ideally
+                        // they would always be overrides of an abstract method or something but whatever
+                        if (!SymbolNameUtils.isDunderName(methodName.d.value)) {
+                            const functionType = this._evaluator.getType(methodName);
+                            if (functionType?.category === TypeCategory.Function && functionType.shared.methodClass) {
+                                const classType = functionType.shared.methodClass;
+                                if (
+                                    !classType.shared.baseClasses
+                                        .filter(isClass)
+                                        .some((mroBaseClass) =>
+                                            lookUpClassMember(
+                                                mroBaseClass,
+                                                methodName.d.value,
+                                                MemberAccessFlags.Default
+                                            )
+                                        )
+                                ) {
+                                    rule = DiagnosticRule.reportUnusedParameter;
+                                }
+                            } else {
                                 rule = DiagnosticRule.reportUnusedParameter;
                             }
-                        } else {
-                            rule = DiagnosticRule.reportUnusedParameter;
                         }
                     }
                 } else {

diff --git a/packages/pyright-internal/src/tests/checker.test.ts b/packages/pyright-internal/src/tests/checker.test.ts
@@ -701,6 +701,7 @@ test('reportUnusedParameter', () => {
         errors: [
             { code: DiagnosticRule.reportUnusedParameter, line: 4 },
             { code: DiagnosticRule.reportUnusedParameter, line: 10 },
+            { code: DiagnosticRule.reportUnusedParameter, line: 22 },
         ],
     });
 });
diff --git a/packages/pyright-internal/src/tests/samples/reportUnusedParameter.py b/packages/pyright-internal/src/tests/samples/reportUnusedParameter.py
@@ -2,19 +2,25 @@
 from typing import override
 
 
-def foo(value: int): ...
+def foo(value: int): ... # error
 
 class Foo:
     @abstractmethod
-    def foo(self, asdf: int): ...
+    def foo(self, asdf: int): ...  # no error, abstract method
 
-    def bar(self, asdf: int): ...
+    def bar(self, asdf: int): ...  # error
 
 class Bar(Foo):
     @override
-    def foo(self, asdf: int): ...
+    def foo(self, asdf: int): ...  # no error, override
 
     @override
-    def bar(self, asdf: int): ...
+    def bar(self, asdf: int): ...  # no error, override
+
+    def __baz__(self, asdf: int):  # no error, dunder
+        ...
+
+    def qux(self, __asdf: int):  # error, unused cringe positional argument
+        ...
 
 def bar(_value: int): ...