Changed behavior of non-ClassVar variables within a protocol definiti…

…on. Previously, an error was reported when such variables were accessed from the class (as opposed to an instance of the class). Mypy (which was the reference implementation for PEP 544) does not report an error here. This addresses microsoft/pylance-release#4389.
jbradaric · Jun 20, 2023 · efea9b9 · efea9b9
1 parent 5568b4d
commit efea9b9
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 32 deletions.
diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts
@@ -2011,29 +2011,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
             );
         }
 
-        // If this is a protocol class X and we're accessing a non ClassVar,
-        // emit an error.
-        if (
-            memberInfo &&
-            memberInfo.classType &&
-            memberInfo.symbol &&
-            isClass(memberInfo.classType) &&
-            ClassType.isProtocolClass(memberInfo.classType)
-        ) {
-            const primaryDecl = getLastTypedDeclaredForSymbol(memberInfo.symbol);
-            if (primaryDecl && primaryDecl.type === DeclarationType.Variable && !memberInfo.isClassVar) {
-                addDiagnostic(
-                    AnalyzerNodeInfo.getFileInfo(errorNode).diagnosticRuleSet.reportGeneralTypeIssues,
-                    DiagnosticRule.reportGeneralTypeIssues,
-                    Localizer.Diagnostic.protocolMemberNotClassVar().format({
-                        memberName,
-                        className: memberInfo.classType.details.name,
-                    }),
-                    errorNode
-                );
-            }
-        }
-
         const isMemberPresentOnClass = memberInfo?.classType !== undefined;
 
         // If it wasn't found on the class, see if it's part of the metaclass.

diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts
@@ -723,10 +723,6 @@ export namespace Localizer {
             );
         export const protocolBaseClassWithTypeArgs = () => getRawString('Diagnostic.protocolBaseClassWithTypeArgs');
         export const protocolIllegal = () => getRawString('Diagnostic.protocolIllegal');
-        export const protocolMemberNotClassVar = () =>
-            new ParameterizedString<{ className: string; memberName: string }>(
-                getRawString('Diagnostic.protocolMemberNotClassVar')
-            );
         export const protocolNotAllowedInTypeArgument = () =>
             getRawString('Diagnostic.protocolNotAllowedInTypeArgument');
         export const protocolUsedInCall = () =>

diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json
@@ -352,7 +352,6 @@
         "protocolBaseClass": "Protocol class \"{classType}\" cannot derive from non-protocol class \"{baseType}\"",
         "protocolBaseClassWithTypeArgs": "Type arguments are not allowed with Protocol class when using type parameter syntax",
         "protocolIllegal": "Use of \"Protocol\" requires Python 3.7 or newer",
-        "protocolMemberNotClassVar": "Protocol class \"{className}\" does not define \"{memberName}\" as a ClassVar",
         "protocolNotAllowedInTypeArgument": "\"Protocol\" cannot be used as a type argument",
         "protocolUsedInCall": "Protocol class cannot be used in \"{name}\" call",
         "protocolVarianceContravariant": "Type variable \"{variable}\" used in generic protocol \"{class}\" should be contravariant",

diff --git a/packages/pyright-internal/src/tests/samples/classVar4.py b/packages/pyright-internal/src/tests/samples/classVar4.py
@@ -1,4 +1,4 @@
-# This sample tests that pyright emits an error when attempting to access
+# This sample tests that an error when attempting to access
 # a non-ClassVar protocol attribute from a protocol class.
 
 from typing import ClassVar, Protocol
@@ -24,10 +24,13 @@ class Class(SomeProtocol):
 
 
 def func1() -> None:
-    # This should generate an error because y is not a ClassVar.
+    # Previously (prior to pyright 1.1.315), this generated an error
+    # because x was not explicitly declared as a ClassVar. This was changed
+    # to match mypy, which treats this as a normal class variable -- one that
+    # can be accessed as both a class an instance variable.
     x: int = Class.x
 
-    # This should generate an error because y is not a ClassVar.
+    # Same as above.
     y: int = Class.y
 
     z: int = Class.z

diff --git a/packages/pyright-internal/src/tests/typeEvaluator4.test.ts b/packages/pyright-internal/src/tests/typeEvaluator4.test.ts
@@ -1049,7 +1049,7 @@ test('ClassVar3', () => {
 test('ClassVar4', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['classVar4.py']);
 
-    TestUtils.validateResults(analysisResults, 2);
+    TestUtils.validateResults(analysisResults, 0);
 });
 
 test('TypeVar1', () => {