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

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

Open
artsiomkaltovich opened this issue Jun 12, 2024 · 2 comments
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs astroid constraint Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@artsiomkaltovich
Copy link

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 artsiomkaltovich added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 12, 2024
@jacobtylerwalls jacobtylerwalls added Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 14, 2024
@jacobtylerwalls
Copy link
Member

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 jacobtylerwalls changed the title False Positive E1133 on field with default None [not-an-iterable] FP for attribute used in comprehension but guarded in if test Jun 14, 2024
@jacobtylerwalls jacobtylerwalls added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jun 14, 2024
@jacobtylerwalls
Copy link
Member

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

@jacobtylerwalls jacobtylerwalls added the Help wanted 🙏 Outside help would be appreciated, good for new contributors label Jul 14, 2024
@jacobtylerwalls jacobtylerwalls added Needs astroid constraint Astroid Related to astroid and removed Astroid Related to astroid labels Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs astroid constraint Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants