From b740ddfae5e6e74fcc819a1c8a78f83bc2571309 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 23 Jan 2022 12:49:27 -0800 Subject: [PATCH] Added new diagnostic check "reportMatchNotExhaustive" which reports cases where a `match` statement doesn't exhaustively cover all cases. --- docs/configuration.md | 15 ++---- .../pyright-internal/src/analyzer/checker.ts | 29 ++++++++++ .../src/common/configOptions.ts | 15 ++++++ .../src/common/diagnosticRules.ts | 1 + .../src/localization/localize.ts | 4 ++ .../src/localization/package.nls.en-us.json | 3 ++ .../src/tests/samples/match10.py | 54 +++++++++++++++++++ .../src/tests/typeEvaluator3.test.ts | 13 +++++ packages/vscode-pyright/package.json | 11 ++++ .../schemas/pyrightconfig.schema.json | 6 +++ 10 files changed, 140 insertions(+), 11 deletions(-) create mode 100644 packages/pyright-internal/src/tests/samples/match10.py diff --git a/docs/configuration.md b/docs/configuration.md index a5533d3e5..596fb1572 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -164,7 +164,9 @@ The following settings control pyright’s diagnostic output (warnings or errors **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'. -**reportUnnecessaryTypeIgnoreComment** [boolean or string, optional]: Generate or suppress diagnostics for a '# type: ignore' comment that would have no effect if removed. +**reportUnnecessaryTypeIgnoreComment** [boolean or string, optional]: Generate or suppress diagnostics for a '# type: ignore' comment that would have no effect if removed. The default value for this setting is 'none'. + +**reportMatchNotExhaustive** [boolean or string, optional]: Generate or suppress diagnostics for a 'match' statement that does not provide cases that exhaustively match against all potential types of the target expression. The default value for this setting is 'none'. ## Execution Environment Options @@ -336,15 +338,6 @@ The following table lists the default severity levels for each diagnostic rule w | reportUnusedCallResult | "none" | "none" | "none" | | reportUnusedCoroutine | "none" | "error" | "error" | | reportUnnecessaryTypeIgnoreComment | "none" | "none" | "none" | - - - - - - - - - - +| reportMatchNotExhaustive | "none" | "none" | "error" | diff --git a/packages/pyright-internal/src/analyzer/checker.ts b/packages/pyright-internal/src/analyzer/checker.ts index d74e323ae..f5cf96ec1 100644 --- a/packages/pyright-internal/src/analyzer/checker.ts +++ b/packages/pyright-internal/src/analyzer/checker.ts @@ -1288,6 +1288,7 @@ export class Checker extends ParseTreeWalker { override visitMatch(node: MatchNode): boolean { this._evaluator.getType(node.subjectExpression); + this._validateExhaustiveMatch(node); return true; } @@ -1321,6 +1322,34 @@ export class Checker extends ParseTreeWalker { return false; } + private _validateExhaustiveMatch(node: MatchNode) { + // This check can be expensive, so skip it if it's disabled. + if (this._fileInfo.diagnosticRuleSet.reportMatchNotExhaustive === 'none') { + return; + } + + const narrowedTypeResult = this._evaluator.evaluateTypeForSubnode(node, () => { + this._evaluator.evaluateTypesForMatchNode(node); + }); + + if (narrowedTypeResult && !isNever(narrowedTypeResult.type)) { + const diagAddendum = new DiagnosticAddendum(); + diagAddendum.addMessage( + Localizer.DiagnosticAddendum.matchIsNotExhaustiveType().format({ + type: this._evaluator.printType(narrowedTypeResult.type), + }) + ); + diagAddendum.addMessage(Localizer.DiagnosticAddendum.matchIsNotExhaustiveHint()); + + this._evaluator.addDiagnostic( + this._fileInfo.diagnosticRuleSet.reportMatchNotExhaustive, + DiagnosticRule.reportMatchNotExhaustive, + Localizer.Diagnostic.matchIsNotExhaustive() + diagAddendum.getString(), + node.subjectExpression + ); + } + } + private _suppressUnboundCheck(callback: () => void) { const wasSuppressed = this._isUnboundCheckSuppressed; this._isUnboundCheckSuppressed = true; diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 658111c86..5f579fa6d 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -281,6 +281,10 @@ export interface DiagnosticRuleSet { // Report cases where the removal of a "# type: ignore" comment would // have no effect. reportUnnecessaryTypeIgnoreComment: DiagnosticLevel; + + // Report cases where the a "match" statement is not exhaustive in + // covering all possible cases. + reportMatchNotExhaustive: DiagnosticLevel; } export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet { @@ -361,6 +365,7 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportUnusedCallResult, DiagnosticRule.reportUnusedCoroutine, DiagnosticRule.reportUnnecessaryTypeIgnoreComment, + DiagnosticRule.reportMatchNotExhaustive, ]; } @@ -439,6 +444,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportUnusedCallResult: 'none', reportUnusedCoroutine: 'none', reportUnnecessaryTypeIgnoreComment: 'none', + reportMatchNotExhaustive: 'none', }; return diagSettings; @@ -513,6 +519,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportUnusedCallResult: 'none', reportUnusedCoroutine: 'error', reportUnnecessaryTypeIgnoreComment: 'none', + reportMatchNotExhaustive: 'none', }; return diagSettings; @@ -587,6 +594,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportUnusedCallResult: 'none', reportUnusedCoroutine: 'error', reportUnnecessaryTypeIgnoreComment: 'none', + reportMatchNotExhaustive: 'error', }; return diagSettings; @@ -1331,6 +1339,13 @@ export class ConfigOptions { DiagnosticRule.reportUnnecessaryTypeIgnoreComment, defaultSettings.reportUnnecessaryTypeIgnoreComment ), + + // Read the "reportUnusedCoroutine" entry. + reportMatchNotExhaustive: this._convertDiagnosticLevel( + configObj.reportMatchNotExhaustive, + DiagnosticRule.reportMatchNotExhaustive, + defaultSettings.reportMatchNotExhaustive + ), }; // Read the "venvPath". diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 8e3c38800..881d8ee1d 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -74,4 +74,5 @@ export enum DiagnosticRule { reportUnusedCallResult = 'reportUnusedCallResult', reportUnusedCoroutine = 'reportUnusedCoroutine', reportUnnecessaryTypeIgnoreComment = 'reportUnnecessaryTypeIgnoreComment', + reportMatchNotExhaustive = 'reportMatchNotExhaustive', } diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index 949bcb748..fcb68df33 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -490,6 +490,7 @@ export namespace Localizer { export const literalEmptyArgs = () => getRawString('Diagnostic.literalEmptyArgs'); export const literalNotCallable = () => getRawString('Diagnostic.literalNotCallable'); export const matchIncompatible = () => getRawString('Diagnostic.matchIncompatible'); + export const matchIsNotExhaustive = () => getRawString('Diagnostic.matchIsNotExhaustive'); export const memberAccess = () => new ParameterizedString<{ name: string; type: string }>(getRawString('Diagnostic.memberAccess')); export const memberDelete = () => @@ -980,6 +981,9 @@ export namespace Localizer { new ParameterizedString<{ sourceType: string; destType: string }>( getRawString('DiagnosticAddendum.literalAssignmentMismatch') ); + export const matchIsNotExhaustiveType = () => + new ParameterizedString<{ type: string }>(getRawString('DiagnosticAddendum.matchIsNotExhaustiveType')); + export const matchIsNotExhaustiveHint = () => getRawString('DiagnosticAddendum.matchIsNotExhaustiveHint'); export const memberAssignment = () => new ParameterizedString<{ type: string; name: string; classType: string }>( getRawString('DiagnosticAddendum.memberAssignment') diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json index df85d0572..8b997bca6 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -218,6 +218,7 @@ "literalEmptyArgs": "Expected one or more type arguments after \"Literal\"", "literalNotCallable": "Literal type cannot be instantiated", "matchIncompatible": "Match statements require Python 3.10 or newer", + "matchIsNotExhaustive": "Match statement does not exhaustively handle all cases", "memberAccess": "Cannot access member \"{name}\" for type \"{type}\"", "memberDelete": "Cannot delete member \"{name}\" for type \"{type}\"", "memberSet": "Cannot assign member \"{name}\" for type \"{type}\"", @@ -504,6 +505,8 @@ "kwargsParamMissing": "Parameter \"**{paramName}\" has no corresponding parameter", "listAssignmentMismatch": "Type \"{type}\" is incompatible with target list", "literalAssignmentMismatch": "\"{sourceType}\" cannot be assigned to type \"{destType}\"", + "matchIsNotExhaustiveType": "Unhandled types: \"{type}\"", + "matchIsNotExhaustiveHint": "If exhaustion is not desired, add \"case _: pass\"", "memberSetClassVar": "Member \"{name}\" cannot be assigned through a class instance because it is a ClassVar", "memberAssignment": "Expression of type \"{type}\" cannot be assigned to member \"{name}\" of class \"{classType}\"", "memberIsAbstract": "\"{type}.{name}\" is abstract", diff --git a/packages/pyright-internal/src/tests/samples/match10.py b/packages/pyright-internal/src/tests/samples/match10.py new file mode 100644 index 000000000..ad42ec051 --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/match10.py @@ -0,0 +1,54 @@ +# This sample tests the reportMatchNotExhaustive diagnostic check. + +from typing import Literal +from enum import Enum + +def func1(subj: Literal["a", "b"], cond: bool): + # This should generate an error if reportMatchNotExhaustive is enabled. + match subj: + case "a": + pass + + case "b" if cond: + pass + + +def func2(subj: object): + # This should generate an error if reportMatchNotExhaustive is enabled. + match subj: + case int(): + pass + +def func3(subj: object): + match subj: + case object(): + pass + +def func4(subj: tuple[str] | tuple[int]): + match subj[0]: + case str(): + pass + + case int(): + pass + +def func5(subj: Literal[1, 2, 3]): + # This should generate an error if reportMatchNotExhaustive is enabled. + match subj: + case 1 | 2: + pass + +class Color(Enum): + red = 0 + green= 1 + blue = 2 + + +def func6(subj: Color): + # This should generate an error if reportMatchNotExhaustive is enabled. + match subj: + case Color.red: + pass + + case Color.green: + pass diff --git a/packages/pyright-internal/src/tests/typeEvaluator3.test.ts b/packages/pyright-internal/src/tests/typeEvaluator3.test.ts index e60a28d0d..a847739a6 100644 --- a/packages/pyright-internal/src/tests/typeEvaluator3.test.ts +++ b/packages/pyright-internal/src/tests/typeEvaluator3.test.ts @@ -787,6 +787,19 @@ test('Match9', () => { TestUtils.validateResults(analysisResults, 0); }); +test('Match10', () => { + const configOptions = new ConfigOptions('.'); + + configOptions.defaultPythonVersion = PythonVersion.V3_10; + configOptions.diagnosticRuleSet.reportMatchNotExhaustive = 'none'; + const analysisResults1 = TestUtils.typeAnalyzeSampleFiles(['match10.py'], configOptions); + TestUtils.validateResults(analysisResults1, 0); + + configOptions.diagnosticRuleSet.reportMatchNotExhaustive = 'error'; + const analysisResults2 = TestUtils.typeAnalyzeSampleFiles(['match10.py'], configOptions); + TestUtils.validateResults(analysisResults2, 4); +}); + test('List1', () => { const analysisResults = TestUtils.typeAnalyzeSampleFiles(['list1.py']); TestUtils.validateResults(analysisResults, 0); diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index e9b38f565..789a1a008 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -739,6 +739,17 @@ "warning", "error" ] + }, + "reportMatchNotExhaustive": { + "type": "string", + "description": "Diagnostics for 'match' statements that do not exhaustively match all possible values.", + "default": "none", + "enum": [ + "none", + "information", + "warning", + "error" + ] } } }, diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index 650ce54a3..c8911d3f3 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -473,6 +473,12 @@ "title": "Controls reporting of '# type: ignore' comments that have no effect'", "default": "none" }, + "reportMatchNotExhaustive": { + "$id": "#/properties/reportMatchNotExhaustive", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of 'match' statements that do not exhaustively match all possible values", + "default": "none" + }, "extraPaths": { "$id": "#/properties/extraPaths", "type": "array",