Added new diagnostic rule "reportUnusedCoroutine" that reports an err…

…or if the result returned by an async function is not consumed (awaited, assigned to a variable, etc.). This detects and reports a common error when using async coroutines.
microsoft · Dec 27, 2020 · fb4a4e9 · fb4a4e9
1 parent 5e9675b
commit fb4a4e9
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 1 deletion.
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -138,6 +138,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 **reportUnusedCallResult** [boolean or string, optional]: Generate or suppress diagnostics for call statements whose return value is not used in any way and is not None. The default value for this setting is 'none'.
 
+**reportUnusedCoroutine** [boolean or string, optional]: Generate or suppress diagnostics for call statements whose return value is not used in any way and is a Coroutine. This identifies a common error where an `await` keyword is mistakenly omitted. The default value for this setting is 'error'.
+
 
 ## Execution Environment Options
 Pyright allows multiple “execution environments” to be defined for different portions of your source tree. For example, a subtree may be designed to run with different import search paths or a different version of the python interpreter than the rest of the source base.

diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts
@@ -459,9 +459,13 @@ export class Checker extends ParseTreeWalker {
             );
         }
 
-        if (this._fileInfo.diagnosticRuleSet.reportUnusedCallResult !== 'none') {
+        if (
+            this._fileInfo.diagnosticRuleSet.reportUnusedCallResult !== 'none' ||
+            this._fileInfo.diagnosticRuleSet.reportUnusedCoroutine !== 'none'
+        ) {
             if (node.parent?.nodeType === ParseNodeType.StatementList) {
                 const returnType = this._evaluator.getType(node);
+
                 if (
                     returnType &&
                     !isNone(returnType) &&
@@ -477,6 +481,15 @@ export class Checker extends ParseTreeWalker {
                         }),
                         node
                     );
+
+                    if (isObject(returnType) && ClassType.isBuiltIn(returnType.classType, 'Coroutine')) {
+                        this._evaluator.addDiagnostic(
+                            this._fileInfo.diagnosticRuleSet.reportUnusedCoroutine,
+                            DiagnosticRule.reportUnusedCoroutine,
+                            Localizer.Diagnostic.unusedCoroutine(),
+                            node
+                        );
+                    }
                 }
             }
         }

diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts
@@ -236,6 +236,10 @@ export interface DiagnosticRuleSet {
     // Report cases where a call expression's return result is not
     // None and is not used in any way.
     reportUnusedCallResult: DiagnosticLevel;
+
+    // Report cases where a call expression's return result is Coroutine
+    // and is not used in any way.
+    reportUnusedCoroutine: DiagnosticLevel;
 }
 
 export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
@@ -304,6 +308,7 @@ export function getDiagLevelDiagnosticRules() {
         DiagnosticRule.reportInvalidStubStatement,
         DiagnosticRule.reportUnsupportedDunderAll,
         DiagnosticRule.reportUnusedCallResult,
+        DiagnosticRule.reportUnusedCoroutine,
     ];
 }
 
@@ -369,6 +374,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
         reportInvalidStubStatement: 'none',
         reportUnsupportedDunderAll: 'none',
         reportUnusedCallResult: 'none',
+        reportUnusedCoroutine: 'none',
     };
 
     return diagSettings;
@@ -430,6 +436,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
         reportInvalidStubStatement: 'none',
         reportUnsupportedDunderAll: 'warning',
         reportUnusedCallResult: 'none',
+        reportUnusedCoroutine: 'error',
     };
 
     return diagSettings;
@@ -491,6 +498,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
         reportInvalidStubStatement: 'error',
         reportUnsupportedDunderAll: 'error',
         reportUnusedCallResult: 'none',
+        reportUnusedCoroutine: 'error',
     };
 
     return diagSettings;
@@ -1140,6 +1148,13 @@ export class ConfigOptions {
                 DiagnosticRule.reportUnusedCallResult,
                 defaultSettings.reportUnusedCallResult
             ),
+
+            // Read the "reportUnusedCoroutine" entry.
+            reportUnusedCoroutine: this._convertDiagnosticLevel(
+                configObj.reportUnusedCoroutine,
+                DiagnosticRule.reportUnusedCoroutine,
+                defaultSettings.reportUnusedCoroutine
+            ),
         };
 
         // Read the "venvPath".

diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts
@@ -62,4 +62,5 @@ export enum DiagnosticRule {
     reportInvalidStubStatement = 'reportInvalidStubStatement',
     reportUnsupportedDunderAll = 'reportUnsupportedDunderAll',
     reportUnusedCallResult = 'reportUnusedCallResult',
+    reportUnusedCoroutine = 'reportUnusedCoroutine',
 }
diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts
@@ -668,6 +668,7 @@ export namespace Localizer {
         export const unsupportedDunderAllOperation = () => getRawString('Diagnostic.unsupportedDunderAllOperation');
         export const unusedCallResult = () =>
             new ParameterizedString<{ type: string }>(getRawString('Diagnostic.unusedCallResult'));
+        export const unusedCoroutine = () => getRawString('Diagnostic.unusedCoroutine');
         export const varAnnotationIllegal = () => getRawString('Diagnostic.varAnnotationIllegal');
         export const walrusIllegal = () => getRawString('Diagnostic.walrusIllegal');
         export const walrusNotAllowed = () => getRawString('Diagnostic.walrusNotAllowed');

diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json
@@ -333,6 +333,7 @@
         "unsupportedDunderAllAssignment": "Expression assigned to \"__all__\" is not supported, so exported symbol list may be incorrect; use list or tuple of string literal values in assignment",
         "unsupportedDunderAllOperation": "Operation on \"__all__\" is not supported, so exported symbol list may not be incorrect",
         "unusedCallResult": "Result of call expression is of type \"{type}\" and is not used; assign to variable \"_\" if this is intentional",
+        "unusedCoroutine": "Result of async function call is not used; use \"await\" or assign result to variable",
         "varAnnotationIllegal": "Type annotations for variables requires Python 3.6 or newer; use type comment for compatibility with previous versions",
         "walrusIllegal": "Operator \":=\" requires Python 3.8 or newer",
         "walrusNotAllowed": "Operator \":=\" not allowed in this context",

diff --git a/packages/pyright-internal/src/tests/samples/unusedCoroutine1.py b/packages/pyright-internal/src/tests/samples/unusedCoroutine1.py
@@ -0,0 +1,23 @@
+# This sample tests the reportUnusedCoroutine diagnostic rule.
+
+
+async def func1():
+    return 3
+
+
+async def func2() -> str:
+    return "5"
+
+
+async def func3():
+    await func1()
+    await func2()
+
+    # This should generate an error
+    func1()
+
+    # This should generate an error
+    func2()
+
+    _ = func1()
+    _ = func2()
diff --git a/packages/pyright-internal/src/tests/typeEvaluator2.test.ts b/packages/pyright-internal/src/tests/typeEvaluator2.test.ts
@@ -1350,3 +1350,8 @@ test('UnusedCallResult1', () => {
     analysisResults = TestUtils.typeAnalyzeSampleFiles(['unusedCallResult1.py'], configOptions);
     TestUtils.validateResults(analysisResults, 3);
 });
+
+test('UnusedCoroutine1', () => {
+    const analysisResults = TestUtils.typeAnalyzeSampleFiles(['unusedCoroutine1.py']);
+    TestUtils.validateResults(analysisResults, 2);
+});
diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json
@@ -609,6 +609,17 @@
                                 "error"
                             ]
                         },
+                        "reportUnusedCoroutine": {
+                            "type": "string",
+                            "description": "Diagnostics for call expressions that return a Coroutine and whose results are not consumed.",
+                            "default": "error",
+                            "enum": [
+                                "none",
+                                "information",
+                                "warning",
+                                "error"
+                            ]
+                        },
                         "reportUnsupportedDunderAll": {
                             "type": "string",
                             "description": "Diagnostics for unsupported operations performed on __all__.",

diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json
@@ -399,6 +399,12 @@
       "title": "Controls reporting of call expressions whose results are not consumed",
       "default": "none"
     },
+    "reportUnusedCoroutine": {
+      "$id": "#/properties/reportUnusedCoroutine",
+      "$ref": "#/definitions/diagnostic",
+      "title": "Controls reporting of call expressions that returns Coroutine whose results are not consumed",
+      "default": "error"
+    },
     "pythonVersion": {
       "$id": "#/properties/pythonVersion",
       "type": "string",