Added error check for member access expressions that attempt to bind …

…an instance or class method to an object or class when the method accepts no "self" or "cls" parameter. This addresses #9942. (#9947)
microsoft · Feb 20, 2025 · e9edd94 · e9edd94
1 parent 16d5a2e
commit e9edd94
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 54 deletions.
diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts
@@ -28010,56 +28010,68 @@ export function createTypeEvaluator(
     ): FunctionType | undefined {
         const constraints = new ConstraintTracker();
 
-        if (firstParamType && memberType.shared.parameters.length > 0) {
-            const memberTypeFirstParam = memberType.shared.parameters[0];
-            const memberTypeFirstParamType = FunctionType.getParamType(memberType, 0);
-
-            if (
-                isTypeVar(memberTypeFirstParamType) &&
-                memberTypeFirstParamType.shared.boundType &&
-                isClassInstance(memberTypeFirstParamType.shared.boundType) &&
-                ClassType.isProtocolClass(memberTypeFirstParamType.shared.boundType)
-            ) {
-                // Handle the protocol class specially. Some protocol classes
-                // contain references to themselves or their subclasses, so if
-                // we attempt to call assignType, we'll risk infinite recursion.
-                // Instead, we'll assume it's assignable.
-                constraints.setBounds(
-                    memberTypeFirstParamType,
-                    TypeBase.isInstantiable(memberTypeFirstParamType)
-                        ? convertToInstance(firstParamType)
-                        : firstParamType
-                );
-            } else {
-                const subDiag = diag?.createAddendum();
+        if (firstParamType) {
+            if (memberType.shared.parameters.length > 0) {
+                const memberTypeFirstParam = memberType.shared.parameters[0];
+                const memberTypeFirstParamType = FunctionType.getParamType(memberType, 0);
 
                 if (
-                    !assignType(
-                        memberTypeFirstParamType,
-                        firstParamType,
-                        subDiag?.createAddendum(),
-                        constraints,
-                        AssignTypeFlags.AllowUnspecifiedTypeArgs,
-                        recursionCount
-                    )
+                    isTypeVar(memberTypeFirstParamType) &&
+                    memberTypeFirstParamType.shared.boundType &&
+                    isClassInstance(memberTypeFirstParamType.shared.boundType) &&
+                    ClassType.isProtocolClass(memberTypeFirstParamType.shared.boundType)
                 ) {
+                    // Handle the protocol class specially. Some protocol classes
+                    // contain references to themselves or their subclasses, so if
+                    // we attempt to call assignType, we'll risk infinite recursion.
+                    // Instead, we'll assume it's assignable.
+                    constraints.setBounds(
+                        memberTypeFirstParamType,
+                        TypeBase.isInstantiable(memberTypeFirstParamType)
+                            ? convertToInstance(firstParamType)
+                            : firstParamType
+                    );
+                } else {
+                    const subDiag = diag?.createAddendum();
+
                     if (
-                        memberTypeFirstParam.name &&
-                        !FunctionParam.isNameSynthesized(memberTypeFirstParam) &&
-                        FunctionParam.isTypeDeclared(memberTypeFirstParam)
+                        !assignType(
+                            memberTypeFirstParamType,
+                            firstParamType,
+                            subDiag?.createAddendum(),
+                            constraints,
+                            AssignTypeFlags.AllowUnspecifiedTypeArgs,
+                            recursionCount
+                        )
                     ) {
-                        if (subDiag) {
-                            subDiag.addMessage(
-                                LocMessage.bindTypeMismatch().format({
-                                    type: printType(firstParamType),
-                                    methodName: memberType.shared.name || '<anonymous>',
-                                    paramName: memberTypeFirstParam.name,
-                                })
-                            );
+                        if (
+                            memberTypeFirstParam.name &&
+                            !FunctionParam.isNameSynthesized(memberTypeFirstParam) &&
+                            FunctionParam.isTypeDeclared(memberTypeFirstParam)
+                        ) {
+                            if (subDiag) {
+                                subDiag.addMessage(
+                                    LocMessage.bindTypeMismatch().format({
+                                        type: printType(firstParamType),
+                                        methodName: memberType.shared.name || '<anonymous>',
+                                        paramName: memberTypeFirstParam.name,
+                                    })
+                                );
+                            }
+                            return undefined;
                         }
-                        return undefined;
                     }
                 }
+            } else {
+                const subDiag = diag?.createAddendum();
+                if (subDiag) {
+                    subDiag.addMessage(
+                        LocMessage.bindParamMissing().format({
+                            methodName: memberType.shared.name || '<anonymous>',
+                        })
+                    );
+                }
+                return undefined;
             }
         }
 

diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts
@@ -280,6 +280,8 @@ export namespace Localizer {
             );
         export const baseClassUnknown = () => getRawString('Diagnostic.baseClassUnknown');
         export const binaryOperationNotAllowed = () => getRawString('Diagnostic.binaryOperationNotAllowed');
+        export const bindParamMissing = () =>
+            new ParameterizedString<{ methodName: string }>(getRawString('Diagnostic.bindParamMissing'));
         export const bindTypeMismatch = () =>
             new ParameterizedString<{ type: string; methodName: string; paramName: string }>(
                 getRawString('Diagnostic.bindTypeMismatch')

diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json
@@ -103,6 +103,10 @@
         "baseClassUnknown": "Base class type is unknown, obscuring type of derived class",
         "baseClassVariableTypeIncompatible": "Base classes for class \"{classType}\" define variable \"{name}\" in incompatible way",
         "binaryOperationNotAllowed": "Binary operator not allowed in type expression",
+        "bindParamMissing": {
+            "message": "Could not bind method \"{methodName}\" because it is missing a \"self\" or \"cls\" parameter",
+            "comment": "Binding is the process through which Pyright determines what object a name refers to"
+        },
         "bindTypeMismatch": {
             "message": "Could not bind method \"{methodName}\" because \"{type}\" is not assignable to parameter \"{paramName}\"",
             "comment": "Binding is the process through which Pyright determines what object a name refers to"

diff --git a/packages/pyright-internal/src/tests/fourslash/completions.autoimport.shadow.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/completions.autoimport.shadow.fourslash.ts
@@ -11,12 +11,12 @@
 // @filename: testLib/__init__.pyi
 // @library: true
 //// class MyShadow:
-////     def method(): ...
+////     def method(self): ...
 
 // @filename: testLib/__init__.py
 // @library: true
 //// class MyShadow:
-////     def method():
+////     def method(self):
 ////         'doc string'
 ////         pass
 

diff --git a/packages/pyright-internal/src/tests/fourslash/completions.wildcardimports.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/completions.wildcardimports.fourslash.ts
@@ -12,7 +12,7 @@
 ////     pass
 ////
 //// class MyType:
-////     def func2():
+////     def func2(self):
 ////         '''func2 docs'''
 ////         pass
 
@@ -58,7 +58,7 @@
 ////
 //// func: ufunc
 //// class MyType:
-////     def func2() -> None : ...
+////     def func2(self) -> None : ...
 //// func3: ufunc
 //// func4: ufunc
 //// func5: ufunc

diff --git a/packages/pyright-internal/src/tests/fourslash/hover.wildcardimports.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/hover.wildcardimports.fourslash.ts
@@ -16,7 +16,7 @@
 ////     pass
 ////
 //// class MyType2:
-////     def func2():
+////     def func2(self):
 ////         '''func2 docs'''
 ////         pass
 
@@ -61,7 +61,7 @@
 //// func: Any
 //// MyType: Any
 //// class MyType2:
-////     def func2() -> None : ...
+////     def func2(self) -> None : ...
 //// func3: Any
 //// func4: Any
 //// func5: Any

diff --git a/...es/pyright-internal/src/tests/fourslash/signature.docstrings.wildcardimports.fourslash.ts b/...es/pyright-internal/src/tests/fourslash/signature.docstrings.wildcardimports.fourslash.ts
@@ -12,7 +12,7 @@
 ////     pass
 ////
 //// class MyType:
-////     def func2():
+////     def func2(self):
 ////         '''func2 docs'''
 ////         pass
 
@@ -58,7 +58,7 @@
 ////
 //// func: ufunc
 //// class MyType:
-////     def func2() -> None : ...
+////     def func2(self) -> None : ...
 //// func3: ufunc
 //// func4: ufunc
 //// func5: ufunc

diff --git a/packages/pyright-internal/src/tests/samples/enum1.py b/packages/pyright-internal/src/tests/samples/enum1.py
@@ -179,19 +179,19 @@ class TestEnum11(Enum):
 reveal_type(te11_A.value, expected_text="int")
 
 
-def func3() -> None:
+def func3(self) -> None:
     pass
 
 
 class TestEnum12(Enum):
     a = 1
-    b = lambda: None
+    b = lambda self: None
     c = func3
 
 
 reveal_type(TestEnum12.a, expected_text="Literal[TestEnum12.a]")
-reveal_type(TestEnum12.b, expected_text="() -> None")
-reveal_type(TestEnum12.c, expected_text="() -> None")
+reveal_type(TestEnum12.b, expected_text="(self: Unknown) -> None")
+reveal_type(TestEnum12.c, expected_text="(self: Unknown) -> None")
 
 
 class TestEnum13(metaclass=CustomEnumMeta1):