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 not-an-iterable for class method with @overload (2.5.7 regression) #1015

Closed
belm0 opened this issue Jun 8, 2021 · 8 comments · Fixed by #1173 or #2426
Closed

false not-an-iterable for class method with @overload (2.5.7 regression) #1015

belm0 opened this issue Jun 8, 2021 · 8 comments · Fixed by #1173 or #2426
Assignees
Labels
Bug 🪳 High priority Issue with more than 10 reactions Regression

Comments

@belm0
Copy link
Contributor

belm0 commented Jun 8, 2021

Steps to reproduce

Starting from astroid 2.5.7, I'm seeing false not-an-iterable when generator class methods are annotated with @overload.

from typing import overload, Iterator

class MyClass:
    @overload
    def transitions(self, foo: int, bar: int) -> Iterator[int]: ...
    @overload
    def transitions(self, baz: str) -> Iterator[str]: ...
    def transitions(self, foo_or_baz, bar=None):
        yield

for _ in MyClass().transitions('hello'):
    pass

If @overload is removed, or the function is moved to the module level, or I switch to astroid 2.5.6, the problem goes away.

It happens with pylint-2.8.3 or pylint-3.0.0a3.

Current behavior

E1133: Non-iterable value MyClass().transitions('hello') is used in an iterating context (not-an-iterable)

Expected behavior

no error

@belm0
Copy link
Contributor Author

belm0 commented Jun 8, 2021

I can work around it by replacing ... with yield, but it doesn't seem that this should be necessary (and it wasn't in prior versions of Astroid). And it is hard to satisfy mypy with this workaround, because it expects yield [value] etc.

@hippo91
Copy link
Contributor

hippo91 commented Jun 14, 2021

@belm0 thanks for the report.

@cdce8p
Copy link
Member

cdce8p commented Sep 12, 2021

The issue seems to have started with #934, the previous commit is fine: 2e8417f. (Tested with pylint 2.8.3.)
@nelfin Would you like to take a look at this?

@nelfin
Copy link
Contributor

nelfin commented Sep 12, 2021

Sure, happy to take this one

@nelfin
Copy link
Contributor

nelfin commented Sep 15, 2021

This doesn't seem to have anything to do with @overload. Here's a minimum reproducing example:

class A:
    def foo(self): ...
    def foo(self):
        yield

for _ in A().foo():
    pass

I think this has to do with the name resolution order, Attribute.foo is inferred as the foo(): ... instead of foo(): yield and the lack of a function body means infer_call_result infers None (arguably correct, just given incorrect inputs). If you switch the order of the functions in the class body then there's no pylint warning (which is a false-negative). I'm chasing down the root cause and a fix.

nelfin added a commit to nelfin/astroid that referenced this issue Sep 15, 2021
Ref pylint-dev#1015. When there are multiple statements defining some attribute
ClassDef.igetattr filters out later definitions if they are not in the
same scope as the first (to support cases like "self.a = 1; self.a = 2"
or "self.items = []; self.items += 1"). However, it checks the scope of
the first attribute against the *parent scope* of the later attributes.
For mundane statements this makes no difference, but for
scope-introducing statements such as FunctionDef these are not the same.
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.8.1 milestone Sep 25, 2021
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.8.1, 2.8.2, 2.8.3 Oct 5, 2021
@Pierre-Sassoulas Pierre-Sassoulas removed this from the 2.8.3 milestone Oct 17, 2021
mscuthbert added a commit to cuthbertLab/music21 that referenced this issue Apr 30, 2022
@Pierre-Sassoulas
Copy link
Member

pylint-dev/pylint#7624 have a use case related to this in pylint

@ringohoffman
Copy link

Getting a similar problem with pylint on torch.nn.Module.to():

from __future__ import annotations

from typing import Any, Union, overload


class device:
    ...


class Tensor:
    ...


class dtype:
    ...


class Module:
    def __call__(self, *args: Any, **kwargs: Any) -> Any:
        ...

    @overload
    def to(self, dtype: Union[dtype, str]) -> Module:
        ...

    @overload
    def to(self, tensor: Tensor) -> Module:
        ...

    def to(self, *args, **kwargs):
        return self


tensor = Tensor()

module = Module()
_ = module(tensor)  # OK

module = module.to("cpu")
_ = module(tensor)  # a.py:46:4: E1102: module is not callable (not-callable)
astroid==2.15.4
pylint==2.17.4

@jacobtylerwalls jacobtylerwalls added the High priority Issue with more than 10 reactions label Jun 23, 2023
jacobtylerwalls pushed a commit to nelfin/astroid that referenced this issue Feb 24, 2024
Ref pylint-dev#1015. When there are multiple statements defining some attribute
ClassDef.igetattr filters out later definitions if they are not in the
same scope as the first (to support cases like "self.a = 1; self.a = 2"
or "self.items = []; self.items += 1"). However, it checks the scope of
the first attribute against the *parent scope* of the later attributes.
For mundane statements this makes no difference, but for
scope-introducing statements such as FunctionDef these are not the same.
jacobtylerwalls added a commit that referenced this issue Mar 8, 2024
…attr()` (#1173)

Ref #1015. When there are multiple statements defining some attribute
ClassDef.igetattr filters out later definitions if they are not in the
same scope as the first (to support cases like "self.a = 1; self.a = 2"
or "self.items = []; self.items += 1"). However, it checks the scope of
the first attribute against the *parent scope* of the later attributes.
For mundane statements this makes no difference, but for
scope-introducing statements such as FunctionDef these are not the same.

Fix this, and then filter to just last declared function (unless a property
is involved).

---------

Co-authored-by: Jacob Walls <[email protected]>
@mscuthbert
Copy link

THANKS @jacobtylerwalls -- this was a major bug for pre-typing code that returned different things depending on settings (would not write new code for that today) and needed @overload to specify the proper return values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment