pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.18k stars 1.1k forks source link

[not-an-iterable] FP for attribute used in comprehension but guarded in if test #9729

Open artsiomkaltovich opened 2 weeks ago

artsiomkaltovich commented 2 weeks ago

Bug description

Steps to reproduce:

class Model:
    field: list[int] | None = None

    def method(self):
        return [f + 1 for f in self.field] if self.field else None

if __name__ == "__main__":
    pass

NB: If replace `field: list[int] | None = None` with `field: list[int] | None` it works as expected

Configuration

No response

Command used

pylint filename.py

Pylint output

E1133: Non-iterable value self.field is used in an iterating context (not-an-iterable)

Expected behavior

No error

Pylint version

3.2.3

OS / Environment

MacOS 14.4.1 Python 3.12.0

Additional dependencies

No response

jacobtylerwalls commented 2 weeks ago

Thanks for the report. This is a good case for extending astroid's constraint framework. I'm showing this is fixed with:

diff --git a/astroid/constraint.py b/astroid/constraint.py
index 08bb80e3..57b41701 100644
--- a/astroid/constraint.py
+++ b/astroid/constraint.py
@@ -63,6 +63,8 @@ class NoneConstraint(Constraint):

         Negate the constraint based on the value of negate.
         """
+        if isinstance(expr, nodes.Attribute):
+            return cls(node=node, negate=negate)
         if isinstance(expr, nodes.Compare) and len(expr.ops) == 1:
             left = expr.left
             op, right = expr.ops[0]
@@ -99,10 +101,10 @@ def get_constraints(
     constraints_mapping: dict[nodes.If, set[Constraint]] = {}
     while current_node is not None and current_node is not frame:
         parent = current_node.parent
-        if isinstance(parent, nodes.If):
+        if isinstance(parent, (nodes.If, nodes.IfExp)):
             branch, _ = parent.locate_child(current_node)
             constraints: set[Constraint] | None = None
-            if branch == "body":
+            if branch in ("body", "test"):
                 constraints = set(_match_constraint(expr, parent.test))
             elif branch == "orelse":
                 constraints = set(_match_constraint(expr, parent.test, invert=True))
jacobtylerwalls commented 2 weeks ago

That patch is probably a little too wide, but I hope it's on the right direction.