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

Co-authored-by: Pierre Sassoulas <[email protected]>

Author: jacobtylerwalls

doc/data/messages/p/possibly-used-before-assignment/bad.py

@@ -0,0 +1,4 @@
+def check_lunchbox(items: list[str]):
+    if not items:
+        empty = True
+    print(empty)  # [possibly-used-before-assignment]

doc/data/messages/p/possibly-used-before-assignment/details.rst

@@ -0,0 +1,15 @@
+If you rely on a pattern like:
+
+.. sourcecode:: python
+
+    if guarded():
+        var = 1
+
+    if guarded():
+        print(var)  # emits possibly-used-before-assignment
+
+you may be concerned that ``possibly-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 or talks to
+a database. (Likewise, for ``guarded`` instead of ``guarded()``, any other
+part of your program may have changed its value in the meantime.)

doc/data/messages/p/possibly-used-before-assignment/good.py

@@ -0,0 +1,5 @@
+def check_lunchbox(items: list[str]):
+    empty = False
+    if not items:
+        empty = True
+    print(empty)

doc/user_guide/checkers/features.rst

@@ -1374,6 +1374,9 @@ Variables checker Messages
   Used when an invalid (non-string) object occurs in __all__.
 :no-name-in-module (E0611): *No name %r in module %r*
   Used when a name cannot be found in a module.
+:possibly-used-before-assignment (E0606): *Possibly using variable %r before assignment*
+  Emitted when a local variable is accessed before its assignment took place in
+  both branches of an if/else switch.
 :undefined-variable (E0602): *Undefined variable %r*
   Used when an undefined variable is accessed.
 :undefined-all-variable (E0603): *Undefined variable name %r in __all__*

doc/user_guide/messages/messages_overview.rst

@@ -139,6 +139,7 @@ All messages in the error category:
    error/not-in-loop
    error/notimplemented-raised
    error/positional-only-arguments-expected
+   error/possibly-used-before-assignment
    error/potential-index-error
    error/raising-bad-type
    error/raising-non-exception

doc/whatsnew/fragments/1727.new_check

@@ -0,0 +1,4 @@
+Add check ``possibly-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

pylint/checkers/variables.py

@@ -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",
@@ -537,6 +543,8 @@ 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[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()]
@@ -636,13 +644,6 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:
         if VariablesChecker._comprehension_between_frame_and_node(node):
             return found_nodes
 
-        # Filter out assignments guarded by always false conditions
-        if found_nodes:
-            uncertain_nodes = self._uncertain_nodes_in_false_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]
-
         # Filter out assignments in ExceptHandlers that node is not contained in
         if found_nodes:
             found_nodes = [
@@ -652,6 +653,13 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None:
                 or n.statement().parent_of(node)
             ]
 
+        # Filter out assignments guarded by always false conditions
+        if found_nodes:
+            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]
+
         # Filter out assignments in an Except clause that the node is not
         # contained in, assuming they may fail
         if found_nodes:
