Skip to content

Commit

Permalink
Distinguish redundant-expr warnings from unreachable warnings (#9125)
Browse files Browse the repository at this point in the history
  • Loading branch information
llchan authored Sep 24, 2020
1 parent 835b427 commit f2dfd91
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 16 deletions.
21 changes: 21 additions & 0 deletions docs/source/error_code_list2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 --enable-error-code>`,
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)]
19 changes: 14 additions & 5 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -2741,6 +2741,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 redundant.
#
# 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 codes.REDUNDANT_EXPR in self.chk.options.enabled_error_codes:
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.
Expand All @@ -2750,10 +2761,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
Expand Down Expand Up @@ -3674,7 +3683,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 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:
Expand All @@ -3687,7 +3696,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 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:
Expand Down
5 changes: 5 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ 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',
default_enabled=False) # type: Final

# Syntax errors are often blocking.
SYNTAX = ErrorCode(
Expand Down
2 changes: 1 addition & 1 deletion mypy/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ 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)

# Note: this group is intentionally added here even though we don't add
Expand Down
4 changes: 2 additions & 2 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions test-data/unit/check-errorcodes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
9 changes: 1 addition & 8 deletions test-data/unit/check-unreachable-code.test
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit f2dfd91

Please sign in to comment.