Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit possibly-used-before-assignment after if/else switches #8952

Merged
merged 20 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/1727.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Emit ``used-before-assignment`` when relying on names after an ``if/else``
switch when one branch failed to define the name, raise, or return.

Closes #1727
19 changes: 19 additions & 0 deletions doc/whatsnew/fragments/1727.false_negative.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
``used-before-assignment`` is now emitted when relying on variable assignments
that were not exhaustively made in every if/else branch.

If you rely on a pattern like this:
```
if guarded():
var = 1

if guarded():
print(var) # now emits used-before-assignment
```
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved

...you may be concerned that ``used-before-assignment`` is not totally useful
in this instance. However, consider that pylint, as a static analysis tool, does
not know if ``guarded()`` is deterministic, has side effects, or talks to
a database. (Likewise, for ``guarded`` instead of ``guarded()``, any other
part of your program may have changed the value in the meantime.)

Closes #1727
44 changes: 22 additions & 22 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ def __init__(self, node: nodes.NodeNG, scope_type: str) -> None:
copy.copy(node.locals), {}, collections.defaultdict(list), scope_type
)
self.node = node
self.names_under_always_false_test = set()

def __repr__(self) -> str:
_to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()]
Expand Down Expand Up @@ -637,7 +638,7 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:

# Filter out assignments guarded by always false conditions
if found_nodes:
uncertain_nodes = self._uncertain_nodes_in_false_tests(found_nodes, node)
uncertain_nodes = self._uncertain_nodes_if_tests(found_nodes, node)
self.consumed_uncertain[node.name] += uncertain_nodes
uncertain_nodes_set = set(uncertain_nodes)
found_nodes = [n for n in found_nodes if n not in uncertain_nodes_set]
Expand Down Expand Up @@ -687,8 +688,9 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:

return found_nodes

@staticmethod
def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> bool:
def _inferred_to_define_name_raise_or_return(
self, name: str, node: nodes.NodeNG
) -> bool:
"""Return True if there is a path under this `if_node`
that is inferred to define `name`, raise, or return.
"""
Expand Down Expand Up @@ -738,17 +740,14 @@ def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> b

# Only search else branch when test condition is inferred to be false
if all_inferred and only_search_else:
return NamesConsumer._branch_handles_name(name, node.orelse)
# Only search if branch when test condition is inferred to be true
if all_inferred and only_search_if:
return NamesConsumer._branch_handles_name(name, node.body)
self.names_under_always_false_test.add(name)
return self._branch_handles_name(name, node.orelse)
# Search both if and else branches
return NamesConsumer._branch_handles_name(
name, node.body
) or NamesConsumer._branch_handles_name(name, node.orelse)
return self._branch_handles_name(name, node.body) and self._branch_handles_name(
name, node.orelse
)

@staticmethod
def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool:
def _branch_handles_name(self, name: str, body: Iterable[nodes.NodeNG]) -> bool:
return any(
NamesConsumer._defines_name_raises_or_returns(name, if_body_stmt)
or isinstance(
Expand All @@ -761,17 +760,15 @@ def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool:
nodes.While,
),
)
and NamesConsumer._inferred_to_define_name_raise_or_return(
name, if_body_stmt
)
and self._inferred_to_define_name_raise_or_return(name, if_body_stmt)
for if_body_stmt in body
)

