diff --git a/docs/configuration.md b/docs/configuration.md index 285de463a..75b326534 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -114,6 +114,8 @@ The following settings control pyright’s 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'. diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index d426b3564..2bdaf7887 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -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(); + + 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, diff --git a/packages/pyright-internal/src/analyzer/parseTreeUtils.ts b/packages/pyright-internal/src/analyzer/parseTreeUtils.ts index 990467112..3e404823b 100644 --- a/packages/pyright-internal/src/analyzer/parseTreeUtils.ts +++ b/packages/pyright-internal/src/analyzer/parseTreeUtils.ts @@ -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; + } +} diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index eebee82ab..df0fe2e72 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -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, diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 5f976d403..c396411a8 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -50,6 +50,7 @@ export enum DiagnosticRule { reportUnknownVariableType = 'reportUnknownVariableType', reportUnknownMemberType = 'reportUnknownMemberType', reportMissingTypeArgument = 'reportMissingTypeArgument', + reportInvalidTypeVarUse = 'reportInvalidTypeVarUse', reportCallInDefaultInitializer = 'reportCallInDefaultInitializer', reportUnnecessaryIsInstance = 'reportUnnecessaryIsInstance', reportUnnecessaryCast = 'reportUnnecessaryCast', diff --git a/packages/pyright-internal/src/tests/samples/typeVar9.py b/packages/pyright-internal/src/tests/samples/typeVar9.py new file mode 100644 index 000000000..be2ce68eb --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/typeVar9.py @@ -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]: + ... diff --git a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts index 1fa34cd22..f19ea9325 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts @@ -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('.'); diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index f59a8cdcd..5d5acadeb 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -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.", diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index a9878c0f8..de42fcb3a 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -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",