Added new diagnostic check "reportMatchNotExhaustive" which reports cases where a match statement doesn't exhaustively cover all cases.

This commit is contained in:
Eric Traut 2022-01-23 12:49:27 -08:00
parent cdce0b34b2
commit b740ddfae5
10 changed files with 140 additions and 11 deletions

View File

@ -164,7 +164,9 @@ The following settings control pyrights 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" |

View File

@ -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;

View File

@ -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".

View File

@ -74,4 +74,5 @@ export enum DiagnosticRule {
reportUnusedCallResult = 'reportUnusedCallResult',
reportUnusedCoroutine = 'reportUnusedCoroutine',
reportUnnecessaryTypeIgnoreComment = 'reportUnnecessaryTypeIgnoreComment',
reportMatchNotExhaustive = 'reportMatchNotExhaustive',
}

View File

@ -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')

View File

@ -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",

View File

@ -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

View File

@ -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);

View File

@ -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"
]
}
}
},

View File

@ -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",