def _uncertain_nodes_in_false_tests(
def _uncertain_nodes_if_tests(
self, found_nodes: list[nodes.NodeNG], node: nodes.NodeNG
) -> list[nodes.NodeNG]:
"""Identify nodes of uncertain execution because they are defined under
tests that evaluate false.
"""Identify nodes of uncertain execution because they are defined under if
tests.

Don't identify a node if there is a path that is inferred to
define the name, raise, or return (e.g. any executed if/elif/else branch).
Expand Down Expand Up @@ -807,7 +804,7 @@ def _uncertain_nodes_in_false_tests(
continue

# Name defined in the if/else control flow
if NamesConsumer._inferred_to_define_name_raise_or_return(name, outer_if):
if self._inferred_to_define_name_raise_or_return(name, outer_if):
continue

uncertain_nodes.append(other_node)
Expand Down Expand Up @@ -1974,9 +1971,12 @@ def _report_unfound_name_definition(
):
return

confidence = (
CONTROL_FLOW if node.name in current_consumer.consumed_uncertain else HIGH
)
confidence = HIGH
if node.name in current_consumer.consumed_uncertain:
confidence = CONTROL_FLOW
if node.name in current_consumer.names_under_always_false_test:
confidence = INFERENCE

self.add_message(
"used-before-assignment",
args=node.name,
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/undefined/undefined_variable.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ undefined-variable:166:4:166:13::Undefined variable 'unicode_2':UNDEFINED
undefined-variable:171:4:171:13::Undefined variable 'unicode_3':UNDEFINED
undefined-variable:226:25:226:37:LambdaClass4.<lambda>:Undefined variable 'LambdaClass4':UNDEFINED
undefined-variable:234:25:234:37:LambdaClass5.<lambda>:Undefined variable 'LambdaClass5':UNDEFINED
used-before-assignment:255:26:255:34:func_should_fail:Using variable 'datetime' before assignment:CONTROL_FLOW
used-before-assignment:255:26:255:34:func_should_fail:Using variable 'datetime' before assignment:INFERENCE
undefined-variable:291:18:291:24:not_using_loop_variable_accordingly:Undefined variable 'iteree':UNDEFINED
undefined-variable:308:27:308:28:undefined_annotation:Undefined variable 'x':UNDEFINED
used-before-assignment:309:7:309:8:undefined_annotation:Using variable 'x' before assignment:HIGH
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/u/undefined/undefined_variable_py38.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ undefined-variable:106:6:106:19::Undefined variable 'else_assign_2':INFERENCE
used-before-assignment:141:10:141:16:type_annotation_used_improperly_after_comprehension:Using variable 'my_int' before assignment:HIGH
used-before-assignment:148:10:148:16:type_annotation_used_improperly_after_comprehension_2:Using variable 'my_int' before assignment:HIGH
used-before-assignment:186:9:186:10::Using variable 'z' before assignment:HIGH
used-before-assignment:193:6:193:19::Using variable 'NEVER_DEFINED' before assignment:CONTROL_FLOW
used-before-assignment:193:6:193:19::Using variable 'NEVER_DEFINED' before assignment:INFERENCE
4 changes: 2 additions & 2 deletions tests/functional/u/used/used_before_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def redefine_time_import_with_global():
VAR5 = True
else:
VAR5 = True
if VAR5:
if VAR5: # [used-before-assignment]
pass

if FALSE:
Expand Down Expand Up @@ -116,7 +116,7 @@ def redefine_time_import_with_global():
VAR11 = num
if VAR11:
VAR12 = False
print(VAR12)
print(VAR12) # [used-before-assignment]

def turn_on2(**kwargs):
"""https://github.com/pylint-dev/pylint/issues/7873"""
Expand Down
14 changes: 8 additions & 6 deletions tests/functional/u/used/used_before_assignment.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ used-before-assignment:10:4:10:9:outer:Using variable 'inner' before assignment:
used-before-assignment:19:20:19:40:ClassWithProperty:Using variable 'redefine_time_import' before assignment:HIGH
used-before-assignment:23:0:23:9::Using variable 'calculate' before assignment:HIGH
used-before-assignment:31:10:31:14:redefine_time_import:Using variable 'time' before assignment:HIGH
used-before-assignment:45:3:45:7::Using variable 'VAR2' before assignment:CONTROL_FLOW
used-before-assignment:63:3:63:7::Using variable 'VAR4' before assignment:CONTROL_FLOW
used-before-assignment:78:3:78:7::Using variable 'VAR6' before assignment:CONTROL_FLOW
used-before-assignment:113:6:113:11::Using variable 'VAR10' before assignment:CONTROL_FLOW
used-before-assignment:144:10:144:14::Using variable 'SALE' before assignment:CONTROL_FLOW
used-before-assignment:176:10:176:18::Using variable 'ALL_DONE' before assignment:CONTROL_FLOW
used-before-assignment:45:3:45:7::Using variable 'VAR2' before assignment:INFERENCE
used-before-assignment:63:3:63:7::Using variable 'VAR4' before assignment:INFERENCE
used-before-assignment:73:3:73:7::Using variable 'VAR5' before assignment:INFERENCE
used-before-assignment:78:3:78:7::Using variable 'VAR6' before assignment:INFERENCE
used-before-assignment:113:6:113:11::Using variable 'VAR10' before assignment:INFERENCE
used-before-assignment:119:6:119:11::Using variable 'VAR12' before assignment:CONTROL_FLOW
used-before-assignment:144:10:144:14::Using variable 'SALE' before assignment:INFERENCE
used-before-assignment:176:10:176:18::Using variable 'ALL_DONE' before assignment:INFERENCE
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
used-before-assignment:16:14:16:29:function:Using variable 'failure_message' before assignment:CONTROL_FLOW
used-before-assignment:120:10:120:13:func_invalid1:Using variable 'msg' before assignment:CONTROL_FLOW
used-before-assignment:131:10:131:13:func_invalid2:Using variable 'msg' before assignment:CONTROL_FLOW
used-before-assignment:150:10:150:13:func_invalid3:Using variable 'msg' before assignment:CONTROL_FLOW
used-before-assignment:150:10:150:13:func_invalid3:Using variable 'msg' before assignment:INFERENCE
used-before-assignment:163:10:163:13:func_invalid4:Using variable 'msg' before assignment:CONTROL_FLOW
used-before-assignment:175:10:175:13:func_invalid5:Using variable 'msg' before assignment:CONTROL_FLOW
used-before-assignment:187:10:187:13:func_invalid6:Using variable 'msg' before assignment:CONTROL_FLOW
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ used-before-assignment:7:7:7:8:used_before_assignment_1:Using variable 'x' befor
redefined-outer-name:8:12:8:13:used_before_assignment_1:Redefining name 'x' from outer scope (line 3):UNDEFINED
used-before-assignment:13:7:13:8:used_before_assignment_2:Using variable 'x' before assignment:HIGH
redefined-outer-name:15:4:15:5:used_before_assignment_2:Redefining name 'x' from outer scope (line 3):UNDEFINED
used-before-assignment:19:7:19:8:used_before_assignment_3:Using variable 'x' before assignment:HIGH
used-before-assignment:19:7:19:8:used_before_assignment_3:Using variable 'x' before assignment:CONTROL_FLOW
redefined-outer-name:21:12:21:13:used_before_assignment_3:Redefining name 'x' from outer scope (line 3):UNDEFINED
redefined-outer-name:30:4:30:5:not_used_before_assignment_2:Redefining name 'x' from outer scope (line 3):UNDEFINED
4 changes: 2 additions & 2 deletions tests/functional/u/used/used_before_assignment_issue2615.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
used-before-assignment:12:14:12:17:main:Using variable 'res' before assignment:CONTROL_FLOW
used-before-assignment:12:14:12:17:main:Using variable 'res' before assignment:INFERENCE
used-before-assignment:30:18:30:35:nested_except_blocks:Using variable 'more_bad_division' before assignment:CONTROL_FLOW
used-before-assignment:31:18:31:21:nested_except_blocks:Using variable 'res' before assignment:CONTROL_FLOW
used-before-assignment:31:18:31:21:nested_except_blocks:Using variable 'res' before assignment:INFERENCE
Original file line number Diff line number Diff line change
@@ -1 +1 @@
used-before-assignment:10:6:10:9::Using variable 'var' before assignment:CONTROL_FLOW
used-before-assignment:10:6:10:9::Using variable 'var' before assignment:INFERENCE
4 changes: 2 additions & 2 deletions tests/functional/u/used/used_before_assignment_scoping.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
used-before-assignment:10:13:10:21:func_two:Using variable 'datetime' before assignment:CONTROL_FLOW
used-before-assignment:16:12:16:20:func:Using variable 'datetime' before assignment:CONTROL_FLOW
used-before-assignment:10:13:10:21:func_two:Using variable 'datetime' before assignment:INFERENCE
used-before-assignment:16:12:16:20:func:Using variable 'datetime' before assignment:INFERENCE
28 changes: 14 additions & 14 deletions tests/functional/u/used/used_before_assignment_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,32 +167,32 @@ class ConditionalImportGuardedWhenUsed: # pylint: disable=too-few-public-method

class TypeCheckingMultiBranch: # pylint: disable=too-few-public-methods,unused-variable
"""Test for defines in TYPE_CHECKING if/elif/else branching"""
def defined_in_elif_branch(self) -> calendar.Calendar:
print(bisect)
def defined_in_elif_branch(self) -> calendar.Calendar: # [used-before-assignment]
print(bisect) # [used-before-assignment]
return calendar.Calendar()

def defined_in_else_branch(self) -> urlopen:
print(zoneinfo)
print(zoneinfo) # [used-before-assignment]
print(pprint())
print(collections())
return urlopen

def defined_in_nested_if_else(self) -> heapq:
def defined_in_nested_if_else(self) -> heapq: # [used-before-assignment]
print(heapq)
return heapq

def defined_in_try_except(self) -> array:
print(types)
print(copy)
print(numbers)
def defined_in_try_except(self) -> array: # [used-before-assignment]
print(types) # [used-before-assignment]
print(copy) # [used-before-assignment]
print(numbers) # [used-before-assignment]
return array

def defined_in_loops(self) -> json:
print(email)
print(mailbox)
print(mimetypes)
def defined_in_loops(self) -> json: # [used-before-assignment]
print(email) # [used-before-assignment]
print(mailbox) # [used-before-assignment]
print(mimetypes) # [used-before-assignment]
return json

def defined_in_with(self) -> base64:
print(binascii)
def defined_in_with(self) -> base64: # [used-before-assignment]
print(binascii) # [used-before-assignment]
return base64
18 changes: 16 additions & 2 deletions tests/functional/u/used/used_before_assignment_typing.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
undefined-variable:69:21:69:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:74:26:74:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:79:20:79:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED
used-before-assignment:140:35:140:39:MyFourthClass.is_close:Using variable 'math' before assignment:CONTROL_FLOW
used-before-assignment:153:20:153:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:CONTROL_FLOW
used-before-assignment:140:35:140:39:MyFourthClass.is_close:Using variable 'math' before assignment:INFERENCE
used-before-assignment:153:20:153:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:INFERENCE
used-before-assignment:170:40:170:48:TypeCheckingMultiBranch.defined_in_elif_branch:Using variable 'calendar' before assignment:INFERENCE
used-before-assignment:171:14:171:20:TypeCheckingMultiBranch.defined_in_elif_branch:Using variable 'bisect' before assignment:INFERENCE
used-before-assignment:175:14:175:22:TypeCheckingMultiBranch.defined_in_else_branch:Using variable 'zoneinfo' before assignment:INFERENCE
used-before-assignment:180:43:180:48:TypeCheckingMultiBranch.defined_in_nested_if_else:Using variable 'heapq' before assignment:INFERENCE
used-before-assignment:184:39:184:44:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'array' before assignment:INFERENCE
used-before-assignment:185:14:185:19:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'types' before assignment:INFERENCE
used-before-assignment:186:14:186:18:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'copy' before assignment:INFERENCE
used-before-assignment:187:14:187:21:TypeCheckingMultiBranch.defined_in_try_except:Using variable 'numbers' before assignment:INFERENCE
used-before-assignment:190:34:190:38:TypeCheckingMultiBranch.defined_in_loops:Using variable 'json' before assignment:INFERENCE
used-before-assignment:191:14:191:19:TypeCheckingMultiBranch.defined_in_loops:Using variable 'email' before assignment:INFERENCE
used-before-assignment:192:14:192:21:TypeCheckingMultiBranch.defined_in_loops:Using variable 'mailbox' before assignment:INFERENCE
used-before-assignment:193:14:193:23:TypeCheckingMultiBranch.defined_in_loops:Using variable 'mimetypes' before assignment:INFERENCE
used-before-assignment:196:33:196:39:TypeCheckingMultiBranch.defined_in_with:Using variable 'base64' before assignment:INFERENCE
used-before-assignment:197:14:197:22:TypeCheckingMultiBranch.defined_in_with:Using variable 'binascii' before assignment:INFERENCE
Loading