microsoft/pyright
1
1
Fork 0
mirror of https://github.com/microsoft/pyright.git synced 2024-08-16 11:20:22 +03:00
Code Issues Projects Releases Wiki Activity

Added new diagnostic check reportInconsistentConstructor that checks for inconsistent input signatures between __new__ and __init__ methods.

Browse Source
This commit is contained in:
Eric Traut 2022-01-11 23:58:05 -08:00
parent f1d8669d1c
commit 4934e906d5
13 changed files with 320 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/configuration.md
View File

@ -110,6 +110,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
**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'.
**reportInconsistentConstructor** [boolean or string, optional]: Generate or suppress diagnostics when an `__init__` method signature is inconsistent with a `__new__` signature. The default value for this setting is 'none'.
**reportOverlappingOverload** [boolean or string, optional]: Generate or suppress diagnostics for function overloads that overlap in signature and obscure each other or have incompatible return types. The default value for this setting is 'none'.
**reportUninitializedInstanceVariable** [boolean or string, optional]: Generate or suppress diagnostics for instance variables within a class that are not initialized or declared within the class body or the `__init__` method. The default value for this setting is 'none'.
@ -304,6 +306,7 @@ The following table lists the default severity levels for each diagnostic rule w
| reportConstantRedefinition | "none" | "none" | "error" |
| reportIncompatibleMethodOverride | "none" | "none" | "error" |
| reportIncompatibleVariableOverride | "none" | "none" | "error" |
| reportInconsistentConstructor | "none" | "none" | "error" |
| reportOverlappingOverload | "none" | "none" | "error" |
| reportUninitializedInstanceVariable | "none" | "none" | "none" |
| reportInvalidStringEscapeSequence | "none" | "warning" | "error" |

187
packages/pyright-internal/src/analyzer/checker.ts
View File

