mirror of
https://github.com/microsoft/pyright.git
synced 2024-08-15 10:50:44 +03:00
Added support for detecting mismatched property types in base classes used for multiple inheritance. This addresses #8167. (#8175)
This commit is contained in:
Eric Traut
GitHub
parent
1bf99d3212
commit
20942b3389
@ -5954,7 +5954,15 @@ export class Checker extends ParseTreeWalker {
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// TODO - check types of property methods fget, fset, fdel.
|
||||
this._validateMultipleInheritancePropertyOverride(
|
||||
overriddenClassAndSymbol.classType,
|
||||
childClassType,
|
||||
overriddenType,
|
||||
overrideType,
|
||||
overrideSymbol,
|
||||
memberName,
|
||||
errorNode
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// This check can be expensive, so don't perform it if the corresponding
|
||||
@ -6051,26 +6059,156 @@ export class Checker extends ParseTreeWalker {
|
||||
}
|
||||
|
||||
if (diag && overrideDecl && overriddenDecl) {
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.baseClassOverriddenType().format({
|
||||
baseClass: this._evaluator.printType(convertToInstance(overriddenClassAndSymbol.classType)),
|
||||
type: this._evaluator.printType(overriddenType),
|
||||
}),
|
||||
overriddenDecl.uri,
|
||||
overriddenDecl.range
|
||||
);
|
||||
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.baseClassOverridesType().format({
|
||||
baseClass: this._evaluator.printType(convertToInstance(overrideClassAndSymbol.classType)),
|
||||
type: this._evaluator.printType(overrideType),
|
||||
}),
|
||||
overrideDecl.uri,
|
||||
overrideDecl.range
|
||||
this._addMultipleInheritanceRelatedInfo(
|
||||
diag,
|
||||
overriddenClassAndSymbol.classType,
|
||||
overriddenType,
|
||||
overriddenDecl,
|
||||
overrideClassAndSymbol.classType,
|
||||
overrideType,
|
||||
overrideDecl
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private _addMultipleInheritanceRelatedInfo(
|
||||
diag: Diagnostic,
|
||||
overriddenClass: ClassType,
|
||||
overriddenType: Type,
|
||||
overriddenDecl: Declaration,
|
||||
overrideClass: ClassType,
|
||||
overrideType: Type,
|
||||
overrideDecl: Declaration
|
||||
) {
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.baseClassOverriddenType().format({
|
||||
baseClass: this._evaluator.printType(convertToInstance(overriddenClass)),
|
||||
type: this._evaluator.printType(overriddenType),
|
||||
}),
|
||||
overriddenDecl.uri,
|
||||
overriddenDecl.range
|
||||
);
|
||||
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.baseClassOverridesType().format({
|
||||
baseClass: this._evaluator.printType(convertToInstance(overrideClass)),
|
||||
type: this._evaluator.printType(overrideType),
|
||||
}),
|
||||
overrideDecl.uri,
|
||||
overrideDecl.range
|
||||
);
|
||||
}
|
||||
|
||||
private _validateMultipleInheritancePropertyOverride(
|
||||
overriddenClassType: ClassType,
|
||||
overrideClassType: ClassType,
|
||||
overriddenSymbolType: Type,
|
||||
overrideSymbolType: Type,
|
||||
overrideSymbol: Symbol,
|
||||
memberName: string,
|
||||
errorNode: ParseNode
|
||||
) {
|
||||
const propMethodInfo: [string, (c: ClassType) => FunctionType | undefined][] = [
|
||||
['fget', (c) => c.fgetInfo?.methodType],
|
||||
['fset', (c) => c.fsetInfo?.methodType],
|
||||
['fdel', (c) => c.fdelInfo?.methodType],
|
||||
];
|
||||
|
||||
propMethodInfo.forEach((info) => {
|
||||
const diagAddendum = new DiagnosticAddendum();
|
||||
const [methodName, methodAccessor] = info;
|
||||
const baseClassPropMethod = methodAccessor(overriddenSymbolType as ClassType);
|
||||
const subclassPropMethod = methodAccessor(overrideSymbolType as ClassType);
|
||||
|
||||
// Is the method present on the base class but missing in the subclass?
|
||||
if (baseClassPropMethod) {
|
||||
const baseClassMethodType = partiallySpecializeType(baseClassPropMethod, overriddenClassType);
|
||||
|
||||
if (isFunction(baseClassMethodType)) {
|
||||
if (!subclassPropMethod) {
|
||||
// The method is missing.
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodMissing().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
|
||||
const decls = overrideSymbol.getDeclarations();
|
||||
|
||||
if (decls.length > 0) {
|
||||
const lastDecl = decls[decls.length - 1];
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: overriddenClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
errorNode
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
this._addMultipleInheritanceRelatedInfo(
|
||||
diag,
|
||||
overriddenClassType,
|
||||
overriddenSymbolType,
|
||||
origDecl,
|
||||
overrideClassType,
|
||||
overrideSymbolType,
|
||||
lastDecl
|
||||
);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
const subclassMethodType = partiallySpecializeType(subclassPropMethod, overrideClassType);
|
||||
|
||||
if (isFunction(subclassMethodType)) {
|
||||
if (
|
||||
!this._evaluator.validateOverrideMethod(
|
||||
baseClassMethodType,
|
||||
subclassMethodType,
|
||||
overrideClassType,
|
||||
diagAddendum.createAddendum()
|
||||
)
|
||||
) {
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodIncompatible().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
const decl = subclassMethodType.details.declaration;
|
||||
|
||||
if (decl && decl.type === DeclarationType.Function) {
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: overriddenClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
errorNode
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
this._addMultipleInheritanceRelatedInfo(
|
||||
diag,
|
||||
overriddenClassType,
|
||||
overriddenSymbolType,
|
||||
origDecl,
|
||||
overrideClassType,
|
||||
overrideSymbolType,
|
||||
decl
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Validates that any overloaded methods are consistent in how they
|
||||
// are decorated. For example, if the first overload is not marked @final
|
||||
// but subsequent ones are, an error should be reported.
|
||||
@ -6572,93 +6710,14 @@ export class Checker extends ParseTreeWalker {
|
||||
);
|
||||
}
|
||||
} else {
|
||||
const baseClassType = baseClass;
|
||||
const propMethodInfo: [string, (c: ClassType) => FunctionType | undefined][] = [
|
||||
['fget', (c) => c.fgetInfo?.methodType],
|
||||
['fset', (c) => c.fsetInfo?.methodType],
|
||||
['fdel', (c) => c.fdelInfo?.methodType],
|
||||
];
|
||||
|
||||
propMethodInfo.forEach((info) => {
|
||||
const diagAddendum = new DiagnosticAddendum();
|
||||
const [methodName, methodAccessor] = info;
|
||||
const baseClassPropMethod = methodAccessor(baseType as ClassType);
|
||||
const subclassPropMethod = methodAccessor(overrideType as ClassType);
|
||||
|
||||
// Is the method present on the base class but missing in the subclass?
|
||||
if (baseClassPropMethod) {
|
||||
const baseClassMethodType = partiallySpecializeType(baseClassPropMethod, baseClassType);
|
||||
if (isFunction(baseClassMethodType)) {
|
||||
if (!subclassPropMethod) {
|
||||
// The method is missing.
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodMissing().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
const decls = overrideSymbol.getDeclarations();
|
||||
if (decls.length > 0) {
|
||||
const lastDecl = decls[decls.length - 1];
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: baseClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
getNameNodeForDeclaration(lastDecl) ?? lastDecl.node
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.overriddenMethod(),
|
||||
origDecl.uri,
|
||||
origDecl.range
|
||||
);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
const subclassMethodType = partiallySpecializeType(subclassPropMethod, childClassType);
|
||||
if (isFunction(subclassMethodType)) {
|
||||
if (
|
||||
!this._evaluator.validateOverrideMethod(
|
||||
baseClassMethodType,
|
||||
subclassMethodType,
|
||||
childClassType,
|
||||
diagAddendum.createAddendum()
|
||||
)
|
||||
) {
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodIncompatible().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
const decl = subclassMethodType.details.declaration;
|
||||
if (decl && decl.type === DeclarationType.Function) {
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: baseClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
decl.node.name
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.overriddenMethod(),
|
||||
origDecl.uri,
|
||||
origDecl.range
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
this._validatePropertyOverride(
|
||||
baseClass,
|
||||
childClassType,
|
||||
baseType,
|
||||
overrideType,
|
||||
overrideSymbol,
|
||||
memberName
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// This check can be expensive, so don't perform it if the corresponding
|
||||
@ -6859,6 +6918,103 @@ export class Checker extends ParseTreeWalker {
|
||||
}
|
||||
}
|
||||
|
||||
private _validatePropertyOverride(
|
||||
baseClassType: ClassType,
|
||||
childClassType: ClassType,
|
||||
baseType: Type,
|
||||
childType: Type,
|
||||
overrideSymbol: Symbol,
|
||||
memberName: string
|
||||
) {
|
||||
const propMethodInfo: [string, (c: ClassType) => FunctionType | undefined][] = [
|
||||
['fget', (c) => c.fgetInfo?.methodType],
|
||||
['fset', (c) => c.fsetInfo?.methodType],
|
||||
['fdel', (c) => c.fdelInfo?.methodType],
|
||||
];
|
||||
|
||||
propMethodInfo.forEach((info) => {
|
||||
const diagAddendum = new DiagnosticAddendum();
|
||||
const [methodName, methodAccessor] = info;
|
||||
const baseClassPropMethod = methodAccessor(baseType as ClassType);
|
||||
const subclassPropMethod = methodAccessor(childType as ClassType);
|
||||
|
||||
// Is the method present on the base class but missing in the subclass?
|
||||
if (baseClassPropMethod) {
|
||||
const baseClassMethodType = partiallySpecializeType(baseClassPropMethod, baseClassType);
|
||||
|
||||
if (isFunction(baseClassMethodType)) {
|
||||
if (!subclassPropMethod) {
|
||||
// The method is missing.
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodMissing().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
|
||||
const decls = overrideSymbol.getDeclarations();
|
||||
|
||||
if (decls.length > 0) {
|
||||
const lastDecl = decls[decls.length - 1];
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: baseClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
getNameNodeForDeclaration(lastDecl) ?? lastDecl.node
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
diag.addRelatedInfo(LocAddendum.overriddenMethod(), origDecl.uri, origDecl.range);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
const subclassMethodType = partiallySpecializeType(subclassPropMethod, childClassType);
|
||||
|
||||
if (isFunction(subclassMethodType)) {
|
||||
if (
|
||||
!this._evaluator.validateOverrideMethod(
|
||||
baseClassMethodType,
|
||||
subclassMethodType,
|
||||
childClassType,
|
||||
diagAddendum.createAddendum()
|
||||
)
|
||||
) {
|
||||
diagAddendum.addMessage(
|
||||
LocAddendum.propertyMethodIncompatible().format({
|
||||
name: methodName,
|
||||
})
|
||||
);
|
||||
const decl = subclassMethodType.details.declaration;
|
||||
|
||||
if (decl && decl.type === DeclarationType.Function) {
|
||||
const diag = this._evaluator.addDiagnostic(
|
||||
DiagnosticRule.reportIncompatibleMethodOverride,
|
||||
LocMessage.propertyOverridden().format({
|
||||
name: memberName,
|
||||
className: baseClassType.details.name,
|
||||
}) + diagAddendum.getString(),
|
||||
decl.node.name
|
||||
);
|
||||
|
||||
const origDecl = baseClassMethodType.details.declaration;
|
||||
if (diag && origDecl) {
|
||||
diag.addRelatedInfo(
|
||||
LocAddendum.overriddenMethod(),
|
||||
origDecl.uri,
|
||||
origDecl.range
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Performs checks on a function that is located within a class
|
||||
// and has been determined not to be a property accessor.
|
||||
private _validateMethod(node: FunctionNode, functionType: FunctionType, classNode: ClassNode) {
|
||||
|
||||
@ -7,96 +7,78 @@ from typing import Generic, Iterable, ParamSpec, TypeVar
|
||||
|
||||
|
||||
class A1:
|
||||
def func1(self, a: int) -> str:
|
||||
...
|
||||
def func1(self, a: int) -> str: ...
|
||||
|
||||
|
||||
class A2:
|
||||
def func1(self, a: int, b: int = 3) -> str:
|
||||
...
|
||||
def func1(self, a: int, b: int = 3) -> str: ...
|
||||
|
||||
|
||||
# This should generate an error because func1 is incompatible.
|
||||
class ASub(A1, A2):
|
||||
...
|
||||
class ASub(A1, A2): ...
|
||||
|
||||
|
||||
class B1:
|
||||
def func1(self) -> int:
|
||||
...
|
||||
def func1(self) -> int: ...
|
||||
|
||||
|
||||
class B2:
|
||||
def func1(self) -> float:
|
||||
...
|
||||
def func1(self) -> float: ...
|
||||
|
||||
|
||||
class BSub(B1, B2):
|
||||
...
|
||||
class BSub(B1, B2): ...
|
||||
|
||||
|
||||
class C1:
|
||||
def func1(self) -> float:
|
||||
...
|
||||
def func1(self) -> float: ...
|
||||
|
||||
|
||||
class C2:
|
||||
def func1(self) -> int:
|
||||
...
|
||||
def func1(self) -> int: ...
|
||||
|
||||
|
||||
# This should generate an error because func1 is incompatible.
|
||||
class CSub(C1, C2):
|
||||
...
|
||||
class CSub(C1, C2): ...
|
||||
|
||||
|
||||
class D1:
|
||||
def func1(self, a: int) -> None:
|
||||
...
|
||||
def func1(self, a: int) -> None: ...
|
||||
|
||||
|
||||
class D2:
|
||||
def func1(self, b: int) -> None:
|
||||
...
|
||||
def func1(self, b: int) -> None: ...
|
||||
|
||||
|
||||
# This should generate an error because func1 is incompatible.
|
||||
class DSub(D1, D2):
|
||||
...
|
||||
class DSub(D1, D2): ...
|
||||
|
||||
|
||||
_T_E = TypeVar("_T_E")
|
||||
|
||||
|
||||
class E1(Generic[_T_E]):
|
||||
def func1(self, a: _T_E) -> None:
|
||||
...
|
||||
def func1(self, a: _T_E) -> None: ...
|
||||
|
||||
|
||||
class E2(Generic[_T_E]):
|
||||
def func1(self, a: _T_E) -> None:
|
||||
...
|
||||
def func1(self, a: _T_E) -> None: ...
|
||||
|
||||
|
||||
class ESub(E1[int], E2[int]):
|
||||
...
|
||||
class ESub(E1[int], E2[int]): ...
|
||||
|
||||
|
||||
_T_F = TypeVar("_T_F")
|
||||
|
||||
|
||||
class F1(Generic[_T_F]):
|
||||
def do_stuff(self) -> Iterable[_T_F]:
|
||||
...
|
||||
def do_stuff(self) -> Iterable[_T_F]: ...
|
||||
|
||||
|
||||
class F2(F1[_T_F]):
|
||||
def do_stuff(self) -> Iterable[_T_F]:
|
||||
...
|
||||
def do_stuff(self) -> Iterable[_T_F]: ...
|
||||
|
||||
|
||||
class F3(F1[_T_F]):
|
||||
...
|
||||
class F3(F1[_T_F]): ...
|
||||
|
||||
|
||||
class FSub1(F3[int], F2[int]):
|
||||
@ -116,43 +98,77 @@ _R = TypeVar("_R")
|
||||
|
||||
|
||||
class G1(Generic[_P, _R]):
|
||||
def f(self, *args: _P.args, **kwargs: _P.kwargs) -> _R:
|
||||
...
|
||||
def f(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: ...
|
||||
|
||||
def g(self) -> _R:
|
||||
...
|
||||
def g(self) -> _R: ...
|
||||
|
||||
|
||||
class G2(G1[_P, _R]):
|
||||
# This should generate an error because f is missing ParamSpec parameters.
|
||||
def f(self) -> _R:
|
||||
...
|
||||
def f(self) -> _R: ...
|
||||
|
||||
def g(self, *args: _P.args, **kwargs: _P.kwargs) -> _R:
|
||||
...
|
||||
def g(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: ...
|
||||
|
||||
|
||||
class G3(G1[[], _R]):
|
||||
def f(self) -> _R:
|
||||
...
|
||||
def f(self) -> _R: ...
|
||||
|
||||
def g(self) -> _R:
|
||||
...
|
||||
def g(self) -> _R: ...
|
||||
|
||||
|
||||
class G4(G1[[int, int], str]):
|
||||
def f(self, a: int, b: int, /) -> str:
|
||||
...
|
||||
def f(self, a: int, b: int, /) -> str: ...
|
||||
|
||||
def g(self) -> str:
|
||||
...
|
||||
def g(self) -> str: ...
|
||||
|
||||
|
||||
class G5(G1[[], str]):
|
||||
# This should generate an error because the specialized
|
||||
# signature of f in the base class has no positional parameters.
|
||||
def f(self, a: int, b: int) -> str:
|
||||
...
|
||||
def f(self, a: int, b: int) -> str: ...
|
||||
|
||||
def g(self) -> str:
|
||||
...
|
||||
def g(self) -> str: ...
|
||||
|
||||
|
||||
class H1:
|
||||
@property
|
||||
def prop1(self) -> int:
|
||||
return 3
|
||||
|
||||
@property
|
||||
def prop2(self) -> int:
|
||||
return 3
|
||||
|
||||
@prop2.setter
|
||||
def prop2(self, val: int) -> None:
|
||||
pass
|
||||
|
||||
@property
|
||||
def prop3(self) -> int:
|
||||
return 3
|
||||
|
||||
@prop3.setter
|
||||
def prop3(self, val: int) -> None:
|
||||
pass
|
||||
|
||||
|
||||
class H2:
|
||||
@property
|
||||
def prop1(self) -> str:
|
||||
return ""
|
||||
|
||||
@property
|
||||
def prop2(self) -> int:
|
||||
return 3
|
||||
|
||||
@property
|
||||
def prop3(self) -> int:
|
||||
return 3
|
||||
|
||||
@prop3.setter
|
||||
def prop3(self, val: str) -> None:
|
||||
pass
|
||||
|
||||
|
||||
# This should generate three errors: prop1, prop2 and prop3.
|
||||
class H(H2, H1): ...
|
||||
|
||||
@ -910,7 +910,7 @@ test('MethodOverride3', () => {
|
||||
|
||||
configOptions.diagnosticRuleSet.reportIncompatibleMethodOverride = 'error';
|
||||
analysisResults = TestUtils.typeAnalyzeSampleFiles(['methodOverride3.py'], configOptions);
|
||||
TestUtils.validateResults(analysisResults, 5);
|
||||
TestUtils.validateResults(analysisResults, 8);
|
||||
});
|
||||
|
||||
test('MethodOverride4', () => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user