From b245efda2db1fcd30f1203294e33576e22191eae Mon Sep 17 00:00:00 2001
From: detachhead <detachhead@users.noreply.github.com>
Date: Sat, 17 Aug 2024 19:15:50 +1000
Subject: [PATCH] don't `reportUnusedParameter` on dunders and do report them
 on old positional arguments

---
 .../pyright-internal/src/analyzer/checker.ts  | 37 ++++++++++++-------
 .../src/tests/checker.test.ts                 |  1 +
 .../tests/samples/reportUnusedParameter.py    | 16 +++++---
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
index f905aa90c7..c4b70e8ce8 100644
--- 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
index f648976d8e..3e3bf419c2 100644
--- 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
index 80e81ab303..1a040e6bc3 100644
--- 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): ...
\ No newline at end of file