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 1 commit
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
24 changes: 20 additions & 4 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ def _has_locals_call_after_node(stmt: nodes.NodeNG, scope: nodes.FunctionDef) ->
"invalid-all-format",
"Used when __all__ has an invalid format.",
),
"E0606": (
"Possibly using variable %r before assignment",
"possibly-used-before-assignment",
"Emitted when a local variable is accessed before its assignment took place "
"in both branches of an if/else switch.",
),
"E0611": (
"No name %r in module %r",
"no-name-in-module",
Expand Down Expand Up @@ -538,6 +544,7 @@ def __init__(self, node: nodes.NodeNG, scope_type: str) -> None:
)
self.node = node
self.names_under_always_false_test: set[str] = set()
self.names_defined_under_one_branch_only: set[str] = set()

def __repr__(self) -> str:
_to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()]
Expand Down Expand Up @@ -744,9 +751,13 @@ def _inferred_to_define_name_raise_or_return(
self.names_under_always_false_test.add(name)
return self._branch_handles_name(name, node.orelse)
# Search both if and else branches
return self._branch_handles_name(name, node.body) and self._branch_handles_name(
name, node.orelse
)
if_branch_handles = self._branch_handles_name(name, node.body)
else_branch_handles = self._branch_handles_name(name, node.orelse)
if if_branch_handles ^ else_branch_handles:
self.names_defined_under_one_branch_only.add(name)
elif name in self.names_defined_under_one_branch_only:
self.names_defined_under_one_branch_only.remove(name)
return if_branch_handles and else_branch_handles

def _branch_handles_name(self, name: str, body: Iterable[nodes.NodeNG]) -> bool:
return any(
Expand Down Expand Up @@ -1996,8 +2007,13 @@ def _report_unfound_name_definition(
elif node.name in current_consumer.consumed_uncertain:
confidence = CONTROL_FLOW

if node.name in current_consumer.names_defined_under_one_branch_only:
msg = "possibly-used-before-assignment"
else:
msg = "used-before-assignment"

self.add_message(
"used-before-assignment",
msg,
args=node.name,
node=node,
confidence=confidence,
Expand Down
1 change: 1 addition & 0 deletions pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ disable=
# We anticipate #3512 where it will become optional
fixme,
consider-using-assignment-expr,
possibly-used-before-assignment,


[REPORTS]
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/u/used/used_before_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def redefine_time_import_with_global():
pass
else:
VAR4 = False
if VAR4: # [used-before-assignment]
if VAR4: # [possibly-used-before-assignment]
pass

if FALSE:
Expand All @@ -70,7 +70,7 @@ def redefine_time_import_with_global():
VAR5 = True
else:
VAR5 = True
if VAR5: # [used-before-assignment]
if VAR5: # [possibly-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) # [used-before-assignment]
print(VAR12) # [possibly-used-before-assignment]

def turn_on2(**kwargs):
"""https://github.com/pylint-dev/pylint/issues/7873"""
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/u/used/used_before_assignment.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ used-before-assignment:19:20:19:40:ClassWithProperty:Using variable 'redefine_ti
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: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
possibly-used-before-assignment:63:3:63:7::Possibly using variable 'VAR4' before assignment:INFERENCE
possibly-used-before-assignment:73:3:73:7::Possibly 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
possibly-used-before-assignment:119:6:119:11::Possibly 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
used-before-assignment:187:6:187:24::Using variable 'NOT_ALWAYS_DEFINED' before assignment:INFERENCE
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def used_before_assignment_2(a):


def used_before_assignment_3(a):
if x == a: # [used-before-assignment]
if x == a: # [possibly-used-before-assignment]
if x > 3:
x = 2 # [redefined-outer-name]
Comment on lines -19 to 21
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a shame, but I can live with it.


Expand Down
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:CONTROL_FLOW
possibly-used-before-assignment:19:7:19:8:used_before_assignment_3:Possibly 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
6 changes: 3 additions & 3 deletions tests/functional/u/used/used_before_assignment_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ 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: # [used-before-assignment]
print(bisect) # [used-before-assignment]
def defined_in_elif_branch(self) -> calendar.Calendar: # [possibly-used-before-assignment]
print(bisect) # [possibly-used-before-assignment]
return calendar.Calendar()

def defined_in_else_branch(self) -> urlopen:
Expand All @@ -177,7 +177,7 @@ def defined_in_else_branch(self) -> urlopen:
print(collections())
return urlopen

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

Expand Down
6 changes: 3 additions & 3 deletions tests/functional/u/used/used_before_assignment_typing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ undefined-variable:74:26:74:33:MyClass.incorrect_nested_typing_method: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: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
possibly-used-before-assignment:170:40:170:48:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly using variable 'calendar' before assignment:INFERENCE
possibly-used-before-assignment:171:14:171:20:TypeCheckingMultiBranch.defined_in_elif_branch:Possibly 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
possibly-used-before-assignment:180:43:180:48:TypeCheckingMultiBranch.defined_in_nested_if_else:Possibly 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
Expand Down
Loading