Added a new diagnostic rule called "reportInvalidTypeVarUse" that flags errors when TypeVars are used incorrectly. In particular, it flags the use of a single instance of a TypeVar within a generic function signature.

This commit is contained in:
Eric Traut 2020-12-11 22:28:19 -08:00
parent ab5f003961
commit b1d9ab7002
9 changed files with 155 additions and 0 deletions

View File

@ -114,6 +114,8 @@ The following settings control pyrights diagnostic output (warnings or errors
**reportMissingTypeArgument** [boolean or string, optional]: Generate or suppress diagnostics when a generic class is used without providing explicit or implicit type arguments. The default value for this setting is 'none'.
**reportInvalidTypeVarUse** [boolean or string, optional]: Generate or suppress diagnostics when a TypeVar is used inappropriately (e.g. if a TypeVar appears only once) within a generic function signature. The default value for this setting is 'none'.
**reportCallInDefaultInitializer** [boolean or string, optional]: Generate or suppress diagnostics for function calls within a default value initialization expression. Such calls can mask expensive operations that are performed at module initialization time. The default value for this setting is 'none'.
**reportUnnecessaryIsInstance** [boolean or string, optional]: Generate or suppress diagnostics for 'isinstance' or 'issubclass' calls where the result is statically determined to be always true or always false. Such calls are often indicative of a programming error. The default value for this setting is 'none'.

View File

@ -348,6 +348,8 @@ export class Checker extends ParseTreeWalker {
this._scopedNodes.push(node);
this._validateFunctionTypeVarUsage(node);
if (functionTypeResult && functionTypeResult.decoratedType.category === TypeCategory.OverloadedFunction) {
const overloads = functionTypeResult.decoratedType.overloads;
if (overloads.length > 1) {
@ -816,6 +818,55 @@ export class Checker extends ParseTreeWalker {
return false;
}
// Verifies that each local type variable is used more than once.
private _validateFunctionTypeVarUsage(node: FunctionNode) {
// Skip this check entirely if it's disabled.
if (this._fileInfo.diagnosticRuleSet.reportInvalidTypeVarUse === 'none') {
return;
}
const localTypeVarUsage = new Map<string, NameNode[]>();
const nameWalker = new ParseTreeUtils.NameNodeWalker((nameNode) => {
const nameType = this._evaluator.getType(nameNode);
if (nameType && isTypeVar(nameType)) {
if (nameType.scopeId === this._evaluator.getScopeIdForNode(node)) {
if (!localTypeVarUsage.has(nameType.details.name)) {
localTypeVarUsage.set(nameType.details.name, [nameNode]);
} else {
localTypeVarUsage.get(nameType.details.name)!.push(nameNode);
}
}
}
});
// Find all of the local type variables in signature.
node.parameters.forEach((param) => {
const annotation = param.typeAnnotation || param.typeAnnotationComment;
if (annotation) {
nameWalker.walk(annotation);
}
});
if (node.returnTypeAnnotation) {
nameWalker.walk(node.returnTypeAnnotation);
}
// Report errors for all local type variables that appear only once.
localTypeVarUsage.forEach((nameNodes) => {
if (nameNodes.length === 1) {
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticRuleSet.reportInvalidTypeVarUse,
DiagnosticRule.reportInvalidTypeVarUse,
Localizer.Diagnostic.typeVarUsedOnlyOnce().format({
name: nameNodes[0].value,
}),
nameNodes[0]
);
}
});
}
private _validateOverloadConsistency(
node: FunctionNode,
functionType: FunctionType,

View File

@ -23,6 +23,7 @@ import {
isExpressionNode,
LambdaNode,
ModuleNode,
NameNode,
ParameterCategory,
ParseNode,
ParseNodeType,
@ -1079,3 +1080,16 @@ export function isAssignmentToDefaultsFollowingNamedTuple(callNode: ParseNode):
return false;
}
// This simple parse tree walker calls a callback function
// for each NameNode it encounters.
export class NameNodeWalker extends ParseTreeWalker {
constructor(private _callback: (node: NameNode) => void) {
super();
}
visitName(node: NameNode) {
this._callback(node);
return true;
}
}

View File

@ -195,6 +195,9 @@ export interface DiagnosticRuleSet {
// Report usage of generic class without explicit type arguments?
reportMissingTypeArgument: DiagnosticLevel;
// Report improper usage of type variables within function signatures?
reportInvalidTypeVarUse: DiagnosticLevel;
// Report usage of function call within default value
// initialization expression?
reportCallInDefaultInitializer: DiagnosticLevel;
@ -289,6 +292,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportUnknownVariableType,
DiagnosticRule.reportUnknownMemberType,
DiagnosticRule.reportMissingTypeArgument,
DiagnosticRule.reportInvalidTypeVarUse,
DiagnosticRule.reportCallInDefaultInitializer,
DiagnosticRule.reportUnnecessaryIsInstance,
DiagnosticRule.reportUnnecessaryCast,
@ -353,6 +357,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownVariableType: 'none',
reportUnknownMemberType: 'none',
reportMissingTypeArgument: 'none',
reportInvalidTypeVarUse: 'none',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none',
@ -413,6 +418,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownVariableType: 'none',
reportUnknownMemberType: 'none',
reportMissingTypeArgument: 'none',
reportInvalidTypeVarUse: 'warning',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'none',
reportUnnecessaryCast: 'none',
@ -473,6 +479,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnknownVariableType: 'error',
reportUnknownMemberType: 'error',
reportMissingTypeArgument: 'error',
reportInvalidTypeVarUse: 'error',
reportCallInDefaultInitializer: 'none',
reportUnnecessaryIsInstance: 'error',
reportUnnecessaryCast: 'error',
@ -1050,6 +1057,13 @@ export class ConfigOptions {
defaultSettings.reportMissingTypeArgument
),
// Read the "reportInvalidTypeVarUse" entry.
reportInvalidTypeVarUse: this._convertDiagnosticLevel(
configObj.reportInvalidTypeVarUse,
DiagnosticRule.reportInvalidTypeVarUse,
defaultSettings.reportInvalidTypeVarUse
),
// Read the "reportCallInDefaultInitializer" entry.
reportCallInDefaultInitializer: this._convertDiagnosticLevel(
configObj.reportCallInDefaultInitializer,

View File

@ -50,6 +50,7 @@ export enum DiagnosticRule {
reportUnknownVariableType = 'reportUnknownVariableType',
reportUnknownMemberType = 'reportUnknownMemberType',
reportMissingTypeArgument = 'reportMissingTypeArgument',
reportInvalidTypeVarUse = 'reportInvalidTypeVarUse',
reportCallInDefaultInitializer = 'reportCallInDefaultInitializer',
reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance',
reportUnnecessaryCast = 'reportUnnecessaryCast',

View File

@ -0,0 +1,50 @@
# This sample tests the reporting of incorrect TypeVar usage within
# a generic function. A TypeVar must appear at least twice to be
# considered legitimate.
# pyright: reportInvalidTypeVarUse=true
from typing import Dict, Generic, List, TypeVar
_T = TypeVar("_T")
_S = TypeVar("_S")
class Foo(Generic[_T]):
def m1(self, v1: _T) -> None:
...
# This should generate an error because _S
# is a local typeVar and appears only once.
def m2(self, v1: _S) -> None:
...
# This should generate an error because _S
# is a local typeVar and appears only once.
def m3(self, v1: _T) -> _S:
...
# This should generate an error because _T
# is a local typeVar and appears only once.
def f1(self, v1: _T) -> None:
...
def f2(self, v1: _T, v2: List[_T]) -> None:
...
def f3(self, v1: _T) -> _T:
...
def f4(self) -> Dict[_T, _T]:
...
# This should generate an error because _T
# is a local typeVar and appears only once.
def f5(self) -> List[_T]:
...

View File

@ -1144,6 +1144,12 @@ test('TypeVar8', () => {
TestUtils.validateResults(analysisResults, 2);
});
test('TypeVar9', () => {
const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeVar9.py']);
TestUtils.validateResults(analysisResults, 4);
});
test('Annotated1', () => {
const configOptions = new ConfigOptions('.');

View File

@ -488,6 +488,17 @@
"error"
]
},
"reportInvalidTypeVarUse": {
"type": "string",
"description": "Diagnostics for improper use of type variables in a function signature.",
"default": "warning",
"enum": [
"none",
"information",
"warning",
"error"
]
},
"reportCallInDefaultInitializer": {
"type": "string",
"description": "Diagnostics for function calls within a default value initialization expression. Such calls can mask expensive operations that are performed at module initialization time.",

View File

@ -327,6 +327,12 @@
"title": "Controls reporting generic class reference with missing type arguments",
"default": "none"
},
"reportInvalidTypeVarUse": {
"$id": "#/properties/reportInvalidTypeVarUse",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting improper use of type variables within function signatures",
"default": "warning"
},
"reportCallInDefaultInitializer": {
"$id": "#/properties/reportCallInDefaultInitializer",
"$ref": "#/definitions/diagnostic",