Added a new check for a TypeGuard or StrictTypeGuard function that do…

…es not have at least one input parameter. This addresses #4448.
microsoft · Jan 13, 2023 · 90f8653 · 90f8653
1 parent bfcf41c
commit 90f8653
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 35 deletions.
diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -649,13 +649,8 @@ export class Checker extends ParseTreeWalker {
             // Verify common dunder signatures.
             this._validateDunderSignatures(node, functionTypeResult.functionType, containingClassNode !== undefined);
 
-            // Verify that strict type guard functions don't violate the constraints
-            // of strict type guards.
-            this._validateStrictTypeGuardFunction(
-                node,
-                functionTypeResult.functionType,
-                containingClassNode !== undefined
-            );
+            // Verify TypeGuard or StrictTypeGuard functions.
+            this._validateTypeGuardFunction(node, functionTypeResult.functionType, containingClassNode !== undefined);
 
             this._validateFunctionTypeVarUsage(node, functionTypeResult);
         }
@@ -3917,44 +3912,70 @@ export class Checker extends ParseTreeWalker {
         });
     }
 
-    private _validateStrictTypeGuardFunction(node: FunctionNode, functionType: FunctionType, isMethod: boolean) {
-        // Is this a strict type guard function?
-        if (!functionType.details.declaredReturnType) {
+    private _validateTypeGuardFunction(node: FunctionNode, functionType: FunctionType, isMethod: boolean) {
+        const returnType = functionType.details.declaredReturnType;
+        if (!returnType) {
             return;
         }
 
-        if (
-            !isClassInstance(functionType.details.declaredReturnType) ||
-            !ClassType.isBuiltIn(functionType.details.declaredReturnType, 'StrictTypeGuard') ||
-            !functionType.details.declaredReturnType.typeArguments ||
-            functionType.details.declaredReturnType.typeArguments.length < 1
-        ) {
+        if (!isClassInstance(returnType) || !returnType.typeArguments || returnType.typeArguments.length < 1) {
             return;
         }
 
-        const typeGuardType = functionType.details.declaredReturnType.typeArguments[0];
+        const isNormalTypeGuard = ClassType.isBuiltIn(returnType, 'TypeGuard');
+        const isStrictTypeGuard = ClassType.isBuiltIn(returnType, 'StrictTypeGuard');
 
-        // Determine the type of the first parameter.
-        const paramIndex = isMethod && !FunctionType.isStaticMethod(functionType) ? 1 : 0;
-        if (paramIndex >= functionType.details.parameters.length) {
+        if (!isNormalTypeGuard && !isStrictTypeGuard) {
             return;
         }
 
-        const paramType = FunctionType.getEffectiveParameterType(functionType, paramIndex);
+        // Make sure there's at least one input parameter provided.
+        let paramCount = functionType.details.parameters.length;
+        if (isMethod) {
+            if (
+                FunctionType.isInstanceMethod(functionType) ||
+                FunctionType.isConstructorMethod(functionType) ||
+                FunctionType.isClassMethod(functionType)
+            ) {
+                paramCount--;
+            }
+        }
 
-        // Verify that the typeGuardType is a narrower type than the paramType.
-        if (!this._evaluator.assignType(paramType, typeGuardType)) {
-            const returnAnnotation = node.returnTypeAnnotation || node.functionAnnotationComment?.returnTypeAnnotation;
-            if (returnAnnotation) {
-                this._evaluator.addDiagnostic(
-                    this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
-                    DiagnosticRule.reportGeneralTypeIssues,
-                    Localizer.Diagnostic.strictTypeGuardReturnType().format({
-                        type: this._evaluator.printType(paramType),
-                        returnType: this._evaluator.printType(typeGuardType),
-                    }),
-                    returnAnnotation
-                );
+        if (paramCount < 1) {
+            this._evaluator.addDiagnostic(
+                this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
+                DiagnosticRule.reportGeneralTypeIssues,
+                Localizer.Diagnostic.typeGuardParamCount(),
+                node.name
+            );
+        }
+
+        if (isStrictTypeGuard) {
+            const typeGuardType = returnType.typeArguments[0];
+
+            // Determine the type of the first parameter.
+            const paramIndex = isMethod && !FunctionType.isStaticMethod(functionType) ? 1 : 0;
+            if (paramIndex >= functionType.details.parameters.length) {
+                return;
+            }
+
+            const paramType = FunctionType.getEffectiveParameterType(functionType, paramIndex);
+
+            // Verify that the typeGuardType is a narrower type than the paramType.
+            if (!this._evaluator.assignType(paramType, typeGuardType)) {
+                const returnAnnotation =
+                    node.returnTypeAnnotation || node.functionAnnotationComment?.returnTypeAnnotation;
+                if (returnAnnotation) {
+                    this._evaluator.addDiagnostic(
+                        this._fileInfo.diagnosticRuleSet.reportGeneralTypeIssues,
+                        DiagnosticRule.reportGeneralTypeIssues,
+                        Localizer.Diagnostic.strictTypeGuardReturnType().format({
+                            type: this._evaluator.printType(paramType),
+                            returnType: this._evaluator.printType(typeGuardType),
+                        }),
+                        returnAnnotation
+                    );
+                }
             }
         }
     }

diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts
@@ -843,6 +843,7 @@ export namespace Localizer {
         export const typeExpectedClass = () =>
             new ParameterizedString<{ type: string }>(getRawString('Diagnostic.typeExpectedClass'));
         export const typeGuardArgCount = () => getRawString('Diagnostic.typeGuardArgCount');
+        export const typeGuardParamCount = () => getRawString('Diagnostic.typeGuardParamCount');
         export const typeNotAwaitable = () =>
             new ParameterizedString<{ type: string }>(getRawString('Diagnostic.typeNotAwaitable'));
         export const typeNotCallable = () =>

diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json
@@ -428,6 +428,7 @@
         "typedDictTotalParam": "Expected \"total\" parameter to have a value of True or False",
         "typeExpectedClass": "Expected type expression but received \"{type}\"",
         "typeGuardArgCount": "Expected a single type argument after \"TypeGuard\"",
+        "typeGuardParamCount": "User-defined type guard functions and methods must have at least one input parameter",
         "typeNotAwaitable": "\"{type}\" is not awaitable",
         "typeNotCallable": "\"{expression}\" has type \"{type}\" and is not callable",
         "typeNotIntantiable": "\"{type}\" cannot be instantiated",

diff --git a/packages/pyright-internal/src/tests/samples/typeGuard1.py b/packages/pyright-internal/src/tests/samples/typeGuard1.py
@@ -66,3 +66,14 @@ def bad5(a: int) -> TypeGuard[int]:
     # This should generate an error because only
     # bool values can be returned.
     return 3
+
+# This should generate an error because a type guard function must
+# accept at least one parameter.
+def bad6() -> TypeGuard[int]:
+    return True
+
+class ClassA:
+    # This should generate an error because a type guard function must
+    # accept at least one parameter.
+    def method1(self) -> TypeGuard[int]:
+        return True
diff --git a/packages/pyright-internal/src/tests/typeEvaluator3.test.ts b/packages/pyright-internal/src/tests/typeEvaluator3.test.ts
@@ -831,7 +831,7 @@ test('Enums10', () => {
 test('TypeGuard1', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeGuard1.py']);
 
-    TestUtils.validateResults(analysisResults, 5);
+    TestUtils.validateResults(analysisResults, 7);
 });
 
 test('TypeGuard2', () => {