mirror of
https://github.com/microsoft/pyright.git
synced 2024-09-11 07:55:56 +03:00
Added new diagnostic check reportImplicitOverride
. This addresses https://github.com/microsoft/pyright/issues/4788.
This commit is contained in:
parent
c7ab8045ba
commit
e2e3a5c9ff
@ -180,6 +180,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
|
||||
|
||||
<a name="reportMatchNotExhaustive"></a> **reportMatchNotExhaustive** [boolean or string, optional]: Generate or suppress diagnostics for a `match` statement that does not provide cases that exhaustively match against all potential types of the target expression. The default value for this setting is `"none"`.
|
||||
|
||||
<a name="reportImplicitOverride"></a> **reportImplicitOverride** [boolean or string, optional]: Generate or suppress diagnostics for overridden methods in a class that are missing an explicit `@override` decorator.
|
||||
|
||||
<a name="reportShadowedImports"></a> **reportShadowedImports** [boolean or string, optional]: Generate or suppress diagnostics for files that are overriding a module in the stdlib. The default value for this setting is `"none"`.
|
||||
|
||||
## Execution Environment Options
|
||||
@ -337,7 +339,6 @@ The following table lists the default severity levels for each diagnostic rule w
|
||||
| reportMissingTypeArgument | "none" | "none" | "error" |
|
||||
| reportOverlappingOverload | "none" | "none" | "error" |
|
||||
| reportPrivateUsage | "none" | "none" | "error" |
|
||||
| reportShadowedImports | "none" | "none" | "none" |
|
||||
| reportTypeCommentUsage | "none" | "none" | "error" |
|
||||
| reportUnknownArgumentType | "none" | "none" | "error" |
|
||||
| reportUnknownLambdaType | "none" | "none" | "error" |
|
||||
@ -357,9 +358,11 @@ The following table lists the default severity levels for each diagnostic rule w
|
||||
| reportUntypedFunctionDecorator | "none" | "none" | "error" |
|
||||
| reportUntypedNamedTuple | "none" | "none" | "error" |
|
||||
| reportCallInDefaultInitializer | "none" | "none" | "none" |
|
||||
| reportImplicitOverride | "none" | "none" | "none" |
|
||||
| reportImplicitStringConcatenation | "none" | "none" | "none" |
|
||||
| reportMissingSuperCall | "none" | "none" | "none" |
|
||||
| reportPropertyTypeMismatch | "none" | "none" | "none" |
|
||||
| reportShadowedImports | "none" | "none" | "none" |
|
||||
| reportUninitializedInstanceVariable | "none" | "none" | "none" |
|
||||
| reportUnnecessaryTypeIgnoreComment | "none" | "none" | "none" |
|
||||
| reportUnusedCallResult | "none" | "none" | "none" |
|
||||
|
@ -5258,7 +5258,7 @@ export class Checker extends ParseTreeWalker {
|
||||
return;
|
||||
}
|
||||
|
||||
let foundOverride = false;
|
||||
let firstOverride: ClassMember | undefined;
|
||||
|
||||
for (const baseClass of classType.details.baseClasses) {
|
||||
if (!isClass(baseClass)) {
|
||||
@ -5279,22 +5279,27 @@ export class Checker extends ParseTreeWalker {
|
||||
continue;
|
||||
}
|
||||
|
||||
foundOverride = true;
|
||||
firstOverride = firstOverride ?? baseClassAndSymbol;
|
||||
|
||||
this._validateBaseClassOverride(baseClassAndSymbol, symbol, typeOfSymbol, classType, name);
|
||||
}
|
||||
|
||||
if (!foundOverride) {
|
||||
if (!firstOverride) {
|
||||
// If this is a method decorated with @override, validate that there
|
||||
// is a base class method of the same name.
|
||||
this._validateOverrideDecorator(typeOfSymbol);
|
||||
this._validateOverrideDecoratorNotPresent(typeOfSymbol);
|
||||
} else {
|
||||
this._validateOverrideDecoratorPresent(typeOfSymbol, firstOverride);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Determines whether the type is a function or overloaded function with an @override
|
||||
// decorator. In this case, an error is reported because no base class has declared
|
||||
// a method of the same name.
|
||||
private _validateOverrideDecorator(overrideType: Type) {
|
||||
private _validateOverrideDecoratorPresent(overrideType: Type, baseMember: ClassMember) {
|
||||
// Skip this check if disabled.
|
||||
if (this._fileInfo.diagnosticRuleSet.reportImplicitOverride === 'none') {
|
||||
return;
|
||||
}
|
||||
|
||||
let overrideFunction: FunctionType | undefined;
|
||||
|
||||
if (isFunction(overrideType)) {
|
||||
@ -5303,11 +5308,7 @@ export class Checker extends ParseTreeWalker {
|
||||
overrideFunction = OverloadedFunctionType.getImplementation(overrideType);
|
||||
}
|
||||
|
||||
if (
|
||||
!overrideFunction ||
|
||||
!FunctionType.isOverridden(overrideFunction) ||
|
||||
!overrideFunction.details.declaration
|
||||
) {
|
||||
if (!overrideFunction?.details.declaration || FunctionType.isOverridden(overrideFunction)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@ -5315,7 +5316,35 @@ export class Checker extends ParseTreeWalker {
|
||||
this._evaluator.addDiagnostic(
|
||||
this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
|
||||
DiagnosticRule.reportGeneralTypeIssues,
|
||||
Localizer.Diagnostic.overrideNotFound().format({ name: funcNode.name.value }),
|
||||
Localizer.Diagnostic.overrideDecoratorMissing().format({
|
||||
name: funcNode.name.value,
|
||||
className: this._evaluator.printType(convertToInstance(baseMember.classType)),
|
||||
}),
|
||||
funcNode.name
|
||||
);
|
||||
}
|
||||
|
||||
// Determines whether the type is a function or overloaded function with an @override
|
||||
// decorator. In this case, an error is reported because no base class has declared
|
||||
// a method of the same name.
|
||||
private _validateOverrideDecoratorNotPresent(overrideType: Type) {
|
||||
let overrideFunction: FunctionType | undefined;
|
||||
|
||||
if (isFunction(overrideType)) {
|
||||
overrideFunction = overrideType;
|
||||
} else if (isOverloadedFunction(overrideType)) {
|
||||
overrideFunction = OverloadedFunctionType.getImplementation(overrideType);
|
||||
}
|
||||
|
||||
if (!overrideFunction?.details.declaration || !FunctionType.isOverridden(overrideFunction)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const funcNode = overrideFunction.details.declaration.node;
|
||||
this._evaluator.addDiagnostic(
|
||||
this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
|
||||
DiagnosticRule.reportGeneralTypeIssues,
|
||||
Localizer.Diagnostic.overriddenMethodNotFound().format({ name: funcNode.name.value }),
|
||||
funcNode.name
|
||||
);
|
||||
}
|
||||
|
@ -310,6 +310,9 @@ export interface DiagnosticRuleSet {
|
||||
|
||||
// Report files that match stdlib modules.
|
||||
reportShadowedImports: DiagnosticLevel;
|
||||
|
||||
// Report missing @override decorator.
|
||||
reportImplicitOverride: DiagnosticLevel;
|
||||
}
|
||||
|
||||
export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
|
||||
@ -405,6 +408,7 @@ export function getDiagLevelDiagnosticRules() {
|
||||
DiagnosticRule.reportUnnecessaryTypeIgnoreComment,
|
||||
DiagnosticRule.reportMatchNotExhaustive,
|
||||
DiagnosticRule.reportShadowedImports,
|
||||
DiagnosticRule.reportImplicitOverride,
|
||||
];
|
||||
}
|
||||
|
||||
@ -490,6 +494,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
|
||||
reportUnnecessaryTypeIgnoreComment: 'none',
|
||||
reportMatchNotExhaustive: 'none',
|
||||
reportShadowedImports: 'none',
|
||||
reportImplicitOverride: 'none',
|
||||
};
|
||||
|
||||
return diagSettings;
|
||||
@ -571,6 +576,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
|
||||
reportUnnecessaryTypeIgnoreComment: 'none',
|
||||
reportMatchNotExhaustive: 'none',
|
||||
reportShadowedImports: 'none',
|
||||
reportImplicitOverride: 'none',
|
||||
};
|
||||
|
||||
return diagSettings;
|
||||
@ -652,6 +658,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
|
||||
reportUnnecessaryTypeIgnoreComment: 'none',
|
||||
reportMatchNotExhaustive: 'error',
|
||||
reportShadowedImports: 'none',
|
||||
reportImplicitOverride: 'none',
|
||||
};
|
||||
|
||||
return diagSettings;
|
||||
|
@ -81,4 +81,5 @@ export enum DiagnosticRule {
|
||||
reportUnnecessaryTypeIgnoreComment = 'reportUnnecessaryTypeIgnoreComment',
|
||||
reportMatchNotExhaustive = 'reportMatchNotExhaustive',
|
||||
reportShadowedImports = 'reportShadowedImports',
|
||||
reportImplicitOverride = 'reportImplicitOverride',
|
||||
}
|
||||
|
@ -629,8 +629,12 @@ export namespace Localizer {
|
||||
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overloadWithImplementation'));
|
||||
export const overloadWithoutImplementation = () =>
|
||||
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overloadWithoutImplementation'));
|
||||
export const overrideNotFound = () =>
|
||||
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overrideNotFound'));
|
||||
export const overriddenMethodNotFound = () =>
|
||||
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.overriddenMethodNotFound'));
|
||||
export const overrideDecoratorMissing = () =>
|
||||
new ParameterizedString<{ name: string; className: string }>(
|
||||
getRawString('Diagnostic.overrideDecoratorMissing')
|
||||
);
|
||||
export const paramAfterKwargsParam = () => getRawString('Diagnostic.paramAfterKwargsParam');
|
||||
export const paramAlreadyAssigned = () =>
|
||||
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.paramAlreadyAssigned'));
|
||||
|
@ -303,7 +303,8 @@
|
||||
"overloadReturnTypeMismatch": "Overload {prevIndex} for \"{name}\" overlaps overload {newIndex} and returns an incompatible type",
|
||||
"overloadWithImplementation": "\"{name}\" is marked as overload, but it includes an implementation",
|
||||
"overloadWithoutImplementation": "\"{name}\" is marked as overload, but no implementation is provided",
|
||||
"overrideNotFound": "Method \"{name}\" is marked as override, but no base method of same name is present",
|
||||
"overriddenMethodNotFound": "Method \"{name}\" is marked as override, but no base method of same name is present",
|
||||
"overrideDecoratorMissing": "Method \"{name}\" is not marked as override but is overriding a method in class \"{className}\"",
|
||||
"paramAfterKwargsParam": "Parameter cannot follow \"**\" parameter",
|
||||
"paramAlreadyAssigned": "Parameter \"{name}\" is already assigned",
|
||||
"paramAnnotationMissing": "Type annotation is missing for parameter \"{name}\"",
|
||||
|
23
packages/pyright-internal/src/tests/samples/override2.py
Normal file
23
packages/pyright-internal/src/tests/samples/override2.py
Normal file
@ -0,0 +1,23 @@
|
||||
# This sample tests the reportImplicitOverride diagnostic check
|
||||
# (strict enforcement of PEP 698).
|
||||
|
||||
from typing_extensions import override
|
||||
|
||||
|
||||
class Base:
|
||||
@override
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
def method1(self):
|
||||
pass
|
||||
|
||||
|
||||
class Child(Base):
|
||||
# This should generate an error if reportImplicitOverride is enabled.
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
# This should generate an error if reportImplicitOverride is enabled.
|
||||
def method1(self):
|
||||
pass
|
@ -150,6 +150,22 @@ test('Override1', () => {
|
||||
TestUtils.validateResults(analysisResults, 2);
|
||||
});
|
||||
|
||||
test('Override1', () => {
|
||||
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['override1.py']);
|
||||
TestUtils.validateResults(analysisResults, 2);
|
||||
});
|
||||
|
||||
test('Override2', () => {
|
||||
const configOptions = new ConfigOptions('.');
|
||||
|
||||
const analysisResults1 = TestUtils.typeAnalyzeSampleFiles(['override2.py'], configOptions);
|
||||
TestUtils.validateResults(analysisResults1, 0);
|
||||
|
||||
configOptions.diagnosticRuleSet.reportImplicitOverride = 'error';
|
||||
const analysisResults2 = TestUtils.typeAnalyzeSampleFiles(['override2.py'], configOptions);
|
||||
TestUtils.validateResults(analysisResults2, 2);
|
||||
});
|
||||
|
||||
test('TypeVarDefault1', () => {
|
||||
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeVarDefault1.py']);
|
||||
TestUtils.validateResults(analysisResults, 12);
|
||||
|
@ -856,6 +856,17 @@
|
||||
"warning",
|
||||
"error"
|
||||
]
|
||||
},
|
||||
"reportImplicitOverride": {
|
||||
"type": "string",
|
||||
"description": "Diagnostics for overridden methods that do not include an `@override` decorator.",
|
||||
"default": "none",
|
||||
"enum": [
|
||||
"none",
|
||||
"information",
|
||||
"warning",
|
||||
"error"
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
|
@ -526,6 +526,12 @@
|
||||
"title": "Controls reporting of shadowed imports of stdlib modules",
|
||||
"default": "none"
|
||||
},
|
||||
"reportImplicitOverride": {
|
||||
"$id": "#/properties/reportImplicitOverride",
|
||||
"$ref": "#/definitions/diagnostic",
|
||||
"title": "Controls reporting overridden methods that are missing an `@override` decorator",
|
||||
"default": "none"
|
||||
},
|
||||
"extraPaths": {
|
||||
"$id": "#/properties/extraPaths",
|
||||
"type": "array",
|
||||
|
Loading…
Reference in New Issue
Block a user