pylint icon indicating copy to clipboard operation
pylint copied to clipboard

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

Open artsiomkaltovich opened this issue 1 year ago • 2 comments

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

artsiomkaltovich avatar Jun 12 '24 14:06 artsiomkaltovich

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 avatar Jun 14 '24 12:06 jacobtylerwalls

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

jacobtylerwalls avatar Jun 14 '24 14:06 jacobtylerwalls