@@ -688,8 +696,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.
         """
@@ -716,8 +725,8 @@ def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> b
         if not isinstance(node, nodes.If):
             return False
 
-        # Be permissive if there is a break
-        if any(node.nodes_of_class(nodes.Break)):
+        # Be permissive if there is a break or a continue
+        if any(node.nodes_of_class(nodes.Break, nodes.Continue)):
             return True
 
         # Is there an assignment in this node itself, e.g. in named expression?
@@ -739,17 +748,18 @@ 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)
-
-    @staticmethod
-    def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool:
+        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(
             NamesConsumer._defines_name_raises_or_returns(name, if_body_stmt)
             or isinstance(
@@ -762,17 +772,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).
@@ -808,7 +816,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)
@@ -930,7 +938,7 @@ def _uncertain_nodes_in_except_blocks(
 
     @staticmethod
     def _defines_name_raises_or_returns(name: str, node: nodes.NodeNG) -> bool:
-        if isinstance(node, (nodes.Raise, nodes.Assert, nodes.Return)):
+        if isinstance(node, (nodes.Raise, nodes.Assert, nodes.Return, nodes.Continue)):
             return True
         if (
             isinstance(node, nodes.AnnAssign)
@@ -1993,11 +2001,19 @@ 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.names_under_always_false_test:
+            confidence = INFERENCE
+        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,

pylintrc

@@ -109,6 +109,7 @@ disable=
     # We anticipate #3512 where it will become optional
     fixme,
     consider-using-assignment-expr,
+    possibly-used-before-assignment,
 
 
 [REPORTS]

tests/functional/r/redefined/redefined_except_handler.txt

@@ -1,4 +1,4 @@
 redefined-outer-name:11:4:12:12::Redefining name 'err' from outer scope (line 8):UNDEFINED
 redefined-outer-name:57:8:58:16::Redefining name 'err' from outer scope (line 51):UNDEFINED
-used-before-assignment:69:14:69:29:func:Using variable 'CustomException' before assignment:CONTROL_FLOW
+used-before-assignment:69:14:69:29:func:Using variable 'CustomException' before assignment:HIGH
 redefined-outer-name:71:4:72:12:func:Redefining name 'CustomException' from outer scope (line 62):UNDEFINED

tests/functional/u/undefined/undefined_variable.txt

@@ -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

tests/functional/u/undefined/undefined_variable_py38.txt

@@ -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

tests/functional/u/used/used_before_assignment.py

@@ -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:
@@ -70,7 +70,7 @@ def redefine_time_import_with_global():
         VAR5 = True
     else:
         VAR5 = True
-if VAR5:
+if VAR5:  # [possibly-used-before-assignment]
     pass
 
 if FALSE:
@@ -116,7 +116,7 @@ def redefine_time_import_with_global():
     VAR11 = num
     if VAR11:
         VAR12 = False
-print(VAR12)
+print(VAR12)  # [possibly-used-before-assignment]
 
 def turn_on2(**kwargs):
     """https://github.com/pylint-dev/pylint/issues/7873"""
@@ -180,3 +180,21 @@ def give_me_none():
 class T:  # pylint: disable=invalid-name, too-few-public-methods, undefined-variable
     '''Issue #8754, no crash from unexpected assignment between attribute and variable'''
     T.attr = attr
+
+
+if outer():
+    NOT_ALWAYS_DEFINED = True
+print(NOT_ALWAYS_DEFINED)  # [used-before-assignment]
+
+
+def inner_if_continues_outer_if_has_no_other_statements():
+    for i in range(5):
+        if isinstance(i, int):
+            # Testing no assignment here, before the inner if
+            if i % 2 == 0:
+                order = None
+            else:
+                continue
+        else:
+            order = None
+        print(order)

tests/functional/u/used/used_before_assignment.txt

@@ -4,9 +4,12 @@ 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
+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
+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

tests/functional/u/used/used_before_assignment_except_handler_for_try_with_return.txt

@@ -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

tests/functional/u/used/used_before_assignment_issue1081.py

@@ -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]
 

tests/functional/u/used/used_before_assignment_issue1081.txt

@@ -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
+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

tests/functional/u/used/used_before_assignment_issue2615.txt

@@ -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

tests/functional/u/used/used_before_assignment_issue626.txt

@@ -1,5 +1,5 @@
 unused-variable:5:4:6:12:main1:Unused variable 'e':UNDEFINED
-used-before-assignment:8:10:8:11:main1:Using variable 'e' before assignment:CONTROL_FLOW
+used-before-assignment:8:10:8:11:main1:Using variable 'e' before assignment:HIGH
 unused-variable:21:4:22:12:main3:Unused variable 'e':UNDEFINED
 unused-variable:31:4:32:12:main4:Unused variable 'e':UNDEFINED
-used-before-assignment:44:10:44:11:main4:Using variable 'e' before assignment:CONTROL_FLOW
+used-before-assignment:44:10:44:11:main4:Using variable 'e' before assignment:HIGH

tests/functional/u/used/used_before_assignment_postponed_evaluation.txt

@@ -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

tests/functional/u/used/used_before_assignment_scoping.txt

@@ -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