From 04e0536a526e53454c30c4b18087c408a67f0673 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 20 Jan 2024 21:56:10 -0800 Subject: [PATCH] Added new diagnostic rule `reportInvalidTypeForm` that controls reporting of invalid type expression forms. This partly addresses #6973. (#7069) --- docs/configuration.md | 3 +++ .../pyright-internal/src/analyzer/binder.ts | 8 +++---- .../pyright-internal/src/analyzer/checker.ts | 10 ++++---- .../src/analyzer/operations.ts | 10 +++----- .../src/analyzer/typeEvaluator.ts | 24 +++++++------------ .../src/common/configOptions.ts | 8 +++++++ .../src/common/diagnosticRules.ts | 1 + packages/vscode-pyright/package.json | 16 +++++++++++++ .../schemas/pyrightconfig.schema.json | 6 +++++ 9 files changed, 54 insertions(+), 32 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 66db39e56436..280aa8bc6695 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -72,6 +72,8 @@ The following settings control pyright’s diagnostic output (warnings or errors **reportMissingModuleSource** [boolean or string, optional]: Generate or suppress diagnostics for imports that have no corresponding source file. This happens when a type stub is found, but the module source file was not found, indicating that the code may fail at runtime when using this execution environment. Type checking will be done using the type stub. The default value for this setting is `"warning"`. + **reportInvalidTypeForm** [boolean or string, optional]: Generate or suppress diagnostics for type annotations that use invalid type expression forms or are semantically invalid. The default value for this setting is `"error"`. + **reportMissingTypeStubs** [boolean or string, optional]: Generate or suppress diagnostics for imports that have no corresponding type stub file (either a typeshed file or a custom type stub). The type checker requires type stubs to do its best job at analysis. The default value for this setting is `"none"`. Note that there is a corresponding quick fix for this diagnostics that let you generate custom type stub to improve editing experiences. **reportImportCycles** [boolean or string, optional]: Generate or suppress diagnostics for cyclical import chains. These are not errors in Python, but they do slow down type analysis and often hint at architectural layering issues. Generally, they should be avoided. The default value for this setting is `"none"`. Note that there are import cycles in the typeshed stdlib typestub files that are ignored by this setting. @@ -310,6 +312,7 @@ The following table lists the default severity levels for each diagnostic rule w | deprecateTypingAliases | false | false | false | false | | enableExperimentalFeatures | false | false | false | false | | reportMissingModuleSource | "warning" | "warning" | "warning" | "warning" | +| reportInvalidTypeForm | "warning" | "error" | "error" | "error" | | reportMissingImports | "warning" | "error" | "error" | "error" | | reportUndefinedVariable | "warning" | "error" | "error" | "error" | | reportAssertAlwaysTrue | "none" | "warning" | "warning" | "error" | diff --git a/packages/pyright-internal/src/analyzer/binder.ts b/packages/pyright-internal/src/analyzer/binder.ts index baff20d28730..2ff46334937c 100644 --- a/packages/pyright-internal/src/analyzer/binder.ts +++ b/packages/pyright-internal/src/analyzer/binder.ts @@ -854,8 +854,8 @@ export class Binder extends ParseTreeWalker { if (node.chainedTypeAnnotationComment) { this._addDiagnostic( - this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, - DiagnosticRule.reportGeneralTypeIssues, + this._fileInfo.diagnosticRuleSet.reportInvalidTypeForm, + DiagnosticRule.reportInvalidTypeForm, LocMessage.annotationNotSupported(), node.chainedTypeAnnotationComment ); @@ -3810,8 +3810,8 @@ export class Binder extends ParseTreeWalker { if (!declarationHandled) { this._addDiagnostic( - this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, - DiagnosticRule.reportGeneralTypeIssues, + this._fileInfo.diagnosticRuleSet.reportInvalidTypeForm, + DiagnosticRule.reportInvalidTypeForm, LocMessage.annotationNotSupported(), typeAnnotation ); diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index 5ec67b4041b8..abf8fcbacf24 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -843,8 +843,8 @@ export class Checker extends ParseTreeWalker { if (node.typeComment) { this._evaluator.addDiagnosticForTextRange( this._fileInfo, - this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, - DiagnosticRule.reportGeneralTypeIssues, + this._fileInfo.diagnosticRuleSet.reportInvalidTypeForm, + DiagnosticRule.reportInvalidTypeForm, LocMessage.annotationNotSupported(), node.typeComment ); @@ -898,8 +898,8 @@ export class Checker extends ParseTreeWalker { if (node.typeComment) { this._evaluator.addDiagnosticForTextRange( this._fileInfo, - this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues, - DiagnosticRule.reportGeneralTypeIssues, + this._fileInfo.diagnosticRuleSet.reportInvalidTypeForm, + DiagnosticRule.reportInvalidTypeForm, LocMessage.annotationNotSupported(), node.typeComment ); @@ -2310,7 +2310,7 @@ export class Checker extends ParseTreeWalker { : LocMessage.generatorSyncReturnType(); this._evaluator.addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, errorMessage.format({ yieldType: this._evaluator.printType(AnyType.create()) }) + diagAddendum.getString(), node.returnTypeAnnotation ?? node.name diff --git a/packages/pyright-internal/src/analyzer/operations.ts b/packages/pyright-internal/src/analyzer/operations.ts index 8857acbae1f8..82137031392e 100644 --- a/packages/pyright-internal/src/analyzer/operations.ts +++ b/packages/pyright-internal/src/analyzer/operations.ts @@ -695,11 +695,7 @@ export function getTypeOfBinaryOperation( if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) { // Exempt "|" because it might be a union operation involving unknowns. if (node.operator !== OperatorType.BitwiseOr) { - evaluator.addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, - LocMessage.binaryOperationNotAllowed(), - node - ); + evaluator.addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.binaryOperationNotAllowed(), node); return { type: UnknownType.create() }; } } @@ -952,7 +948,7 @@ export function getTypeOfUnaryOperation( inferenceContext: InferenceContext | undefined ): TypeResult { if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) { - evaluator.addDiagnostic(DiagnosticRule.reportGeneralTypeIssues, LocMessage.unaryOperationNotAllowed(), node); + evaluator.addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.unaryOperationNotAllowed(), node); return { type: UnknownType.create() }; } @@ -1069,7 +1065,7 @@ export function getTypeOfTernaryOperation( const fileInfo = getFileInfo(node); if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) { - evaluator.addDiagnostic(DiagnosticRule.reportGeneralTypeIssues, LocMessage.ternaryNotAllowed(), node); + evaluator.addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.ternaryNotAllowed(), node); return { type: UnknownType.create() }; } diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index f1b6be8e23b3..0730edb77073 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -4483,7 +4483,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions (flags & EvaluatorFlags.DoNotSpecialize) !== 0 ) { addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, LocMessage.typeAnnotationVariable(), node ); @@ -7643,7 +7643,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions const diag = new DiagnosticAddendum(); diag.addMessage(LocAddendum.useTupleInstead()); addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, LocMessage.tupleInAnnotation() + diag.getString(), node ); @@ -7890,7 +7890,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions const diag = new DiagnosticAddendum(); diag.addMessage(LocAddendum.useTypeInstead()); addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, LocMessage.typeCallNotAllowed() + diag.getString(), node ); @@ -8015,7 +8015,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions } if ((flags & EvaluatorFlags.ExpectingTypeAnnotation) !== 0) { - addDiagnostic(DiagnosticRule.reportGeneralTypeIssues, LocMessage.typeAnnotationCall(), node); + addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.typeAnnotationCall(), node); typeResult = { type: UnknownType.create() }; } @@ -12955,11 +12955,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions ) { const diag = new DiagnosticAddendum(); diag.addMessage(LocAddendum.useDictInstead()); - addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, - LocMessage.dictInAnnotation() + diag.getString(), - node - ); + addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.dictInAnnotation() + diag.getString(), node); } // If the expected type is a union, analyze for each of the subtypes @@ -13464,11 +13460,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions ) { const diag = new DiagnosticAddendum(); diag.addMessage(LocAddendum.useListInstead()); - addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, - LocMessage.listInAnnotation() + diag.getString(), - node - ); + addDiagnostic(DiagnosticRule.reportInvalidTypeForm, LocMessage.listInAnnotation() + diag.getString(), node); } // If the expected type is a union, recursively call for each of the subtypes @@ -15600,7 +15592,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions if (!isLegalTypeAliasExpressionForm(node.rightExpression)) { addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, LocMessage.typeAliasIllegalExpressionForm(), node.rightExpression ); @@ -19519,7 +19511,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions // Treat type[function] as illegal. if (isFunction(typeArgs[0].type) || isOverloadedFunction(typeArgs[0].type)) { addDiagnostic( - DiagnosticRule.reportGeneralTypeIssues, + DiagnosticRule.reportInvalidTypeForm, LocMessage.typeAnnotationWithCallable(), typeArgs[0].node ); diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 0edfddf62878..4a73cf2b2a54 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -132,6 +132,9 @@ export interface DiagnosticRuleSet { // Report missing imported module source files? reportMissingModuleSource: DiagnosticLevel; + // Report invalid type annotation forms? + reportInvalidTypeForm: DiagnosticLevel; + // Report missing type stub files? reportMissingTypeStubs: DiagnosticLevel; @@ -363,6 +366,7 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportFunctionMemberAccess, DiagnosticRule.reportMissingImports, DiagnosticRule.reportMissingModuleSource, + DiagnosticRule.reportInvalidTypeForm, DiagnosticRule.reportMissingTypeStubs, DiagnosticRule.reportImportCycles, DiagnosticRule.reportUnusedImport, @@ -452,6 +456,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportFunctionMemberAccess: 'none', reportMissingImports: 'warning', reportMissingModuleSource: 'warning', + reportInvalidTypeForm: 'warning', reportMissingTypeStubs: 'none', reportImportCycles: 'none', reportUnusedImport: 'none', @@ -537,6 +542,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportFunctionMemberAccess: 'none', reportMissingImports: 'error', reportMissingModuleSource: 'warning', + reportInvalidTypeForm: 'error', reportMissingTypeStubs: 'none', reportImportCycles: 'none', reportUnusedImport: 'none', @@ -622,6 +628,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet { reportFunctionMemberAccess: 'error', reportMissingImports: 'error', reportMissingModuleSource: 'warning', + reportInvalidTypeForm: 'error', reportMissingTypeStubs: 'none', reportImportCycles: 'none', reportUnusedImport: 'none', @@ -707,6 +714,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportFunctionMemberAccess: 'error', reportMissingImports: 'error', reportMissingModuleSource: 'warning', // Not overridden by strict mode + reportInvalidTypeForm: 'error', reportMissingTypeStubs: 'error', reportImportCycles: 'none', reportUnusedImport: 'error', diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index d2799099c32d..00f3276552d9 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -26,6 +26,7 @@ export enum DiagnosticRule { reportFunctionMemberAccess = 'reportFunctionMemberAccess', reportMissingImports = 'reportMissingImports', reportMissingModuleSource = 'reportMissingModuleSource', + reportInvalidTypeForm = 'reportInvalidTypeForm', reportMissingTypeStubs = 'reportMissingTypeStubs', reportImportCycles = 'reportImportCycles', reportUnusedImport = 'reportUnusedImport', diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 95aaec83ccd5..393ac769cc89 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -271,6 +271,22 @@ false ] }, + "reportInvalidTypeForm": { + "type": [ + "string", + "boolean" + ], + "description": "Diagnostics for type expression that uses an invalid form.", + "default": "error", + "enum": [ + "none", + "information", + "warning", + "error", + true, + false + ] + }, "reportMissingTypeStubs": { "type": [ "string", diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index 52777efe30c5..e207e5ab1081 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -197,6 +197,12 @@ "title": "Controls reporting of imports that cannot be resolved to source files", "default": "warning" }, + "reportInvalidTypeForm": { + "$id": "#/properties/reportInvalidTypeForm", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of type expressions that use an invalid form", + "default": "warning" + }, "reportMissingTypeStubs": { "$id": "#/properties/reportMissingTypeStubs", "$ref": "#/definitions/diagnostic",