Augmented the reportIncompatibleMethodOverride diagnostic rule to check for cases where a non-function symbol within a subclass redefines a function symbol in a base class.

This commit is contained in:
Eric Traut 2020-07-26 09:46:19 -07:00
parent f19c9b2c87
commit 2568f190c5
5 changed files with 55 additions and 7 deletions

View File

@ -1736,15 +1736,22 @@ export class Checker extends ParseTreeWalker {
return;
}
const typeOfBaseClassMethod = this._evaluator.getEffectiveTypeOfSymbol(baseClassAndSymbol.symbol);
// If the base class doesn't provide a type declaration, we won't bother
// proceeding with additional checks. Type inference is too inaccurate
// in this case, plus it would be very slow.
if (!baseClassAndSymbol.symbol.hasTypedDeclarations()) {
return;
}
const baseClassSymbolType = this._evaluator.getEffectiveTypeOfSymbol(baseClassAndSymbol.symbol);
const diagAddendum = new DiagnosticAddendum();
if (
typeOfBaseClassMethod.category === TypeCategory.Function ||
typeOfBaseClassMethod.category === TypeCategory.OverloadedFunction
baseClassSymbolType.category === TypeCategory.Function ||
baseClassSymbolType.category === TypeCategory.OverloadedFunction
) {
if (typeOfSymbol.category === TypeCategory.Function) {
if (!this._evaluator.canOverrideMethod(typeOfBaseClassMethod, typeOfSymbol, diagAddendum)) {
if (!this._evaluator.canOverrideMethod(baseClassSymbolType, typeOfSymbol, diagAddendum)) {
const decl = getLastTypedDeclaredForSymbol(symbol);
if (decl && decl.type === DeclarationType.Function) {
const diag = this._evaluator.addDiagnostic(
@ -1768,8 +1775,8 @@ export class Checker extends ParseTreeWalker {
}
}
if (typeOfBaseClassMethod.category === TypeCategory.Function) {
if (FunctionType.isFinal(typeOfBaseClassMethod)) {
if (baseClassSymbolType.category === TypeCategory.Function) {
if (FunctionType.isFinal(baseClassSymbolType)) {
const decl = getLastTypedDeclaredForSymbol(symbol);
if (decl && decl.type === DeclarationType.Function) {
const diag = this._evaluator.addError(
@ -1791,6 +1798,29 @@ export class Checker extends ParseTreeWalker {
}
}
}
} else if (!isAnyOrUnknown(typeOfSymbol)) {
const decls = symbol.getDeclarations();
if (decls.length > 0) {
const lastDecl = decls[decls.length - 1];
const diag = this._evaluator.addDiagnostic(
this._fileInfo.diagnosticRuleSet.reportIncompatibleMethodOverride,
DiagnosticRule.reportIncompatibleMethodOverride,
Localizer.Diagnostic.methodOverridden().format({
name,
className: baseClassAndSymbol.classType.details.name,
}) + diagAddendum.getString(),
lastDecl.node
);
const origDecl = getLastTypedDeclaredForSymbol(baseClassAndSymbol.symbol);
if (diag && origDecl) {
diag.addRelatedInfo(
Localizer.DiagnosticAddendum.overriddenMethod(),
origDecl.path,
origDecl.range
);
}
}
}
}
});

View File

@ -357,6 +357,8 @@ export namespace Localizer {
export const methodNotDefinedOnType = () =>
new ParameterizedString<{ name: string; type: string }>(getRawString('Diagnostic.methodNotDefinedOnType'));
export const methodOrdering = () => getRawString('Diagnostic.methodOrdering');
export const methodOverridden = () =>
new ParameterizedString<{ name: string; className: string }>(getRawString('Diagnostic.methodOverridden'));
export const methodReturnsNonObject = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.methodReturnsNonObject'));
export const moduleContext = () => getRawString('Diagnostic.moduleContext');

View File

@ -152,6 +152,7 @@
"methodNotDefined": "\"{name}\" method not defined",
"methodNotDefinedOnType": "\"{name}\" method not defined on type \"{type}\"",
"methodOrdering": "Cannot create consistent method ordering",
"methodOverridden": "\"{name}\" overrides method of same name in class \"{className}\"",
"methodReturnsNonObject": "\"{name}\" method does not return an object",
"moduleContext": "Module not allowed in this context",
"moduleUnknownMember": "\"{name}\" is not a known member of module",

View File

@ -986,7 +986,7 @@ test('Classes2', () => {
// Turn on errors.
configOptions.diagnosticRuleSet.reportIncompatibleMethodOverride = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['classes2.py'], configOptions);
validateResults(analysisResults, 8);
validateResults(analysisResults, 10);
});
test('Classes3', () => {

View File

@ -47,6 +47,12 @@ class ParentClass():
def my_method14(self, a: int) -> int:
return 1
def my_method15(self, a: int) -> int:
return 1
def my_method16(self, a: int) -> int:
return 1
class ChildClass(ParentClass):
# This should generate an error because the type of 'a' doesn't match.
def my_method1(self, a: str):
@ -100,3 +106,12 @@ class ChildClass(ParentClass):
# wider than in the original method.
def my_method14(self, a: int) -> Union[int, str]:
return 1
# This should generate an error because we're overriding a
# method with a variable.
my_method15 = 3
# This should generate an error because we're overriding a
# method with a class.
class my_method16:
pass