Added new diagnostic rule "reportIncompatibleVariableOverride" which is similar to "reportIncompatibleMethodOverride" except that it reports incompatible overrides of variables (non-methods).

client/schemas/pyrightconfig.schema.json

@@ -255,6 +255,12 @@
       "title": "Controls reporting of method overrides in subclasses that redefine the method in an incompatible way",
       "default": "none"
     },
+    "reportIncompatibleVariableOverride": {
+      "$id": "#/properties/reportIncompatibleVariableOverride",
+      "$ref": "#/definitions/diagnostic",
+      "title": "Controls reporting of overrides in subclasses that redefine a variable in an incompatible way",
+      "default": "none"
+    },
     "reportInvalidStringEscapeSequence": {
       "$id": "#/properties/reportInvalidStringEscapeSequence",
       "$ref": "#/definitions/diagnostic",

docs/configuration.md

@@ -88,7 +88,9 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 **reportConstantRedefinition** [boolean or string, optional]: Generate or suppress diagnostics for attempts to redefine variables whose names are all-caps with underscores and numerals. The default value for this setting is 'none'.
 
-**reportIncompatibleMethodOverride** [boolean or string, optional]: Generate or suppress diagnostics for methods that override a method of the same name in a base class in an incompatible manner (different number of parameters, different parameter types, or a different return type). The default value for this setting is 'none'.
+**reportIncompatibleMethodOverride** [boolean or string, optional]: Generate or suppress diagnostics for methods that override a method of the same name in a base class in an incompatible manner (wrong number of parameters, incompatible parameter types, or incompatible return type). The default value for this setting is 'none'.
+
+**reportIncompatibleVariableOverride** [boolean or string, optional]: Generate or suppress diagnostics for class variable declarations that override a symbol of the same name in a base class with a type that is incompatible with the base class symbol type. The default value for this setting is 'none'.
 
 **reportInvalidStringEscapeSequence** [boolean or string, optional]: Generate or suppress diagnostics for invalid escape sequences used within string literals. The Python specification indicates that such sequences will generate a syntax error in future versions. The default value for this setting is 'warning'.
 

server/src/analyzer/checker.ts

@@ -1729,6 +1729,11 @@ export class Checker extends ParseTreeWalker {
             // Get the symbol type defined in this class.
             const typeOfSymbol = this._evaluator.getEffectiveTypeOfSymbol(symbol);
 
+            // If the type of the override symbol isn't known, stop here.
+            if (isAnyOrUnknown(typeOfSymbol)) {
+                return;
+            }
+
             // Get the symbol defined in the base class.
             const baseClassAndSymbol = lookUpClassMember(classType, name, ClassMemberLookupFlags.SkipOriginalClass);
 
@@ -1822,6 +1827,39 @@ export class Checker extends ParseTreeWalker {
                         }
                     }
                 }
+            } else {
+                // This check can be expensive, so don't perform it if the corresponding
+                // rule is disabled.
+                if (this._fileInfo.diagnosticRuleSet.reportIncompatibleVariableOverride !== 'none') {
+                    // Verify that the override type is assignable to (same or narrower than)
+                    // the declared type of the base symbol.
+                    if (!this._evaluator.canAssignType(baseClassSymbolType, typeOfSymbol, diagAddendum)) {
+                        const decls = symbol.getDeclarations();
+                        if (decls.length > 0) {
+                            const lastDecl = decls[decls.length - 1];
+                            if (lastDecl) {
+                                const diag = this._evaluator.addDiagnostic(
+                                    this._fileInfo.diagnosticRuleSet.reportIncompatibleVariableOverride,
+                                    DiagnosticRule.reportIncompatibleVariableOverride,
+                                    Localizer.Diagnostic.symbolOverridden().format({
+                                        name,
+                                        className: baseClassAndSymbol.classType.details.name,
+                                    }) + diagAddendum.getString(),
+                                    lastDecl.node
+                                );
+
+                                const origDecl = getLastTypedDeclaredForSymbol(baseClassAndSymbol.symbol);
+                                if (diag && origDecl) {
+                                    diag.addRelatedInfo(
+                                        Localizer.DiagnosticAddendum.overriddenSymbol(),
+                                        origDecl.path,
+                                        origDecl.range
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }
             }
         });
     }

server/src/common/configOptions.ts

@@ -142,6 +142,10 @@ export interface DiagnosticRuleSet {
     // the base class method of the same name?
     reportIncompatibleMethodOverride: DiagnosticLevel;
 
+    // Report usage of variable override that is incompatible with
+    // the base class symbol of the same name?
+    reportIncompatibleVariableOverride: DiagnosticLevel;
+
     // Report usage of invalid escape sequences in string literals?
     reportInvalidStringEscapeSequence: DiagnosticLevel;
 
@@ -231,6 +235,7 @@ export function getDiagLevelDiagnosticRules() {
         DiagnosticRule.reportPrivateUsage,
         DiagnosticRule.reportConstantRedefinition,
         DiagnosticRule.reportIncompatibleMethodOverride,
+        DiagnosticRule.reportIncompatibleVariableOverride,
         DiagnosticRule.reportInvalidStringEscapeSequence,
         DiagnosticRule.reportUnknownParameterType,
         DiagnosticRule.reportUnknownArgumentType,
@@ -287,6 +292,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
         reportPrivateUsage: 'none',
         reportConstantRedefinition: 'none',
         reportIncompatibleMethodOverride: 'none',
+        reportIncompatibleVariableOverride: 'none',
         reportInvalidStringEscapeSequence: 'none',
         reportUnknownParameterType: 'none',
         reportUnknownArgumentType: 'none',
@@ -339,6 +345,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
         reportPrivateUsage: 'none',
         reportConstantRedefinition: 'none',
         reportIncompatibleMethodOverride: 'none',
+        reportIncompatibleVariableOverride: 'none',
         reportInvalidStringEscapeSequence: 'warning',
         reportUnknownParameterType: 'none',
         reportUnknownArgumentType: 'none',
@@ -391,6 +398,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
         reportPrivateUsage: 'error',
         reportConstantRedefinition: 'error',
         reportIncompatibleMethodOverride: 'error',
+        reportIncompatibleVariableOverride: 'error',
         reportInvalidStringEscapeSequence: 'error',
         reportUnknownParameterType: 'error',
         reportUnknownArgumentType: 'error',
@@ -857,6 +865,13 @@ export class ConfigOptions {
                 defaultSettings.reportIncompatibleMethodOverride
             ),
 
+            // Read the "reportIncompatibleVariableOverride" entry.
+            reportIncompatibleVariableOverride: this._convertDiagnosticLevel(
+                configObj.reportIncompatibleVariableOverride,
+                DiagnosticRule.reportIncompatibleVariableOverride,
+                defaultSettings.reportIncompatibleVariableOverride
+            ),
+
             // Read the "reportInvalidStringEscapeSequence" entry.
             reportInvalidStringEscapeSequence: this._convertDiagnosticLevel(
                 configObj.reportInvalidStringEscapeSequence,

server/src/common/diagnosticRules.ts

@@ -37,6 +37,7 @@ export const enum DiagnosticRule {
     reportPrivateUsage = 'reportPrivateUsage',
     reportConstantRedefinition = 'reportConstantRedefinition',
     reportIncompatibleMethodOverride = 'reportIncompatibleMethodOverride',
+    reportIncompatibleVariableOverride = 'reportIncompatibleVariableOverride',
     reportInvalidStringEscapeSequence = 'reportInvalidStringEscapeSequence',
     reportUnknownParameterType = 'reportUnknownParameterType',
     reportUnknownArgumentType = 'reportUnknownArgumentType',

server/src/localization/localize.ts

@@ -472,6 +472,8 @@ export namespace Localizer {
             new ParameterizedString<{ name: string }>(getRawString('Diagnostic.symbolIsUndefined'));
         export const symbolIsPossiblyUnbound = () =>
             new ParameterizedString<{ name: string }>(getRawString('Diagnostic.symbolIsPossiblyUnbound'));
+        export const symbolOverridden = () =>
+            new ParameterizedString<{ name: string; className: string }>(getRawString('Diagnostic.symbolOverridden'));
         export const tupleInAnnotation = () => getRawString('Diagnostic.tupleInAnnotation');
         export const tupleSizeMismatch = () =>
             new ParameterizedString<{ expected: number; received: number }>(
@@ -656,6 +658,7 @@ export namespace Localizer {
         export const overloadCallName = () =>
             new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.overloadCallName'));
         export const overriddenMethod = () => getRawString('DiagnosticAddendum.overriddenMethod');
+        export const overriddenSymbol = () => getRawString('DiagnosticAddendum.overriddenSymbol');
         export const overrideParamCount = () =>
             new ParameterizedString<{ baseCount: number; overrideCount: number }>(
                 getRawString('DiagnosticAddendum.overrideParamCount')

server/src/localization/package.nls.en-us.json

@@ -228,6 +228,7 @@
         "symbolIsUnbound": "\"{name}\" is unbound",
         "symbolIsUndefined": "\"{name}\" is not defined",
         "symbolIsPossiblyUnbound": "\"{name}\" is possibly unbound",
+        "symbolOverridden": "\"{name}\" overrides symbol of same name in class \"{className}\"",
         "tupleInAnnotation": "Tuple expression not allowed in type annotation",
         "tupleSizeMismatch": "Tuple size mismatch: expected {expected} but received {received}",
         "typeAbstract": "Cannot instantiate abstract class \"{type}\"",
@@ -323,6 +324,7 @@
         "noOverloadAssignable": "No overloaded function matches type \"{type}\"",
         "overloadCallName": "Calling function \"{name}\"",
         "overriddenMethod": "Overridden method",
+        "overriddenSymbol": "Overridden symbol",
         "overrideParamCount": "Parameter count mismatch; base method has {baseCount}, but override has {overrideCount}",
         "overrideParamName": "Parameter {index} name mismatch: base parameter is named \"{baseName}\", override parameter is named \"{overrideName}\"",
         "overrideParamType": "Parameter {index} type mismatch: base parameter is type \"{baseType}\", override parameter is type \"{overrideType}\"",

server/src/tests/checker.test.ts

@@ -1001,6 +1001,19 @@ test('Classes4', () => {
     validateResults(analysisResults, 0);
 });
 
+test('Classes5', () => {
+    const configOptions = new ConfigOptions('.');
+
+    // By default, optional diagnostics are ignored.
+    let analysisResults = TestUtils.typeAnalyzeSampleFiles(['classes5.py'], configOptions);
+    validateResults(analysisResults, 0);
+
+    // Turn on errors.
+    configOptions.diagnosticRuleSet.reportIncompatibleVariableOverride = 'error';
+    analysisResults = TestUtils.typeAnalyzeSampleFiles(['classes5.py'], configOptions);
+    validateResults(analysisResults, 4);
+});
+
 test('Mro1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['mro1.py']);
 

server/src/tests/samples/classes5.py

@@ -0,0 +1,70 @@
+# This sample tests the reportIncompatibleVariableOverride
+# configuration option.
+
+from typing import Union
+from abc import ABC, abstractmethod
+
+class ParentClass(ABC):
+    var1: int
+    var2: str
+    var3: Union[int, str]
+    var4: int
+    var5: int
+
+    @property
+    def property1(self) -> int:
+        return 1
+    
+    @property
+    def property2(self) -> int:
+        return 1
+
+    @property
+    def property3(self) -> int:
+        return 1
+
+    @property
+    def property4(self) -> Union[str, int]:
+        return 1
+
+    @property
+    @abstractmethod
+    def property5(self) -> int:
+        raise NotImplementedError()
+
+
+class Subclass(ParentClass):
+    # This should generate an error because the type is incompatible.
+    var1: str
+
+    var2: str
+
+    var3: int
+
+    # This should generate an error because the type is incompatible.
+    var4 = ""
+
+    var5 = 5
+
+    # This should generate an error because a
+    # property object is different from a simple
+    # variable.
+    property1: int
+
+    @property
+    def property2(self) -> int:
+        return 3
+
+    # This should generate an error because it is
+    # an incompatible property.
+    @property
+    def property3(self) -> str:
+        return ""
+
+    @property
+    def property4(self) -> int:
+        return 1
+
+    @property
+    def property5(self) -> int:
+        return 4