Added reportSelfClsParameterName setting and defaulted it to warning.

This commit is contained in:
Eric Traut 2019-12-24 13:30:57 -07:00
parent f689312051
commit d87356c076
5 changed files with 72 additions and 23 deletions

View File

@ -84,6 +84,18 @@
"title": "Infer strict types for dictionary expressions",
"default": "false"
},
"strictParameterNoneValue": {
"$id": "#/properties/strictParameterNoneValue",
"type": "boolean",
"title": "Allow implicit Optional when default parameter value is None",
"default": "false"
},
"enableTypeIgnoreComments": {
"$id": "#/properties/enableTypeIgnoreComments",
"type": "boolean",
"title": "Allow “# type: ignore” comments",
"default": "true"
},
"reportTypeshedErrors": {
"$id": "#/properties/reportTypeshedErrors",
"$ref": "#/definitions/diagnostic",
@ -220,7 +232,7 @@
"$id": "#/properties/reportInvalidStringEscapeSequence",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of invalid escape sequences used within string literals",
"default": "none"
"default": "warning"
},
"reportUnknownParameterType": {
"$id": "#/properties/reportUnknownParameterType",
@ -268,7 +280,13 @@
"$id": "#/properties/reportAssertAlwaysTrue",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting assert expressions that will always evaluate to true",
"default": "none"
"default": "warning"
},
"reportSelfClsParameterName": {
"$id": "#/properties/reportSelfClsParameterName",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting assert expressions that will always evaluate to true",
"default": "warning"
},
"pythonVersion": {
"$id": "#/properties/pythonVersion",

View File

@ -102,6 +102,8 @@ The following settings control pyrights diagnostic output (warnings or errors
**reportAssertAlwaysTrue** [boolean or string, optional]: Generate or suppress diagnostics for 'assert' statement that will provably always assert. This can be indicative of a programming error. The default value for this setting is 'warning'.
**reportSelfClsParameterName** [boolean or string, optional]: Generate or suppress diagnostics for a missing or misnamed “self” parameter in instance methods and “cls” parameter in class methods. Instance methods in metaclasses (classes that derive from “type”) are allowed to use “cls” for instance methods. The default value for this setting is 'warning'.
## Execution Environment Options
Pyright allows multiple “execution environments” to be defined for different portions of your source tree. For example, a subtree may be designed to run with different import search paths or a different version of the python interpreter than the rest of the source base.

View File

@ -117,7 +117,7 @@ export class Checker extends ParseTreeWalker {
});
if (containingClassNode) {
this._validateMethod(node, functionTypeResult.functionType);
this._validateMethod(node, functionTypeResult.functionType, containingClassNode);
}
}
@ -1283,13 +1283,15 @@ export class Checker extends ParseTreeWalker {
// 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) {
private _validateMethod(node: FunctionNode, functionType: FunctionType, classNode: ClassNode) {
if (node.name && node.name.value === '__new__') {
// __new__ overrides should have a "cls" parameter.
if (node.parameters.length === 0 || !node.parameters[0].name ||
(node.parameters[0].name.value !== 'cls' &&
node.parameters[0].name.value !== 'mcs')) {
this._evaluator.addError(
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticSettings.reportSelfClsParameterName,
DiagnosticRule.reportSelfClsParameterName,
`The __new__ override should take a 'cls' parameter`,
node.parameters.length > 0 ? node.parameters[0] : node.name);
}
@ -1297,7 +1299,9 @@ export class Checker extends ParseTreeWalker {
// __init_subclass__ overrides should have a "cls" parameter.
if (node.parameters.length === 0 || !node.parameters[0].name ||
node.parameters[0].name.value !== 'cls') {
this._evaluator.addError(
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticSettings.reportSelfClsParameterName,
DiagnosticRule.reportSelfClsParameterName,
`The __init_subclass__ override should take a 'cls' parameter`,
node.parameters.length > 0 ? node.parameters[0] : node.name);
}
@ -1306,7 +1310,9 @@ export class Checker extends ParseTreeWalker {
if (node.parameters.length > 0 && node.parameters[0].name) {
const paramName = node.parameters[0].name.value;
if (paramName === 'self' || paramName === 'cls') {
this._evaluator.addError(
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticSettings.reportSelfClsParameterName,
DiagnosticRule.reportSelfClsParameterName,
`Static methods should not take a 'self' or 'cls' parameter`,
node.parameters[0].name);
}
@ -1321,7 +1327,9 @@ export class Checker extends ParseTreeWalker {
// cases in the stdlib pyi files.
if (paramName !== 'cls') {
if (!this._fileInfo.isStubFile || (!paramName.startsWith('_') && paramName !== 'metacls')) {
this._evaluator.addError(
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticSettings.reportSelfClsParameterName,
DiagnosticRule.reportSelfClsParameterName,
`Class methods should take a 'cls' parameter`,
node.parameters.length > 0 ? node.parameters[0] : node.name);
}
@ -1342,17 +1350,26 @@ export class Checker extends ParseTreeWalker {
}
}
// Instance methods should have a "self" parameter. We'll exempt parameter
// names that start with an underscore since those are used in a few
// cases in the stdlib pyi files.
if (firstParamIsSimple && paramName !== 'self' && !paramName.startsWith('_')) {
// Special-case the ABCMeta.register method in abc.pyi.
const isRegisterMethod = this._fileInfo.isStubFile &&
paramName === 'cls' &&
node.name.value === 'register';
// Instance methods should have a "self" parameter.
if (firstParamIsSimple && paramName !== 'self') {
// Special-case metaclasses, which can use "cls".
let isLegalMetaclassName = false;
if (paramName === 'cls') {
const classTypeInfo = this._evaluator.getTypeOfClass(classNode);
const typeType = this._evaluator.getBuiltInType(classNode, 'type');
if (typeType && typeType.category === TypeCategory.Class &&
classTypeInfo && classTypeInfo.classType.category === TypeCategory.Class) {
if (!isRegisterMethod) {
this._evaluator.addError(
if (derivesFromClassRecursive(classTypeInfo.classType, typeType)) {
isLegalMetaclassName = true;
}
}
}
if (!isLegalMetaclassName) {
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticSettings.reportSelfClsParameterName,
DiagnosticRule.reportSelfClsParameterName,
`Instance methods should take a 'self' parameter`,
node.parameters.length > 0 ? node.parameters[0] : node.name);
}

View File

@ -152,6 +152,9 @@ export interface DiagnosticSettings {
// Report assert expressions that will always evaluate to true.
reportAssertAlwaysTrue: DiagnosticLevel;
// Report when "self" or "cls" parameter is missing or is misnamed.
reportSelfClsParameterName: DiagnosticLevel;
}
export function cloneDiagnosticSettings(
@ -206,7 +209,8 @@ export function getDiagLevelSettings() {
DiagnosticRule.reportCallInDefaultInitializer,
DiagnosticRule.reportUnnecessaryIsInstance,
DiagnosticRule.reportUnnecessaryCast,
DiagnosticRule.reportAssertAlwaysTrue
DiagnosticRule.reportAssertAlwaysTrue,
DiagnosticRule.reportSelfClsParameterName
];
}
@ -246,7 +250,8 @@ export function getStrictDiagnosticSettings(): DiagnosticSettings {
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'error',
reportUnnecessaryCast: 'error',
reportAssertAlwaysTrue: 'error'
reportAssertAlwaysTrue: 'error',
reportSelfClsParameterName: 'error'
};
return diagSettings;
@ -288,7 +293,8 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings {
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none',
reportAssertAlwaysTrue: 'warning'
reportAssertAlwaysTrue: 'warning',
reportSelfClsParameterName: 'warning'
};
return diagSettings;
@ -649,7 +655,12 @@ export class ConfigOptions {
// Read the "reportAssertAlwaysTrue" entry.
reportAssertAlwaysTrue: this._convertDiagnosticLevel(
configObj.reportAssertAlwaysTrue, DiagnosticRule.reportAssertAlwaysTrue,
defaultSettings.reportAssertAlwaysTrue)
defaultSettings.reportAssertAlwaysTrue),
// Read the "reportSelfClsParameterName" entry.
reportSelfClsParameterName: this._convertDiagnosticLevel(
configObj.reportSelfClsParameterName, DiagnosticRule.reportSelfClsParameterName,
defaultSettings.reportSelfClsParameterName)
};
// Read the "venvPath".

View File

@ -44,5 +44,6 @@ export const enum DiagnosticRule {
reportCallInDefaultInitializer = 'reportCallInDefaultInitializer',
reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance',
reportUnnecessaryCast = 'reportUnnecessaryCast',
reportAssertAlwaysTrue = 'reportAssertAlwaysTrue'
reportAssertAlwaysTrue = 'reportAssertAlwaysTrue',
reportSelfClsParameterName = 'reportSelfClsParameterName'
}