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

member lookup with Generic base class in 2.5.3 #942

Closed
farmio opened this issue Apr 11, 2021 · 6 comments · Fixed by pylint-dev/pylint#4471 or #992
Closed

member lookup with Generic base class in 2.5.3 #942

farmio opened this issue Apr 11, 2021 · 6 comments · Fixed by pylint-dev/pylint#4471 or #992
Labels
Milestone

Comments

@farmio
Copy link

farmio commented Apr 11, 2021

Steps to reproduce

from abc import ABC
from typing import Generic, TypeVar

Anything = TypeVar("Anything")
MoreSpecific = TypeVar("MoreSpecific", str, int)


class A(ABC, Generic[Anything]):
    def a_method(self) -> None:
        print("hello")


class B(A[MoreSpecific]):
    pass


class C(B[str]):
    pass


c = C()
c.a_method()

Current behavior

pylint fails with 22:0: E1101: Instance of 'C' has no 'a_method' member (no-member) when using astroid 2.5.3 but doesn't when using astroid 2.5.2

Expected behavior

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

When failing:

> pylint --version
pylint 2.7.4
astroid 2.5.3
Python 3.9.1 (default, Feb 15 2021, 10:36:56)
[Clang 10.0.0 (clang-1000.10.44.4)]

Without failing:

> pylint --version
pylint 2.7.4
astroid 2.5.2
Python 3.9.1 (default, Feb 15 2021, 10:36:56)
[Clang 10.0.0 (clang-1000.10.44.4)]
@farmio farmio changed the title member lookup in 2.5.3 member lookup with Generic base class in 2.5.3 Apr 11, 2021
@sodul
Copy link

sodul commented Apr 13, 2021

We are experiencing the same issue with Astroid 2.5.3, rolling back to 2.5.2 addresses it.

We suspect it was introduced by: cd0f896

@cdce8p
Copy link
Member

cdce8p commented Apr 18, 2021

I can confirm the issue. #931 might indeed be related but I don't believe it's the sole reason.

After some investigation, I found that the ancestors of C are calculated incorrectly. Here is a test case for astroid that illustrates the problem:

from astroid import builder

def test_ancestors_with_generics():
    tree = builder.parse(
        """
    from typing import TypeVar, Generic
    T = TypeVar("T")
    class A(Generic[T]):
        def a_method(self):
            print("hello")
    class B(A[T]): pass
    class C(B[str]): pass
    """
    )
    node_b = tree["B"]
    inferred_b = next(node_b.infer())
    ancestors_b = list(inferred_b.ancestors())
    assert [cdef.name for cdef in ancestors_b] == ["A", "Generic", "object"]

    node_c = tree["C"]
    inferred_c = next(node_c.infer())
    ancestors_c = list(inferred_c.ancestors())
    assert [cdef.name for cdef in ancestors_c] == ["B", "A", "Generic", "object"]  # fails!

test_ancestors_with_generics()

At the moment, only B is inferred as ancestors which then results in the no-member error.

@hippo91 Do you have an idea how to fix that? It's probably related to the changes in ClassDef.getitem we did to support __class_getitem__.

@hippo91
Copy link
Contributor

hippo91 commented Apr 18, 2021

@cdce8p i just had a look to the pb. In fact the ancestors method failed at line

for grandpa in baseobj.ancestors(recurs=True, context=context):

in fact this is the context̀ obj which is the origin of the pb.
An interesting fact is that #927 seems to solve the pb.
However using pylint and astroid with #927 leads to another false positive:

E1136: Value 'A' is unsubscriptable (unsubscriptable-object)

I will try to investigate at bit more ASAP, but a sure thing is that, IMHO, we definitely need #927

@nelfin
Copy link
Contributor

nelfin commented Apr 21, 2021

@hippo91 and @cdce8p: both your test and the E1136 pass after merging #927 and #946 (I had a suspicion)

(venv39) (venv39) ~/p/o/a/fixtures (master) $ cat issue942.py
# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use
from abc import ABC
from typing import Generic, TypeVar

Anything = TypeVar("Anything")
MoreSpecific = TypeVar("MoreSpecific", str, int)


class A(ABC, Generic[Anything]):
    def a_method(self) -> None:
        print("hello")


class B(A[MoreSpecific]):
    pass


class C(B[str]):
    pass


c = C()
c.a_method()
(venv39) (venv39) ~/p/o/a/fixtures (master) $ python -m pylint -- issue942.py 
************* Module issue942
issue942.py:23:0: E1101: Instance of 'C' has no 'a_method' member (no-member)

------------------------------------------------------------------
Your code has been rated at 6.15/10 (previous run: 6.15/10, +0.00)

(venv39) (venv39) ~/p/o/a/fixtures (master) $ git co fix/926-strong-mutable-ref-in-context-clone 
Switched to branch 'fix/926-strong-mutable-ref-in-context-clone'
(venv39) (venv39) ~/p/o/a/fixtures (fix/926-strong-mutable-ref-in-context-clone) $ 
python -m pylint -- issue942.py 
************* Module issue942
issue942.py:14:8: E1136: Value 'A' is unsubscriptable (unsubscriptable-object)

------------------------------------------------------------------
Your code has been rated at 6.15/10 (previous run: 6.15/10, +0.00)

(venv39) (venv39) ~/p/o/a/fixtures (fix/926-strong-mutable-ref-in-context-clone) $ 
git co fix/942-merge 
Switched to branch 'fix/942-merge'
(venv39) (venv39) ~/p/o/a/fixtures (fix/942-merge) $ python -m pylint -- issue942.py   # <-- here they pass

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 6.15/10, +3.85)

I've pushed my this to https://github.com/nelfin/astroid/tree/fix/942-merge on my fork if you'd like to try something else.

@hippo91
Copy link
Contributor

hippo91 commented Apr 21, 2021

@nelfin nice catch!
Thanks. I hope both PR will be merged soon.

@cdce8p
Copy link
Member

cdce8p commented May 12, 2021

This issue is fixed in the current astroid master branch and will be included in the next pylint release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants