From 70b10cdcf586c7db9b64be2c53559e27576fe2da Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Fri, 10 Jul 2020 14:37:18 -0500 Subject: [PATCH 1/7] Distinguish redundant-expr warnings from unreachable warnings --- mypy/checkexpr.py | 19 ++++++++++++++----- mypy/errorcodes.py | 2 ++ mypy/main.py | 5 ++++- mypy/messages.py | 4 ++-- mypy/options.py | 5 +++++ mypy/test/testcheck.py | 1 + test-data/unit/check-redundant-expr.test | 17 +++++++++++++++++ test-data/unit/check-unreachable-code.test | 9 +-------- 8 files changed, 46 insertions(+), 16 deletions(-) create mode 100644 test-data/unit/check-redundant-expr.test diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index aa371548127e..d202d8343fe1 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -2740,6 +2740,17 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: restricted_left_type = true_only(left_type) result_is_left = not left_type.can_be_false + # If left_map is None then we know mypy considers the left expression + # to be reundant. + # + # Note that we perform these checks *before* we take into account + # the analysis from the semanal phase below. We assume that nodes + # marked as unreachable during semantic analysis were done so intentionally. + # So, we shouldn't report an error. + if self.chk.options.warn_redundant_expr: + if left_map is None: + self.msg.redundant_left_operand(e.op, e.left) + # If right_map is None then we know mypy considers the right branch # to be unreachable and therefore any errors found in the right branch # should be suppressed. @@ -2749,10 +2760,8 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: # marked as unreachable during semantic analysis were done so intentionally. # So, we shouldn't report an error. if self.chk.options.warn_unreachable: - if left_map is None: - self.msg.redundant_left_operand(e.op, e.left) if right_map is None: - self.msg.redundant_right_operand(e.op, e.right) + self.msg.unreachable_right_operand(e.op, e.right) if e.right_unreachable: right_map = None @@ -3673,7 +3682,7 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No for var, type in true_map.items(): self.chk.binder.put(var, type) - if self.chk.options.warn_unreachable: + if self.chk.options.warn_redundant_expr: if true_map is None: self.msg.redundant_condition_in_comprehension(False, condition) elif false_map is None: @@ -3686,7 +3695,7 @@ def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = F # Gain type information from isinstance if it is there # but only for the current expression if_map, else_map = self.chk.find_isinstance_check(e.cond) - if self.chk.options.warn_unreachable: + if self.chk.options.warn_redundant_expr: if if_map is None: self.msg.redundant_condition_in_if(False, e.cond) elif else_map is None: diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index 0df1989d17dd..d4bc911ed36f 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -112,6 +112,8 @@ def __str__(self) -> str: 'General') # type: Final UNREACHABLE = ErrorCode( 'unreachable', "Warn about unreachable statements or expressions", 'General') # type: Final +REDUNDANT_EXPR = ErrorCode( + 'redundant-expr', "Warn about redundant expressions", 'General') # type: Final # Syntax errors are often blocking. SYNTAX = ErrorCode( diff --git a/mypy/main.py b/mypy/main.py index 7038ce40a5c1..2fc0a958082b 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -576,7 +576,10 @@ def add_invertible_flag(flag: str, group=lint_group) add_invertible_flag('--warn-unreachable', default=False, strict_flag=False, help="Warn about statements or expressions inferred to be" - " unreachable or redundant", + " unreachable", + group=lint_group) + add_invertible_flag('--warn-redundant-expr', default=False, strict_flag=False, + help="Warn about expressions inferred to be redundant", group=lint_group) # Note: this group is intentionally added here even though we don't add diff --git a/mypy/messages.py b/mypy/messages.py index 8b689861548f..6067c162ad05 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1280,7 +1280,7 @@ def redundant_left_operand(self, op_name: str, context: Context) -> None: """ self.redundant_expr("Left operand of '{}'".format(op_name), op_name == 'and', context) - def redundant_right_operand(self, op_name: str, context: Context) -> None: + def unreachable_right_operand(self, op_name: str, context: Context) -> None: """Indicates that the right operand of a boolean expression is redundant: it does not change the truth value of the entire condition as a whole. 'op_name' should either be the string "and" or the string "or". @@ -1299,7 +1299,7 @@ def redundant_condition_in_assert(self, truthiness: bool, context: Context) -> N def redundant_expr(self, description: str, truthiness: bool, context: Context) -> None: self.fail("{} is always {}".format(description, str(truthiness).lower()), - context, code=codes.UNREACHABLE) + context, code=codes.REDUNDANT_EXPR) def impossible_intersection(self, formatted_base_class_list: str, diff --git a/mypy/options.py b/mypy/options.py index 975c3d8b40f6..4e3b6c7f7bc1 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -51,6 +51,7 @@ class BuildType: "strict_optional_whitelist", "warn_no_return", "warn_return_any", + "warn_redundant_expr", "warn_unreachable", "warn_unused_ignores", } # type: Final @@ -174,6 +175,10 @@ def __init__(self) -> None: # type analysis. self.warn_unreachable = False + # Report an error for any expressions inferred to be redundant as a result of + # type analysis. + self.warn_redundant_expr = False + # Variable names considered True self.always_true = [] # type: List[str] diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py index 49a85861b6e3..3885668cc90a 100644 --- a/mypy/test/testcheck.py +++ b/mypy/test/testcheck.py @@ -42,6 +42,7 @@ 'check-typevar-values.test', 'check-unsupported.test', 'check-unreachable-code.test', + 'check-redundant-expr.test', 'check-unions.test', 'check-isinstance.test', 'check-lists.test', diff --git a/test-data/unit/check-redundant-expr.test b/test-data/unit/check-redundant-expr.test new file mode 100644 index 000000000000..2c77e84105cc --- /dev/null +++ b/test-data/unit/check-redundant-expr.test @@ -0,0 +1,17 @@ +-- Type checker test cases for conditional checks that result in some +-- expressions classified as redundant. + +[case testRedundantExpressions] +# flags: --warn-redundant-expr +def foo() -> bool: ... + +lst = [1, 2, 3, 4] + +b = False or foo() # E: Left operand of 'or' is always false +c = True and foo() # E: Left operand of 'and' is always true +g = 3 if True else 4 # E: If condition is always true +h = 3 if False else 4 # E: If condition is always false +i = [x for x in lst if True] # E: If condition in comprehension is always true +j = [x for x in lst if False] # E: If condition in comprehension is always false +k = [x for x in lst if isinstance(x, int) or foo()] # E: If condition in comprehension is always true +[builtins fixtures/isinstancelist.pyi] diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index 262ac86e49ad..f5b49d87289a 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -898,18 +898,11 @@ def foo() -> bool: ... lst = [1, 2, 3, 4] a = True or foo() # E: Right operand of 'or' is never evaluated -b = False or foo() # E: Left operand of 'or' is always false -c = True and foo() # E: Left operand of 'and' is always true d = False and foo() # E: Right operand of 'and' is never evaluated e = True or (True or (True or foo())) # E: Right operand of 'or' is never evaluated f = (True or foo()) or (True or foo()) # E: Right operand of 'or' is never evaluated -g = 3 if True else 4 # E: If condition is always true -h = 3 if False else 4 # E: If condition is always false -i = [x for x in lst if True] # E: If condition in comprehension is always true -j = [x for x in lst if False] # E: If condition in comprehension is always false -k = [x for x in lst if isinstance(x, int) or foo()] # E: If condition in comprehension is always true \ - # E: Right operand of 'or' is never evaluated +k = [x for x in lst if isinstance(x, int) or foo()] # E: Right operand of 'or' is never evaluated [builtins fixtures/isinstancelist.pyi] [case testUnreachableFlagMiscTestCaseMissingMethod] From a70cca803bcfaafc0366b1ed3f1ac3195d829ae6 Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Tue, 11 Aug 2020 21:46:34 -0500 Subject: [PATCH 2/7] Use new --enable-error-codes functionlity --- mypy/checkexpr.py | 6 +++--- mypy/main.py | 3 --- mypy/options.py | 5 ----- test-data/unit/check-redundant-expr.test | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index d202d8343fe1..65d192e252fa 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -2747,7 +2747,7 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: # the analysis from the semanal phase below. We assume that nodes # marked as unreachable during semantic analysis were done so intentionally. # So, we shouldn't report an error. - if self.chk.options.warn_redundant_expr: + if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes: if left_map is None: self.msg.redundant_left_operand(e.op, e.left) @@ -3682,7 +3682,7 @@ def check_for_comp(self, e: Union[GeneratorExpr, DictionaryComprehension]) -> No for var, type in true_map.items(): self.chk.binder.put(var, type) - if self.chk.options.warn_redundant_expr: + if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes: if true_map is None: self.msg.redundant_condition_in_comprehension(False, condition) elif false_map is None: @@ -3695,7 +3695,7 @@ def visit_conditional_expr(self, e: ConditionalExpr, allow_none_return: bool = F # Gain type information from isinstance if it is there # but only for the current expression if_map, else_map = self.chk.find_isinstance_check(e.cond) - if self.chk.options.warn_redundant_expr: + if codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes: if if_map is None: self.msg.redundant_condition_in_if(False, e.cond) elif else_map is None: diff --git a/mypy/main.py b/mypy/main.py index 2fc0a958082b..6917d08c691d 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -578,9 +578,6 @@ def add_invertible_flag(flag: str, help="Warn about statements or expressions inferred to be" " unreachable", group=lint_group) - add_invertible_flag('--warn-redundant-expr', default=False, strict_flag=False, - help="Warn about expressions inferred to be redundant", - group=lint_group) # Note: this group is intentionally added here even though we don't add # --strict to this group near the end. diff --git a/mypy/options.py b/mypy/options.py index 4e3b6c7f7bc1..975c3d8b40f6 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -51,7 +51,6 @@ class BuildType: "strict_optional_whitelist", "warn_no_return", "warn_return_any", - "warn_redundant_expr", "warn_unreachable", "warn_unused_ignores", } # type: Final @@ -175,10 +174,6 @@ def __init__(self) -> None: # type analysis. self.warn_unreachable = False - # Report an error for any expressions inferred to be redundant as a result of - # type analysis. - self.warn_redundant_expr = False - # Variable names considered True self.always_true = [] # type: List[str] diff --git a/test-data/unit/check-redundant-expr.test b/test-data/unit/check-redundant-expr.test index 2c77e84105cc..6e3fe685a1c6 100644 --- a/test-data/unit/check-redundant-expr.test +++ b/test-data/unit/check-redundant-expr.test @@ -2,7 +2,7 @@ -- expressions classified as redundant. [case testRedundantExpressions] -# flags: --warn-redundant-expr +# flags: --enable-error-code redundant-expr def foo() -> bool: ... lst = [1, 2, 3, 4] From c21bc4d412530cb198c9aeb0cd6a200d6549f27d Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Fri, 14 Aug 2020 14:10:24 -0500 Subject: [PATCH 3/7] Set redundant-expr to disabled by default --- mypy/errorcodes.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index d4bc911ed36f..bbcc6e854260 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -113,7 +113,10 @@ def __str__(self) -> str: UNREACHABLE = ErrorCode( 'unreachable', "Warn about unreachable statements or expressions", 'General') # type: Final REDUNDANT_EXPR = ErrorCode( - 'redundant-expr', "Warn about redundant expressions", 'General') # type: Final + 'redundant-expr', + "Warn about redundant expressions", + 'General', + default_enabled=False) # type: Final # Syntax errors are often blocking. SYNTAX = ErrorCode( From 5676631892d2aadb438afccfb57a4c3602eb34fa Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Fri, 14 Aug 2020 14:10:35 -0500 Subject: [PATCH 4/7] Move redundant-expr tests to check-errorcodes.test --- test-data/unit/check-errorcodes.test | 15 +++++++++++++++ test-data/unit/check-redundant-expr.test | 17 ----------------- 2 files changed, 15 insertions(+), 17 deletions(-) delete mode 100644 test-data/unit/check-redundant-expr.test diff --git a/test-data/unit/check-errorcodes.test b/test-data/unit/check-errorcodes.test index c325f568081d..8e075fa8d1e9 100644 --- a/test-data/unit/check-errorcodes.test +++ b/test-data/unit/check-errorcodes.test @@ -771,3 +771,18 @@ def f(**x: int) -> None: f(**1) # type: ignore[arg-type] [builtins fixtures/dict.pyi] [typing fixtures/typing-typeddict.pyi] + +[case testRedundantExpressions] +# flags: --enable-error-code redundant-expr +def foo() -> bool: ... + +lst = [1, 2, 3, 4] + +b = False or foo() # E: Left operand of 'or' is always false [redundant-expr] +c = True and foo() # E: Left operand of 'and' is always true [redundant-expr] +g = 3 if True else 4 # E: If condition is always true [redundant-expr] +h = 3 if False else 4 # E: If condition is always false [redundant-expr] +i = [x for x in lst if True] # E: If condition in comprehension is always true [redundant-expr] +j = [x for x in lst if False] # E: If condition in comprehension is always false [redundant-expr] +k = [x for x in lst if isinstance(x, int) or foo()] # E: If condition in comprehension is always true [redundant-expr] +[builtins fixtures/isinstancelist.pyi] diff --git a/test-data/unit/check-redundant-expr.test b/test-data/unit/check-redundant-expr.test deleted file mode 100644 index 6e3fe685a1c6..000000000000 --- a/test-data/unit/check-redundant-expr.test +++ /dev/null @@ -1,17 +0,0 @@ --- Type checker test cases for conditional checks that result in some --- expressions classified as redundant. - -[case testRedundantExpressions] -# flags: --enable-error-code redundant-expr -def foo() -> bool: ... - -lst = [1, 2, 3, 4] - -b = False or foo() # E: Left operand of 'or' is always false -c = True and foo() # E: Left operand of 'and' is always true -g = 3 if True else 4 # E: If condition is always true -h = 3 if False else 4 # E: If condition is always false -i = [x for x in lst if True] # E: If condition in comprehension is always true -j = [x for x in lst if False] # E: If condition in comprehension is always false -k = [x for x in lst if isinstance(x, int) or foo()] # E: If condition in comprehension is always true -[builtins fixtures/isinstancelist.pyi] From bc5dcc815221f78a75b123c9f2ed77e2b596e9f1 Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Fri, 14 Aug 2020 14:27:52 -0500 Subject: [PATCH 5/7] Remove reference to deleted check-redundant-expr.test --- mypy/test/testcheck.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy/test/testcheck.py b/mypy/test/testcheck.py index 3885668cc90a..49a85861b6e3 100644 --- a/mypy/test/testcheck.py +++ b/mypy/test/testcheck.py @@ -42,7 +42,6 @@ 'check-typevar-values.test', 'check-unsupported.test', 'check-unreachable-code.test', - 'check-redundant-expr.test', 'check-unions.test', 'check-isinstance.test', 'check-lists.test', From da88269dfed525a6afd2b2414f2b90acc53816fb Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Thu, 24 Sep 2020 15:56:19 -0500 Subject: [PATCH 6/7] Fix typo in redundant expr comment --- mypy/checkexpr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 65d192e252fa..74687d2edf24 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -2741,7 +2741,7 @@ def check_boolean_op(self, e: OpExpr, context: Context) -> Type: result_is_left = not left_type.can_be_false # If left_map is None then we know mypy considers the left expression - # to be reundant. + # to be redundant. # # Note that we perform these checks *before* we take into account # the analysis from the semanal phase below. We assume that nodes From f8ee3bd3366259d1e54d58f739adcaaf8a603c98 Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Thu, 24 Sep 2020 16:22:45 -0500 Subject: [PATCH 7/7] Add documentation for redundant-expr --- docs/source/error_code_list2.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/source/error_code_list2.rst b/docs/source/error_code_list2.rst index c91c1ba20a2c..6f12e8a8d5eb 100644 --- a/docs/source/error_code_list2.rst +++ b/docs/source/error_code_list2.rst @@ -193,3 +193,24 @@ incorrect control flow or conditional checks that are accidentally always true o return # Error: Statement is unreachable [unreachable] print('unreachable') + +Check that expression is redundant [redundant-expr] +--------------------------------------------------- + +If you use :option:`--enable-error-code redundant-expr `, +mypy generates an error if it thinks that an expression is redundant. + +.. code-block:: python + + # mypy: enable-error-code redundant-expr + + def example(x: int) -> None: + # Error: Left operand of 'and' is always true [redundant-expr] + if isinstance(x, int) and x > 0: + pass + + # Error: If condition is always true [redundant-expr] + 1 if isinstance(x, int) else 0 + + # Error: If condition in comprehension is always true [redundant-expr] + [i for i in range(x) if isinstance(i, int)]