@ -99,6 +99,7 @@ import {
ClassType,
combineTypes,
FunctionType,
FunctionTypeFlags,
isAnyOrUnknown,
isClass,
isClassInstance,
@ -323,6 +324,8 @@ export class Checker extends ParseTreeWalker {
this._validateMultipleInheritanceCompatibility(classTypeResult.classType, node.name);
this._validateConstructorConsistency(classTypeResult.classType);
this._validateFinalMemberOverrides(classTypeResult.classType);
this._validateInstanceVariableInitialization(classTypeResult.classType);
@ -3590,6 +3593,190 @@ export class Checker extends ParseTreeWalker {
});
}
// Validates that the __init__ and __new__ method signatures are consistent.
private _validateConstructorConsistency(classType: ClassType) {
const initMember = lookUpClassMember(
classType,
'__init__',
ClassMemberLookupFlags.SkipObjectBaseClass | ClassMemberLookupFlags.SkipInstanceVariables
);
const newMember = lookUpClassMember(
classType,
'__new__',
ClassMemberLookupFlags.SkipObjectBaseClass | ClassMemberLookupFlags.SkipInstanceVariables
);
if (!initMember || !newMember || !isClass(initMember.classType) || !isClass(newMember.classType)) {
return;
}
// If both the __new__ and __init__ come from subclasses, don't bother
// checking for this class.
if (
!ClassType.isSameGenericClass(newMember.classType, classType) &&
!ClassType.isSameGenericClass(initMember.classType, classType)
) {
return;
}
// If the class that provides the __new__ method has a custom metaclass with a
// __call__ method, skip this check.
const metaclass = newMember.classType.details.effectiveMetaclass;
if (metaclass && isClass(metaclass) && !ClassType.isBuiltIn(metaclass, 'type')) {
const callMethod = lookUpClassMember(
metaclass,
'__call__',
ClassMemberLookupFlags.SkipTypeBaseClass | ClassMemberLookupFlags.SkipInstanceVariables
);
if (callMethod) {
return;
}
}
let newMemberType: Type | undefined = this._evaluator.getTypeOfMember(newMember);
if (!isFunction(newMemberType) && !isOverloadedFunction(newMemberType)) {
return;
}
newMemberType = this._evaluator.bindFunctionToClassOrObject(
classType,
newMemberType,
/* memberClass */ undefined,
/* errorNode */ undefined,
/* recursionCount */ undefined,
/* treatConstructorAsClassMember */ true
);
if (!newMemberType) {
return;
}
if (isOverloadedFunction(newMemberType)) {
// Find the implementation, not the overloaded signatures.
newMemberType = newMemberType.overloads.find((func) => !FunctionType.isOverloaded(func));
if (!newMemberType) {
return;
}
}
let initMemberType: Type | undefined = this._evaluator.getTypeOfMember(initMember);
if (!isFunction(initMemberType) && !isOverloadedFunction(initMemberType)) {
return;
}
initMemberType = this._evaluator.bindFunctionToClassOrObject(
ClassType.cloneAsInstance(classType),
initMemberType
);
if (!initMemberType) {
return;
}
if (isOverloadedFunction(initMemberType)) {
// Find the implementation, not the overloaded signatures.
initMemberType = initMemberType.overloads.find((func) => !FunctionType.isOverloaded(func));
if (!initMemberType) {
return;
}
}
if (!isFunction(initMemberType) || !isFunction(newMemberType)) {
return;
}
// If either of the functions has a default parameter signature
// (* args: Any, ** kwargs: Any), don't proceed with the check.
if (FunctionType.hasDefaultParameters(initMemberType) || FunctionType.hasDefaultParameters(newMemberType)) {
return;
}
// We'll set the "SkipArgsKwargs" flag for pragmatic reasons since __new__
// often has an *args and/or **kwargs. We'll also set the ParamSpecValue
// because we don't care about the return type for this check.
initMemberType = FunctionType.cloneWithNewFlags(
initMemberType,
initMemberType.details.flags |
FunctionTypeFlags.SkipArgsKwargsCompatibilityCheck |
FunctionTypeFlags.ParamSpecValue
);
newMemberType = FunctionType.cloneWithNewFlags(
newMemberType,
initMemberType.details.flags |
FunctionTypeFlags.SkipArgsKwargsCompatibilityCheck |
FunctionTypeFlags.ParamSpecValue
);
if (
!this._evaluator.canAssignType(
newMemberType,
initMemberType,
/* diag */ undefined,
/* typeVarMap */ undefined,
CanAssignFlags.SkipFunctionReturnTypeCheck
) ||
!this._evaluator.canAssignType(
initMemberType,
newMemberType,
/* diag */ undefined,
/* typeVarMap */ undefined,
CanAssignFlags.SkipFunctionReturnTypeCheck
)
) {
const displayOnInit = ClassType.isSameGenericClass(initMember.classType, classType);
const initDecl = getLastTypedDeclaredForSymbol(initMember.symbol);
const newDecl = getLastTypedDeclaredForSymbol(newMember.symbol);
if (initDecl && newDecl) {
const mainDecl = displayOnInit ? initDecl : newDecl;
const mainDeclNode =
mainDecl.node.nodeType === ParseNodeType.Function ? mainDecl.node.name : mainDecl.node;
const diagAddendum = new DiagnosticAddendum();
const initSignature = this._evaluator.printType(initMemberType);
const newSignature = this._evaluator.printType(newMemberType);
diagAddendum.addMessage(
Localizer.DiagnosticAddendum.initMethodSignature().format({
type: initSignature,
})
);
diagAddendum.addMessage(
Localizer.DiagnosticAddendum.newMethodSignature().format({
type: newSignature,
})
);
const diagnostic = this._evaluator.addDiagnostic(
this._fileInfo.diagnosticRuleSet.reportInconsistentConstructor,
DiagnosticRule.reportInconsistentConstructor,
Localizer.Diagnostic.constructorParametersMismatch().format({
classType: this._evaluator.printType(
ClassType.cloneAsInstance(displayOnInit ? initMember.classType : newMember.classType)
),
}) + diagAddendum.getString(),
mainDeclNode
);
if (diagnostic) {
const secondaryDecl = displayOnInit ? newDecl : initDecl;
diagnostic.addRelatedInfo(
(displayOnInit
? Localizer.DiagnosticAddendum.newMethodLocation()
: Localizer.DiagnosticAddendum.initMethodLocation()
).format({
type: this._evaluator.printType(
ClassType.cloneAsInstance(displayOnInit ? newMember.classType : initMember.classType)
),
}),
secondaryDecl.path,
secondaryDecl.range
);
}
}
}
}
// Validates that any methods in multiple base classes are compatible with each other.
private _validateMultipleInheritanceCompatibility(classType: ClassType, errorNode: ParseNode) {
// Skip this check if reportIncompatibleMethodOverride is disabled because it's

7
packages/pyright-internal/src/analyzer/typeEvaluatorTypes.ts
View File

@ -360,7 +360,12 @@ export interface TypeEvaluator {
) => Type | undefined;
bindFunctionToClassOrObject: (
baseType: ClassType | undefined,
memberType: FunctionType | OverloadedFunctionType
memberType: FunctionType | OverloadedFunctionType,
memberClass?: ClassType,
errorNode?: ParseNode,
recursionCount?: number,
treatConstructorAsClassMember?: boolean,
firstParamType?: ClassType | TypeVarType
) => FunctionType | OverloadedFunctionType | undefined;
getCallSignatureInfo: (node: CallNode, activeIndex: number, activeOrFake: boolean) => CallSignatureInfo | undefined;
getTypeAnnotationForParameter: (node: FunctionNode, paramIndex: number) => ExpressionNode | undefined;

3
packages/pyright-internal/src/analyzer/typeEvaluatorWithTracker.ts
View File

@ -124,8 +124,7 @@ export function createTypeEvaluatorWithTracker(
getTypeFromObjectMember: typeEvaluator.getTypeFromObjectMember,
getBoundMethod: typeEvaluator.getBoundMethod,
getTypeFromMagicMethodReturn: typeEvaluator.getTypeFromMagicMethodReturn,
bindFunctionToClassOrObject: (b, m) =>
run('bindFunctionToClassOrObject', () => typeEvaluator.bindFunctionToClassOrObject(b, m), m),
bindFunctionToClassOrObject: typeEvaluator.bindFunctionToClassOrObject,
getCallSignatureInfo: (n, i, a) =>
run('getCallSignatureInfo', () => typeEvaluator.getCallSignatureInfo(n, i, a), n),
getTypeAnnotationForParameter: (n, p) =>

40
packages/pyright-internal/src/analyzer/types.ts
View File

@ -1283,6 +1283,17 @@ export namespace FunctionType {
return newFunction;
}
export function cloneWithNewFlags(type: FunctionType, flags: FunctionTypeFlags) {
const newFunction = { ...type };
// Make a shallow clone of the details.
newFunction.details = { ...type.details };
newFunction.details.flags = flags;
return newFunction;
}
export function cloneForParamSpecApplication(type: FunctionType, paramSpecValue: ParamSpecValue) {
const newFunction = { ...type };
@ -1375,6 +1386,35 @@ export namespace FunctionType {
});
}
// Indicates whether the input signature consists of (*args: Any, **kwargs: Any).
export function hasDefaultParameters(functionType: FunctionType) {
let sawArgs = false;
let sawKwargs = false;
for (let i = 0; i < functionType.details.parameters.length; i++) {
const param = functionType.details.parameters[i];
// Ignore nameless separator parameters.
if (!param.name) {
continue;
}
if (param.category === ParameterCategory.Simple) {
return false;
} else if (param.category === ParameterCategory.VarArgList) {
sawArgs = true;
} else if (param.category === ParameterCategory.VarArgDictionary) {
sawKwargs = true;
}
if (!isAnyOrUnknown(FunctionType.getEffectiveParameterType(functionType, i))) {
return false;
}
}
return sawArgs && sawKwargs;
}
export function isInstanceMethod(type: FunctionType): boolean {
return (
(type.details.flags &

14
packages/pyright-internal/src/common/configOptions.ts
View File

@ -188,6 +188,9 @@ export interface DiagnosticRuleSet {
// the base class symbol of the same name?
reportIncompatibleVariableOverride: DiagnosticLevel;
// Report inconsistencies between __init__ and __new__ signatures.
reportInconsistentConstructor: DiagnosticLevel;
// Report function overloads that overlap in signature but have
// incompatible return types.
reportOverlappingOverload: DiagnosticLevel;
@ -327,6 +330,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportConstantRedefinition,
DiagnosticRule.reportIncompatibleMethodOverride,
DiagnosticRule.reportIncompatibleVariableOverride,
DiagnosticRule.reportInconsistentConstructor,
DiagnosticRule.reportOverlappingOverload,
DiagnosticRule.reportUninitializedInstanceVariable,
DiagnosticRule.reportInvalidStringEscapeSequence,
@ -403,6 +407,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportConstantRedefinition: 'none',
reportIncompatibleMethodOverride: 'none',
reportIncompatibleVariableOverride: 'none',
reportInconsistentConstructor: 'none',
reportOverlappingOverload: 'none',
reportUninitializedInstanceVariable: 'none',
reportInvalidStringEscapeSequence: 'none',
@ -475,6 +480,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportConstantRedefinition: 'none',
reportIncompatibleMethodOverride: 'none',
reportIncompatibleVariableOverride: 'none',
reportInconsistentConstructor: 'none',
reportOverlappingOverload: 'none',
reportUninitializedInstanceVariable: 'none',
reportInvalidStringEscapeSequence: 'warning',
@ -547,6 +553,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportConstantRedefinition: 'error',
reportIncompatibleMethodOverride: 'error',
reportIncompatibleVariableOverride: 'error',
reportInconsistentConstructor: 'error',
reportOverlappingOverload: 'error',
reportUninitializedInstanceVariable: 'none',
reportInvalidStringEscapeSequence: 'error',
@ -1122,6 +1129,13 @@ export class ConfigOptions {
defaultSettings.reportIncompatibleVariableOverride
),
// Read the "reportInconsistentConstructor" entry.
reportInconsistentConstructor: this._convertDiagnosticLevel(
configObj.reportInconsistentConstructor,
DiagnosticRule.reportInconsistentConstructor,
defaultSettings.reportInconsistentConstructor
),
// Read the "reportOverlappingOverload" entry.
reportOverlappingOverload: this._convertDiagnosticLevel(
configObj.reportOverlappingOverload,

1
packages/pyright-internal/src/common/diagnosticRules.ts
View File

@ -46,6 +46,7 @@ export enum DiagnosticRule {
reportConstantRedefinition = 'reportConstantRedefinition',
reportIncompatibleMethodOverride = 'reportIncompatibleMethodOverride',
reportIncompatibleVariableOverride = 'reportIncompatibleVariableOverride',
reportInconsistentConstructor = 'reportInconsistentConstructor',
reportOverlappingOverload = 'reportOverlappingOverload',
reportUninitializedInstanceVariable = 'reportUninitializedInstanceVariable',
reportInvalidStringEscapeSequence = 'reportInvalidStringEscapeSequence',

10
packages/pyright-internal/src/localization/localize.ts
View File

@ -286,6 +286,8 @@ export namespace Localizer {
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.constantRedefinition'));
export const constructorNoArgs = () =>
new ParameterizedString<{ type: string }>(getRawString('Diagnostic.constructorNoArgs'));
export const constructorParametersMismatch = () =>
new ParameterizedString<{ classType: string }>(getRawString('Diagnostic.constructorParametersMismatch'));
export const continueInFinally = () => getRawString('Diagnostic.continueInFinally');
export const continueOutsideLoop = () => getRawString('Diagnostic.continueOutsideLoop');
export const dataClassBaseClassNotFrozen = () => getRawString('Diagnostic.dataClassBaseClassNotFrozen');
@ -937,6 +939,10 @@ export namespace Localizer {
export const incompatibleGetter = () => getRawString('DiagnosticAddendum.incompatibleGetter');
export const incompatibleSetter = () => getRawString('DiagnosticAddendum.incompatibleSetter');
export const incompatibleDeleter = () => getRawString('DiagnosticAddendum.incompatibleDeleter');
export const initMethodLocation = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.initMethodLocation'));
export const initMethodSignature = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.initMethodSignature'));
export const functionTooManyParams = () =>
new ParameterizedString<{ expected: number; received: number }>(
getRawString('DiagnosticAddendum.functionTooManyParams')
@ -986,6 +992,10 @@ export namespace Localizer {
new ParameterizedString<{ name: string; sourceType: string; destType: string }>(
getRawString('DiagnosticAddendum.namedParamTypeMismatch')
);
export const newMethodLocation = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.newMethodLocation'));
export const newMethodSignature = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.newMethodSignature'));
export const noOverloadAssignable = () =>
new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.noOverloadAssignable'));
export const orPatternMissingName = () =>

5
packages/pyright-internal/src/localization/package.nls.en-us.json
View File

@ -69,6 +69,7 @@
"concatenateParamSpecMissing": "Last type argument for \"Concatenate\" must be a ParamSpec",
"concatenateTypeArgsMissing": "\"Concatenate\" requires at least two type arguments",
"constantRedefinition": "\"{name}\" is constant (because it is uppercase) and cannot be redefined",
"constructorParametersMismatch": "Mismatch between signature of __new__ and __init__ in class \"{classType}\"",
"continueInFinally": "\"continue\" cannot be used within a finally clause",
"continueOutsideLoop": "\"continue\" can be used only within a loop",
"constructorNoArgs": "Expected no arguments to \"{type}\" constructor",
@ -489,7 +490,9 @@
"functionTooManyParams": "Function accepts too many positional parameters; expected {expected} but received {received}",
"incompatibleGetter": "Property getter method is incompatible",
"incompatibleSetter": "Property setter method is incompatible",
"initMethodLocation": "The __init__ method is defined in class \"{type}\"",
"incompatibleDeleter": "Property deleter method is incompatible",
"initMethodSignature": "Signature of __init__ is \"{type}\"",
"kwargsParamMissing": "Parameter \"**{paramName}\" has no corresponding parameter",
"literalAssignmentMismatch": "\"{sourceType}\" cannot be assigned to type \"{destType}\"",
"memberSetClassVar": "Member \"{name}\" cannot be assigned through a class instance because it is a ClassVar",
@ -507,6 +510,8 @@
"namedParamMissingInDest": "Keyword parameter \"{name}\" is missing in destination",
"namedParamMissingInSource": "Keyword parameter \"{name}\" is missing in source",
"namedParamTypeMismatch": "Keyword parameter \"{name}\" of type \"{sourceType}\" cannot be assigned to type \"{destType}\"",
"newMethodLocation": "The __new__ method is defined in class \"{type}\"",
"newMethodSignature": "Signature of __new__ is \"{type}\"",
"noOverloadAssignable": "No overloaded function matches type \"{type}\"",
"orPatternMissingName": "Missing names: {name}",
"overloadMethod": "Overload method is defined here",

23
packages/pyright-internal/src/tests/samples/inconsistentConstructor1.py Normal file
View File

@ -0,0 +1,23 @@
# This sample tests the reportInconsistentConstructor diagnostic check.
class Parent1:
def __init__(self, a: int) -> None:
...
class Child1(Parent1):
# This should generate an error if reportInconsistentConstructor is enabled.
def __new__(cls, a: int | str):
...
class Parent2:
def __init__(self, b: int) -> None:
...
class Child2(Parent2):
# This should generate an error if reportInconsistentConstructor is enabled.
def __new__(cls, b: str):
...

13
packages/pyright-internal/src/tests/typeEvaluator3.test.ts
View File

@ -869,6 +869,19 @@ test('Constructor11', () => {
TestUtils.validateResults(analysisResults, 0);
});
test('InconsistentConstructor1', () => {
const configOptions = new ConfigOptions('.');
configOptions.diagnosticRuleSet.reportInconsistentConstructor = 'none';
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['inconsistentConstructor1.py'], configOptions);
TestUtils.validateResults(analysisResults, 0);
// Enable it as an error.
configOptions.diagnosticRuleSet.reportInconsistentConstructor = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['inconsistentConstructor1.py'], configOptions);
TestUtils.validateResults(analysisResults, 2);
});
test('ClassGetItem1', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['classGetItem1.py']);

11
packages/vscode-pyright/package.json
View File

@ -432,6 +432,17 @@
"error"
]
},
"reportInconsistentConstructor": {
"type": "string",
"description": "Diagnostics for __init__ and __new__ methods whose signatures are inconsistent.",
"default": "none",
"enum": [
"none",
"information",
"warning",
"error"
]
},
"reportOverlappingOverload": {
"type": "string",
"description": "Diagnostics for function overloads that overlap in signature and obscure each other or have incompatible return types.",

6
packages/vscode-pyright/schemas/pyrightconfig.schema.json
View File

@ -305,6 +305,12 @@
"title": "Controls reporting of overrides in subclasses that redefine a variable in an incompatible way",
"default": "none"
},
"reportInconsistentConstructor": {
"$id": "#/properties/reportInconsistentConstructor",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of __init__ and __new__ methods whose signatures are inconsistent",
"default": "none"
},
"reportOverlappingOverload": {
"$id": "#/properties/reportOverlappingOverload",
"$ref": "#/definitions/diagnostic",