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

False negative for used-before-assignment for variables only defined beneath always false conditions #4913

Closed
bugale opened this issue Aug 25, 2021 · 3 comments · Fixed by #6677
Assignees
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check Control flow Requires control flow understanding Enhancement ✨ Improvement to a component False Negative 🦋 No message is emitted but something is wrong with the code
Milestone

Comments

@bugale
Copy link

bugale commented Aug 25, 2021

Bug description

The following code produces no warning:

""" bla """

VAR1 = False
if VAR1:
    VAR2 = True
if VAR2:
    pass

even though VAR2 is clearly uninitialized.

This is similar to the issue reported in #4045 , thouhg in a much simpler scenario, without the need for exceptions to be handled correctly.

Command used

pylint test.py

Pylint output

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Expected behavior

Warning about the possibly uninitialized variable

Pylint version

Reproduces on 2.5.0, 2.7.0, 2.8.0, 2.9.0, 3.0.0

OS / Environment

Windows

@bugale bugale added Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 25, 2021
@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 25, 2021
@DanielNoord DanielNoord added C: undefined-variable Issues related to 'undefined-variable' check Control flow Requires control flow understanding labels Dec 21, 2021
@jacobtylerwalls jacobtylerwalls self-assigned this Apr 3, 2022
@jacobtylerwalls
Copy link
Member

I have a draft patch for this, but it's not mergeable without revisiting pylint-dev/astroid#853. (See below)

diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index e1c5e777..5bbf1bbb 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -633,6 +633,13 @@ scope_type : {self._atomic.scope_type}
         if VariablesChecker._comprehension_between_frame_and_node(node):
             return found_nodes
 
+        # Filter out assignments guarded by if False: (or equivalent falsy constants)
+        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 = [
@@ -678,6 +685,22 @@ scope_type : {self._atomic.scope_type}
 
         return found_nodes
 
+    @staticmethod
+    def _uncertain_nodes_in_false_tests(
+        found_nodes: Iterable[nodes.NodeNG], node: nodes.NodeNG
+    ) -> None:
+        uncertain_nodes = []
+        for other_node in found_nodes:
+            closest_if = utils.get_node_first_ancestor_of_type(other_node, nodes.If)
+            if closest_if is not None and node.frame() is closest_if.frame():
+                if isinstance(closest_if.test, nodes.Name):
+                    if all(
+                        isinstance(inferred, nodes.Const) and not inferred.value
+                        for inferred in utils.infer_all(closest_if.test)
+                    ):
+                        uncertain_nodes.append(other_node)
+        return uncertain_nodes
+
     @staticmethod
     def _uncertain_nodes_in_except_blocks(
         found_nodes: List[nodes.NodeNG],
diff --git a/tests/functional/u/used/used_before_assignment.py b/tests/functional/u/used/used_before_assignment.py
index 5b469041..5b14488a 100644
--- a/tests/functional/u/used/used_before_assignment.py
+++ b/tests/functional/u/used/used_before_assignment.py
@@ -6,3 +6,9 @@ __revision__ = None
 MSG = "hello %s" % MSG  # [used-before-assignment]
 
 MSG2 = "hello %s" % MSG2  # [used-before-assignment]
+
+VAR1 = False
+if VAR1:
+    VAR2 = True
+if VAR2:  # [used-before-assignment]
+    pass

Result of linting pylint itself:

************* Module pylint.checkers.exceptions
pylint/checkers/exceptions.py:429:23: E0601: Using variable 'excs_in_current_handler' before assignment (used-before-assignment)
pylint/checkers/exceptions.py:435:56: E0601: Using variable 'exc_in_current_handler' before assignment (used-before-assignment)

Which ultimately has to do with a pattern like this:

x = False
for i in range(2):
  if x:
    blah = None
    print(blah)
  if i == 0:
    x = True

How will we know that blah is not actually under a constant false test? astroid only tells us that x can be False without pylint-dev/astroid#853.

@jacobtylerwalls jacobtylerwalls removed their assignment Apr 4, 2022
@jacobtylerwalls jacobtylerwalls added Needs astroid update Needs an astroid update (probably a release too) before being mergable Astroid Related to astroid C: used-before-assignment Issues related to 'used-before-assignment' check and removed Needs astroid update Needs an astroid update (probably a release too) before being mergable C: undefined-variable Issues related to 'undefined-variable' check labels Apr 4, 2022
@jacobtylerwalls jacobtylerwalls changed the title Undetected possibly uninitialized variables False negative for used-before-assignment for variables only defined beneath always false conditions Apr 4, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Duplicate 🐫 Duplicate of an already existing issue label May 18, 2022
@Pierre-Sassoulas
Copy link
Member

Closing as duplicate of #1727

@jacobtylerwalls
Copy link
Member

Actually, I don't think this is a duplicate of #1727, which is about control flow ambiguity in general. This one is more specific -- just to issue warnings when we can detect "always false" conditions, which is simpler to solve. But let me know if you think I'm missing something, of course!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check Control flow Requires control flow understanding Enhancement ✨ Improvement to a component False Negative 🦋 No message is emitted but something is wrong with the code
Projects
None yet
4 participants