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

Reproducible but incorrect Pylint: E1133:not-an-iterable in library #9253

Closed
davetapley opened this issue Nov 21, 2023 · 3 comments
Closed
Assignees
Labels
Astroid Related to astroid Duplicate 🐫 Duplicate of an already existing issue

Comments

@davetapley
Copy link

Bug description

# iter_bug.py

from textual.app import App
from textual.widgets import Input


class MyApp(App[None]):
    def whatever(self):
        print([i.id for i in self.query(Input)])

Configuration

No response

Command used

pylint iter_bug.py

Pylint output

************* Module iter_bug
iter_bug.py:7:29: E1133: Non-iterable value self.query(Input) is used in an iterating context (not-an-iterable)

Expected behavior

No errors.

Pylint version

pylint 3.0.2
astroid 3.0.1
Python 3.11.6 (main, Nov  2 2023, 02:14:33) [GCC 9.4.0]

OS / Environment

Ubuntu 20.04.6 LTS

Additional dependencies

textual == 0.40.0
@davetapley davetapley added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 21, 2023
@davetapley
Copy link
Author

davetapley commented Nov 21, 2023

Creating as requested here. Initially reported here:


Some notes:

All overloads of a method query return an instance of DOMQuery:
https://github.com/Textualize/textual/blob/81531e87647eae26b83ad31f1941016b4f2bab41/src/textual/dom.py#L1073-L1083

and DOMQuery implements __iter__:

https://github.com/Textualize/textual/blob/81531e87647eae26b83ad31f1941016b4f2bab41/src/textual/css/query.py#L141-L142

By my reckoning that should be enough? 🤔


texual maintainer suggests workaround of using a results method they added, which works:
https://github.com/Textualize/textual/blob/896aa9f9244ce852659946bb3abc8551b38bbb03/src/textual/css/query.py#L309-L310


I did some 👀 myself:

I use VSCode debug with launch.json:

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "pylint",
            "type": "python",
            "request": "launch",
            "module": "pylint",
            "justMyCode": false,
            "args": ["iter_bug.py"]
        },

Then break point here:

@only_required_for_messages("not-an-iterable")
def visit_listcomp(self, node: nodes.ListComp) -> None:
for gen in node.generators:
self._check_iterable(gen.iter, check_async=gen.is_async)

Add disabled break point here:
https://github.com/pylint-dev/astroid/blob/2d309cbb823e27d73bf695447a50f7875a852612/astroid/decorators.py#L94

Hit first break, then enable second break and continue.

Will hit raise InferenceError(**error.args[0]) with something like:

error.args[0]
{'node': <Comprehension l.73 ...7bc9233d0>, 'unknown': <AssignName.i l.73 a...7bc923450>, 'assign_path': None, 'context': <astroid.context.Inf...7ade519c0>}

which causes yield Uninferable, then inferred = Const.NoneType,
and here gives not is_iterable(inferred,.. 😞

But that's so deep in to astroid internals I have no idea why InferenceError is raised 🤔

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 22, 2023
@jacobtylerwalls
Copy link
Member

Thanks for the investigation. From the source for query, which you linked, I noticed the use of @overload. We lack support for that (see pylint-dev/astroid#1015), so that's my best guess for why Uninferable turns up.

More specifically, though, the not-an-iterable check should quiet itself when Uninferable turns up. That's a trivial patch. Coming right up!

@jacobtylerwalls jacobtylerwalls added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Nov 25, 2023
@jacobtylerwalls jacobtylerwalls self-assigned this Nov 25, 2023
@jacobtylerwalls
Copy link
Member

More specifically, though, the not-an-iterable check should quiet itself when Uninferable turns up. That's a trivial patch. Coming right up!

Ah, it does that already. I didn't notice this part of your report:

then inferred = Const.NoneType,

So really this is just a duplicate of pylint-dev/astroid#1015, unfortunately. Sorry there is no workaround at the moment!

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Nov 25, 2023
@jacobtylerwalls jacobtylerwalls added Astroid Related to astroid Duplicate 🐫 Duplicate of an already existing issue and removed False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid Duplicate 🐫 Duplicate of an already existing issue
Projects
None yet
Development

No branches or pull requests

3 participants