From 00f66c9eae54d1abb77b8b45033899566e7acbd8 Mon Sep 17 00:00:00 2001 From: Lawrence Chan Date: Fri, 10 Jul 2020 14:37:18 -0500 Subject: [PATCH] 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 5af1147673572..ad7c1f5227061 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 @@ -3669,7 +3678,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: @@ -3682,7 +3691,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 47206c53e9de3..02bc8d0421bbe 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -105,6 +105,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 8e80364234b4f..580223b051b64 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -575,7 +575,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 8b689861548fd..6067c162ad058 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 54e106d2efe7a..58e07b5f81ef9 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -48,6 +48,7 @@ class BuildType: "strict_optional_whitelist", "warn_no_return", "warn_return_any", + "warn_redundant_expr", "warn_unreachable", "warn_unused_ignores", } # type: Final @@ -171,6 +172,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 49a85861b6e32..3885668cc90af 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 0000000000000..2c77e84105cc0 --- /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 262ac86e49ade..f5b49d87289a3 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]