Added new diagnostic rule reportInvalidTypeForm that controls repor…

…ting of invalid type expression forms. This partly addresses microsoft#6973. (microsoft#7069)
Domma143 · Jan 21, 2024 · 04e0536 · 04e0536
1 parent 566f333
commit 04e0536
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 32 deletions.
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -72,6 +72,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 <a name="reportMissingModuleSource"></a> **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"`.
 
+<a name="reportInvalidTypeForm"></a> **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"`.
+
 <a name="reportMissingTypeStubs"></a> **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.
 
 <a name="reportImportCycles"></a> **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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",