Added code to enforce invariance of class-scoped variables in overrid…

…es when the `reportIncompatibleVariableOverride` rule is enabled. This addresses #5933.
microsoft · Sep 12, 2023 · 0932cf4 · 0932cf4
1 parent 669a738
commit 0932cf4
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 5 deletions.
diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -5877,12 +5877,37 @@ export class Checker extends ParseTreeWalker {
             // rule is disabled.
             if (this._fileInfo.diagnosticRuleSet.reportIncompatibleVariableOverride !== 'none') {
                 const decls = overrideSymbol.getDeclarations();
+
                 if (decls.length > 0) {
                     const lastDecl = decls[decls.length - 1];
+                    const primaryDecl = decls[0];
+
                     // Verify that the override type is assignable to (same or narrower than)
                     // the declared type of the base symbol.
-                    const diagAddendum = new DiagnosticAddendum();
-                    if (!this._evaluator.assignType(baseType, overrideType, diagAddendum)) {
+                    const isInvariant = primaryDecl?.type === DeclarationType.Variable && !primaryDecl.isFinal;
+
+                    let diagAddendum = new DiagnosticAddendum();
+                    if (
+                        !this._evaluator.assignType(
+                            baseType,
+                            overrideType,
+                            diagAddendum,
+                            /* destTypeVarContext */ undefined,
+                            /* srcTypeVarContext */ undefined,
+                            isInvariant ? AssignTypeFlags.EnforceInvariance : AssignTypeFlags.Default
+                        )
+                    ) {
+                        if (isInvariant) {
+                            diagAddendum = new DiagnosticAddendum();
+                            diagAddendum.addMessage(Localizer.DiagnosticAddendum.overrideIsInvariant());
+                            diagAddendum.createAddendum().addMessage(
+                                Localizer.DiagnosticAddendum.overrideInvariantMismatch().format({
+                                    overrideType: this._evaluator.printType(overrideType),
+                                    baseType: this._evaluator.printType(baseType),
+                                })
+                            );
+                        }
+
                         const diag = this._evaluator.addDiagnostic(
                             this._fileInfo.diagnosticRuleSet.reportIncompatibleVariableOverride,
                             DiagnosticRule.reportIncompatibleVariableOverride,

diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts
@@ -1248,6 +1248,11 @@ export namespace Localizer {
             new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.overloadNotAssignable'));
         export const overriddenMethod = () => getRawString('DiagnosticAddendum.overriddenMethod');
         export const overriddenSymbol = () => getRawString('DiagnosticAddendum.overriddenSymbol');
+        export const overrideIsInvariant = () => getRawString('DiagnosticAddendum.overrideIsInvariant');
+        export const overrideInvariantMismatch = () =>
+            new ParameterizedString<{ overrideType: string; baseType: string }>(
+                getRawString('DiagnosticAddendum.overrideInvariantMismatch')
+            );
         export const overrideNoOverloadMatches = () => getRawString('DiagnosticAddendum.overrideNoOverloadMatches');
         export const overrideNotClassMethod = () => getRawString('DiagnosticAddendum.overrideNotClassMethod');
         export const overrideNotInstanceMethod = () => getRawString('DiagnosticAddendum.overrideNotInstanceMethod');

diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json
@@ -634,6 +634,8 @@
         "overloadNotAssignable": "One or more overloads of \"{name}\" is not assignable",
         "overriddenMethod": "Overridden method",
         "overriddenSymbol": "Overridden symbol",
+        "overrideIsInvariant": "Variable is mutable so its type is invariant",
+        "overrideInvariantMismatch": "Override type \"{overrideType}\" is not the same as base type \"{baseType}\"",
         "overrideNoOverloadMatches": "No overload signature in override is compatible with base method",
         "overrideNotClassMethod": "Base method is declared as a classmethod but override is not",
         "overrideNotInstanceMethod": "Base method is declared as an instance method but override is not",

diff --git a/packages/pyright-internal/src/tests/samples/classes5.py b/packages/pyright-internal/src/tests/samples/classes5.py
@@ -49,6 +49,8 @@ class Subclass1(ParentClass1):
 
     var2: str
 
+    # This should generate an error if reportIncompatibleVariableOverride is
+    # enabled because the member is mutable, and is therefore invariant.
     var3: int
 
     # This should generate an error.
@@ -119,7 +121,7 @@ def __init__(self):
 
 
 class SubclassDeclared2(ParentClass2):
-    cv_decl_1: int
+    cv_decl_1: float
 
     # This should generate an error if reportIncompatibleVariableOverride
     # is enabled.
@@ -134,6 +136,8 @@ class SubclassDeclared2(ParentClass2):
     cv_infer_3: float | None
 
     def __init__(self):
+        # This should generate an error if reportIncompatibleVariableOverride
+        # is enabled because the member is mutable and therefore invariant.
         self.cv_decl_4: int
 
         # This should generate an error if reportIncompatibleVariableOverride
@@ -241,6 +245,7 @@ def test3(self) -> int:
     test4: int
     test5: Any
     test6: float
+    test7: float
 
 
 class PeerClass2:
@@ -254,9 +259,10 @@ def test4(self) -> int:
 
     test5: int
     test6: Any
+    test7: int
 
 
-# This should generate 3 errors if reportIncompatibleVariableOverride
+# This should generate 4 errors if reportIncompatibleVariableOverride
 # is enabled.
 class MultipleInheritance1(PeerClass1, PeerClass2):
     pass

diff --git a/packages/pyright-internal/src/tests/typeEvaluator3.test.ts b/packages/pyright-internal/src/tests/typeEvaluator3.test.ts
@@ -760,7 +760,7 @@ test('Classes5', () => {
     // Turn on reportIncompatibleVariableOverride.
     configOptions.diagnosticRuleSet.reportIncompatibleVariableOverride = 'error';
     analysisResults = TestUtils.typeAnalyzeSampleFiles(['classes5.py'], configOptions);
-    TestUtils.validateResults(analysisResults, 32);
+    TestUtils.validateResults(analysisResults, 36);
 });
 
 test('Classes6', () => {