Skip to content

Commit

Permalink
Distinguish redundant-expr warnings from unreachable warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
llchan committed Jul 10, 2020
1 parent b1f5121 commit 00f66c9
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 16 deletions.
19 changes: 14 additions & 5 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions mypy/errorcodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 4 additions & 1 deletion mypy/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
5 changes: 5 additions & 0 deletions mypy/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]

Expand Down
1 change: 1 addition & 0 deletions mypy/test/testcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
17 changes: 17 additions & 0 deletions test-data/unit/check-redundant-expr.test
Original file line number Diff line number Diff line change
@@ -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]
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 00f66c9

Please sign in to comment.