Added check for data protocols used in an issubclass call. PEP 544 …

…indicates that this isn't allowed. This addresses #6881. (#6926)

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

@@ -105,6 +105,7 @@ import { getParameterListDetails } from './parameterUtils';
 import * as ParseTreeUtils from './parseTreeUtils';
 import { ParseTreeWalker } from './parseTreeWalker';
 import { validateClassPattern } from './patternMatching';
+import { isMethodOnlyProtocol } from './protocols';
 import { RegionComment, RegionCommentType, getRegionComments } from './regions';
 import { ScopeType } from './scope';
 import { getScopeForNode } from './scopeUtils';
@@ -3691,6 +3692,35 @@ export class Checker extends ParseTreeWalker {
             );
         }
 
+        // If this call is an issubclass, check for the use of a "data protocol",
+        // which PEP 544 says cannot be used in issubclass.
+        if (!isInstanceCheck) {
+            const diag = new DiagnosticAddendum();
+
+            doForEachSubtype(arg1Type, (arg1Subtype) => {
+                if (
+                    isInstantiableClass(arg1Subtype) &&
+                    ClassType.isProtocolClass(arg1Subtype) &&
+                    !isMethodOnlyProtocol(arg1Subtype)
+                ) {
+                    diag.addMessage(
+                        Localizer.DiagnosticAddendum.dataProtocolUnsupported().format({
+                            name: arg1Subtype.details.name,
+                        })
+                    );
+                }
+            });
+
+            if (!diag.isEmpty()) {
+                this._evaluator.addDiagnostic(
+                    this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
+                    DiagnosticRule.reportGeneralTypeIssues,
+                    Localizer.Diagnostic.dataProtocolInSubclassCheck(),
+                    node.arguments[1]
+                );
+            }
+        }
+
         // If this call is within an assert statement, we won't check whether
         // it's unnecessary.
         let curNode: ParseNode | undefined = node;

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

@@ -166,6 +166,33 @@ export function assignModuleToProtocol(
     );
 }
 
+// Determines whether the specified class is a protocol class that has
+// only methods, no other symbol types like variables.
+export function isMethodOnlyProtocol(classType: ClassType): boolean {
+    if (!ClassType.isProtocolClass(classType)) {
+        return false;
+    }
+
+    // First check for data members in any protocol base classes.
+    for (const baseClass of classType.details.baseClasses) {
+        if (isClass(baseClass) && ClassType.isProtocolClass(baseClass) && !isMethodOnlyProtocol(baseClass)) {
+            return false;
+        }
+    }
+
+    for (const [, symbol] of classType.details.fields) {
+        if (symbol.isIgnoredForProtocolMatch()) {
+            continue;
+        }
+
+        if (symbol.getDeclarations().some((decl) => decl.type !== DeclarationType.Function)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 // Looks up the protocol compatibility in the cache. If it's not found,
 // return undefined.
 function getProtocolCompatibility(

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

@@ -371,6 +371,7 @@ export namespace Localizer {
             getRawString('Diagnostic.dataClassTransformPositionalParam');
         export const dataClassTransformUnknownArgument = () =>
             new ParameterizedString<{ name: string }>(getRawString('Diagnostic.dataClassTransformUnknownArgument'));
+        export const dataProtocolInSubclassCheck = () => getRawString('Diagnostic.dataProtocolInSubclassCheck');
         export const declaredReturnTypePartiallyUnknown = () =>
             new ParameterizedString<{ returnType: string }>(
                 getRawString('Diagnostic.declaredReturnTypePartiallyUnknown')
@@ -1197,6 +1198,8 @@ export namespace Localizer {
         export const dataClassFrozen = () =>
             new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.dataClassFrozen'));
         export const dataClassFieldLocation = () => getRawString('DiagnosticAddendum.dataClassFieldLocation');
+        export const dataProtocolUnsupported = () =>
+            new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.dataProtocolUnsupported'));
         export const descriptorAccessBindingFailed = () =>
             new ParameterizedString<{ name: string; className: string }>(
                 getRawString('DiagnosticAddendum.descriptorAccessBindingFailed')

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

@@ -97,6 +97,7 @@
         "dataClassTransformFieldSpecifier": "Expected tuple of classes or functions but received type \"{type}\"",
         "dataClassTransformPositionalParam": "All arguments to \"dataclass_transform\" must be keyword arguments",
         "dataClassTransformUnknownArgument": "Argument \"{name}\" is not supported by dataclass_transform",
+        "dataProtocolInSubclassCheck": "Data protocols (which include non-method attributes) are not allowed in issubclass calls",
         "declaredReturnTypePartiallyUnknown": "Declared return type, \"{returnType}\", is partially unknown",
         "declaredReturnTypeUnknown": "Declared return type is unknown",
         "defaultValueContainsCall": "Function calls and mutable objects not allowed within parameter default value expression",
@@ -617,6 +618,7 @@
         "bytesTypePromotions": "Set disableBytesTypePromotions to false to enable type promotion behavior for \"bytearray\" and \"memoryview\"",
         "conditionalRequiresBool": "Method __bool__ for type \"{operandType}\" returns type \"{boolReturnType}\" rather than \"bool\"",
         "dataClassFieldLocation": "Field declaration",
+        "dataProtocolUnsupported": "\"{name}\" is a data protocol",
         "descriptorAccessBindingFailed": "Failed to bind method \"{name}\" for descriptor class \"{className}\"",
         "descriptorAccessCallFailed": "Failed to call method \"{name}\" for descriptor class \"{className}\"",
         "dataClassFrozen": "\"{name}\" is frozen",

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

@@ -0,0 +1,48 @@
+# This tests error reporting for the use of data protocols in an
+# issubclass call.
+
+from typing import Any, Protocol, runtime_checkable
+
+# > isinstance() can be used with both data and non-data protocols, while
+# > issubclass() can be used only with non-data protocols.
+
+
+@runtime_checkable
+class DataProtocol(Protocol):
+    name: str
+
+    def method1(self) -> int:
+        ...
+
+
+@runtime_checkable
+class DataProtocol2(DataProtocol, Protocol):
+    def method2(self) -> int:
+        ...
+
+
+@runtime_checkable
+class NonDataProtocol(Protocol):
+    def method1(self) -> int:
+        ...
+
+
+def func2(a: Any):
+    if isinstance(a, DataProtocol):
+        return
+
+    if isinstance(a, NonDataProtocol):
+        return
+
+    # This should generate an error because data protocols
+    # are not allowed with issubclass checks.
+    if issubclass(a, DataProtocol):
+        return
+
+    # This should generate an error because data protocols
+    # are not allowed with issubclass checks.
+    if issubclass(a, DataProtocol2):
+        return
+
+    if issubclass(a, NonDataProtocol):
+        return

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

@@ -305,6 +305,12 @@ test('isInstance4', () => {
     TestUtils.validateResults(analysisResults, 2);
 });
 
+test('isInstance5', () => {
+    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['isinstance5.py']);
+
+    TestUtils.validateResults(analysisResults, 2);
+});
+
 test('Unbound1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unbound1.py']);