Added new diagnostic check reportUnusedExpression to catch bugs like a == 4 when a = 4 was intended.

This commit is contained in:
Eric Traut 2022-03-13 15:25:18 -06:00
parent 1a61f7328f
commit d28c481535
24 changed files with 135 additions and 28 deletions

View File

@ -164,6 +164,8 @@ 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'.
**reportUnusedExpression** [boolean or string, optional]: Generate or suppress diagnostics for simple expressions whose results are not used in any way. The default value for this setting is 'none'.
**reportUnnecessaryTypeIgnoreComment** [boolean or string, optional]: Generate or suppress diagnostics for a '# type: ignore' or '# pyright: 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'.
@ -337,6 +339,7 @@ The following table lists the default severity levels for each diagnostic rule w
| reportUnsupportedDunderAll | "none" | "warning" | "error" |
| reportUnusedCallResult | "none" | "none" | "none" |
| reportUnusedCoroutine | "none" | "error" | "error" |
| reportUnusedExpression | "none" | "warning" | "error" |
| reportUnnecessaryTypeIgnoreComment | "none" | "none" | "none" |
| reportMatchNotExhaustive | "none" | "none" | "error" |

View File

@ -267,6 +267,8 @@ export class Checker extends ParseTreeWalker {
// through lazy analysis. This will mark referenced symbols as
// accessed and report any errors associated with it.
this._evaluator.getType(statement);
this._reportUnusedExpression(statement);
}
});
@ -1317,6 +1319,28 @@ export class Checker extends ParseTreeWalker {
return false;
}
private _reportUnusedExpression(node: ParseNode) {
if (this._fileInfo.diagnosticRuleSet.reportUnusedExpression === 'none') {
return;
}
const simpleExpressionTypes = [
ParseNodeType.UnaryOperation,
ParseNodeType.BinaryOperation,
ParseNodeType.Number,
ParseNodeType.Constant
];
if (simpleExpressionTypes.some((nodeType) => nodeType === node.nodeType)) {
this._evaluator.addDiagnostic(
this._fileInfo.diagnosticRuleSet.reportUnusedExpression,
DiagnosticRule.reportUnusedExpression,
Localizer.Diagnostic.unusedExpression(),
node
);
}
}
private _validateExhaustiveMatch(node: MatchNode) {
// This check can be expensive, so skip it if it's disabled.
if (this._fileInfo.diagnosticRuleSet.reportMatchNotExhaustive === 'none') {

View File

@ -279,6 +279,9 @@ export interface DiagnosticRuleSet {
// and is not used in any way.
reportUnusedCoroutine: DiagnosticLevel;
// Report cases where a simple expression result is not used in any way.
reportUnusedExpression: DiagnosticLevel;
// Report cases where the removal of a "# type: ignore" or "# pyright: ignore"
// comment would have no effect.
reportUnnecessaryTypeIgnoreComment: DiagnosticLevel;
@ -373,6 +376,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportUnsupportedDunderAll,
DiagnosticRule.reportUnusedCallResult,
DiagnosticRule.reportUnusedCoroutine,
DiagnosticRule.reportUnusedExpression,
DiagnosticRule.reportUnnecessaryTypeIgnoreComment,
DiagnosticRule.reportMatchNotExhaustive,
];
@ -452,6 +456,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'none',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'none',
reportUnusedExpression: 'none',
reportUnnecessaryTypeIgnoreComment: 'none',
reportMatchNotExhaustive: 'none',
};
@ -527,6 +532,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'warning',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'error',
reportUnusedExpression: 'warning',
reportUnnecessaryTypeIgnoreComment: 'none',
reportMatchNotExhaustive: 'none',
};
@ -602,6 +608,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'error',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'error',
reportUnusedExpression: 'error',
reportUnnecessaryTypeIgnoreComment: 'none',
reportMatchNotExhaustive: 'error',
};

View File

@ -73,6 +73,7 @@ export enum DiagnosticRule {
reportUnsupportedDunderAll = 'reportUnsupportedDunderAll',
reportUnusedCallResult = 'reportUnusedCallResult',
reportUnusedCoroutine = 'reportUnusedCoroutine',
reportUnusedExpression = 'reportUnusedExpression',
reportUnnecessaryTypeIgnoreComment = 'reportUnnecessaryTypeIgnoreComment',
reportMatchNotExhaustive = 'reportMatchNotExhaustive',
}

View File

@ -914,6 +914,7 @@ export namespace Localizer {
export const unusedCallResult = () =>
new ParameterizedString<{ type: string }>(getRawString('Diagnostic.unusedCallResult'));
export const unusedCoroutine = () => getRawString('Diagnostic.unusedCoroutine');
export const unusedExpression = () => getRawString('Diagnostic.unusedExpression');
export const varAnnotationIllegal = () => getRawString('Diagnostic.varAnnotationIllegal');
export const variadicTypeArgsTooMany = () => getRawString('Diagnostic.variadicTypeArgsTooMany');
export const variadicTypeParamTooManyAlias = () =>

View File

@ -468,6 +468,7 @@
"unsupportedDunderAllOperation": "Operation on \"__all__\" is not supported, so exported symbol list may 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",
"unusedExpression": "Expression value is unused",
"varAnnotationIllegal": "Type annotations for variables requires Python 3.6 or newer; use type comment for compatibility with previous versions",
"variadicTypeArgsTooMany": "Type argument list can have at most one unpacked TypeVarTuple or Tuple",
"variadicTypeParamTooManyAlias": "Type alias can have at most one TypeVarTuple type parameter but received multiple ({names})",

View File

@ -396,6 +396,24 @@ test('Strings1', () => {
TestUtils.validateResults(analysisResults2, 2);
});
test('UnusedExpression1', () => {
const configOptions = new ConfigOptions('.');
// By default, this is a warning.
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['unusedExpression1.py'], configOptions);
TestUtils.validateResults(analysisResults, 0, 9);
// Disable it.
configOptions.diagnosticRuleSet.reportUnusedExpression = 'none';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['unusedExpression1.py'], configOptions);
TestUtils.validateResults(analysisResults, 0);
// Enable it as an error.
configOptions.diagnosticRuleSet.reportUnusedExpression = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['unusedExpression1.py'], configOptions);
TestUtils.validateResults(analysisResults, 9);
});
// For now, this functionality is disabled.
// test('Deprecated1', () => {

View File

@ -1,5 +1,7 @@
# This sample tests the Python 3.8 assignment expressions.
# pyright: reportUnusedExpression=false
def func1():
b = 'a'
d = 'b'

View File

@ -40,7 +40,7 @@ def func2(v1: Optional[int]):
if v1 is not None:
def func2_inner1():
v1 + 5
x = v1 + 5
def func2_inner2():
lambda: v1 + 5

View File

@ -33,7 +33,7 @@ d4 = DataTuple(id=1, aid=Other(), name=None)
id = d1.id
h4: Hashable = d4
d3 == d4
v = d3 == d4
# This should generate an error because the name argument
# is the incorrect type.

View File

@ -37,7 +37,7 @@ def func1(obj: A) -> Literal[3]:
obj.prop1 = 3
obj.prop1 + 1
v1 = obj.prop1 + 1
return obj.prop1
@ -100,7 +100,7 @@ def func4(obj: B) -> Literal[3]:
obj.desc1 = 3
obj.desc1 + 1
v1 = obj.desc1 + 1
return obj.desc1

View File

@ -21,4 +21,4 @@ def func2(a: _T) -> bool | _T:
y = func2(None)
if y is not True:
y or func2(False)
z = y or func2(False)

View File

@ -8,4 +8,4 @@ import unresolved_import
def test_zero_division():
with unresolved_import.raises(ZeroDivisionError):
1 / 0
v = 1 / 0

View File

@ -28,6 +28,6 @@ def func1(a: Foo):
reveal_type(type(a) == Foo, expected_text="str")
# This should generate an error
str + str
x = str + str
reveal_type(Foo + Foo, expected_text="int")

View File

@ -18,5 +18,5 @@ class B:
a, b = A(), B()
a @ b
b @ a
v1 = a @ b
v2 = b @ a

View File

@ -72,6 +72,6 @@ e = None
if 1:
e = 4
e + 4
e < 5
not e
v1 = e + 4
v2 = e < 5
v3 = not e

View File

@ -6,19 +6,19 @@ from typing import Optional
def foo(self, x: Optional[int]) -> str:
# This should suppress the error
x + "hi" # pyright: ignore - test
v1 = x + "hi" # pyright: ignore - test
# This is unnecessary
x + x # pyright: ignore
v2 = x + x # pyright: ignore
# This will not suppress the error
# These are both unnecessary
x + x # pyright: ignore [foo, bar]
v3 = x + x # pyright: ignore [foo, bar]
# This will not suppress the error
x + x # pyright: ignore []
v4 = x + x # pyright: ignore []
# One of these is unnecessary
x + "hi" # pyright: ignore [reportGeneralTypeIssues, foo]
v5 = x + "hi" # pyright: ignore [reportGeneralTypeIssues, foo]
return 3 # pyright: ignore [reportGeneralTypeIssues]

View File

@ -3,6 +3,8 @@
# errors, but it should exhibit good recovery, preferably
# emitting one error per instance, not a cascade of errors.
# pyright: reportUnusedExpression=false
# This should generate an error.
print 3 + 3

View File

@ -13,12 +13,12 @@ class ClassA:
a = ClassA()
b = ClassA()
a < b
a <= b
a > b
a >= b
a == b
a != b
v1 = a < b
v2 = a <= b
v3 = a > b
v4 = a >= b
v5 = a == b
v6 = a != b
# This should generate an error because it doesn't declare
# any of the required ordering functions.

View File

@ -7,6 +7,6 @@ exc: Type[Exception] = Exception
try:
1 / 0
v = 1 / 0
except exc:
print("exc")

View File

@ -5,17 +5,17 @@ from typing import Tuple
def func1(t1: Tuple[int, ...], t2: Tuple[int, ...]):
t1 >= t2
return t1 >= t2
def func2(t1: Tuple[int, ...], t2: Tuple[str, int]):
t1 < t2
return t1 < t2
def func3(t1: Tuple[int, int], t2: Tuple[int, ...]):
t1 > t2
return t1 > t2
def func4(t1: Tuple[int, ...], t2: Tuple[str, ...]):
# This should generate an error
t1 <= t2
return t1 <= t2

View File

@ -0,0 +1,31 @@
# This sample tests the reportUnusedExpression diagnostic rule.
t = 1
# This should generate a diagnostic.
-4
# This should generate a diagnostic.
4j
# This should generate a diagnostic.
4j + 4
# This should generate a diagnostic.
False
# This should generate a diagnostic.
t == 1
# This should generate a diagnostic.
t != 2
# This should generate a diagnostic.
t <= t
# This should generate a diagnostic.
not t
# This should generate a diagnostic.
None

View File

@ -729,6 +729,17 @@
"error"
]
},
"reportUnusedExpression": {
"type": "string",
"description": "Diagnostics for simple expressions whose value is not used in any way.",
"default": "warning",
"enum": [
"none",
"information",
"warning",
"error"
]
},
"reportUnnecessaryTypeIgnoreComment": {
"type": "string",
"description": "Diagnostics for '# type: ignore' comments that have no effect.",

View File

@ -467,6 +467,12 @@
"title": "Controls reporting of call expressions that returns Coroutine whose results are not consumed",
"default": "error"
},
"reportUnusedExpression": {
"$id": "#/properties/reportUnusedExpression",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of simple expressions whose value is not used in any way",
"default": "warning"
},
"reportUnnecessaryTypeIgnoreComment": {
"$id": "#/properties/reportUnnecessaryTypeIgnoreComment",
"$ref": "#/definitions/diagnostic",