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

Refactor all member access to go through checkmember.py (meta issue) #7724

Open
ilevkivskyi opened this issue Oct 16, 2019 · 3 comments
Open
Labels
meta Issues tracking a broad area of work needs discussion priority-0-high refactoring Changing mypy's internals

Comments

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Oct 16, 2019

There are bunch of places where we duplicate some parts of code for special logic in member access (such as properties, class methods, descriptors, __getattr__(), etc):

  • The checkmember.py itself
  • Override checks for variables
  • Override checks for methods
  • Multiple inheritance checks
  • Checks for special methods via check_op() and has_member()
  • Protocol implementation check

Here are some issues that will be solved (or significantly simplified) by refactoring this to go through the same place (probably checkmember.py):

Also I think having member access cleaned-up, centralized, and documented will open the way to finally solving #708 (currently the oldest high priority issue), and to having general support for descriptor protocol (for historical reasons, several things like properties are supported via some special-casing).

ilevkivskyi added a commit that referenced this issue Nov 27, 2019
Fixes #8020

There is a bunch of code/logic duplication around `bind_self()`, mostly because of #7724. This PR makes all three main code paths consistently follow the same structure:
1. `freshen_function_type_vars()`
2. `bind_self()`
3. `expand_type_by_instance(..., map_instance_to_supertype())` (a.k.a `map_type_from_supertype()`)

I briefly scrolled through other code paths, and it looks like this was last major/obvious inconsistency (although code around `__getattr__`/`__setattr__`/`__get__`/`__set__` looks a bit suspicious).
ilevkivskyi added a commit that referenced this issue Nov 29, 2019
This is a follow up for #8021, that applies the fix also to non-callable types. Currently non-callable types are still wrong:
```python
R = TypeVar('R')
T = TypeVar('T')
# Can be any decorator that makes type non-callable.
def classproperty(f: Callable[..., R]) -> R: ...

class C(Generic[T]):
    @classproperty
    def test(self) -> T: ...

x: C[int]
y: Type[C[int]]
reveal_type(x.test)  # Revealed type is 'int', OK
reveal_type(y.test)  # Revealed type is 'T' ???
```

So, #7724 strikes again. It turns out there is not only duplicated logic for attribute kinds (decorators vs normal methods), but also for callable vs non-callable types. In latter case we still need to expand the type (like in other places, e.g., `analyze_var`).
@sobolevn
Copy link
Member

sobolevn commented Jun 23, 2021

Plugin management is also really important here, because right now protocols do not support objects with custom get_attribute_hook / get_method_sig_hook items. Quick example:

# ex.py
class A(object):
    attr: str  # is set via plugin

    def method(self) -> str:  # is set via plugin
        ...

a = A()
reveal_type(a.attr)
# Revealed type is "builtins.float"
reveal_type(a.method())
# Revealed type is "builtins.int"

Plugin:

from mypy.plugin import Plugin

class MyPlugin(Plugin):
    def get_attribute_hook(self, fullname):
        if fullname == 'ex.A.attr':
            def test(ctx):
                return ctx.api.named_type('builtins.float')
            return test

    def get_method_signature_hook(self, fullname: str):
        if fullname == 'ex.A.method':
            def test(ctx):
                return ctx.default_signature.copy_modified(
                    ret_type=ctx.api.named_type('builtins.int'),
                )
            return test

def plugin(version):
    return MyPlugin

At the moment, everything works as expected.
Now, let's define a protocol and check if it works.

Protocol (problematic part):

from typing_extensions import Protocol

class Some(Protocol):
    attr: float

    def method(self) -> int:
        ...

def accepts_protocol(some: Some) -> int:
    return some.method()

accepts_protocol(a)
# ex.py:26: error: Argument 1 to "accepts_protocol" has incompatible type "A"; expected "Some"
# ex.py:26: note: Following member(s) of "A" have conflicts:
# ex.py:26: note:     attr: expected "float", got "str"
# ex.py:26: note:     Expected:
# ex.py:26: note:         def method(self) -> int
# ex.py:26: note:     Got:
# ex.py:26: note:         def method(self) -> str

find_member does not call get_attribute_hook because it does not even have chk instance.

@sobolevn
Copy link
Member

At the moment I am trying to refactor find_member to be almost an alias to analyze_member_access. It is not so-commonly used. grep:

» ag find_member
mypy/checkmember.py
139:    # TODO: This and following functions share some logic with subtypes.find_member;

mypy/join.py
15:    is_protocol_implementation, find_member
592:        return find_member('__call__', t, t, is_operator=True)

mypy/messages.py
38:    is_subtype, find_member, get_member_flags,
599:            call = find_member('__call__', original_caller_type, original_caller_type,
1922:        if not find_member(member, left, left):
1936:        supertype = find_member(member, right, left)
1938:        subtype = find_member(member, left, left)
1957:        if find_member(member, left, left):

mypy/constraints.py
324:                    call = mypy.subtypes.find_member('__call__', template, actual,
435:            inst = mypy.subtypes.find_member(member, instance, subtype)
436:            temp = mypy.subtypes.find_member(member, template, subtype)
481:            call = mypy.subtypes.find_member('__call__', self.actual, self.actual,

mypy/checker.py
61:    unify_generic_callable, find_member
4678:            call = find_member('__call__', subtype, subtype, is_operator=True)
4683:                call = find_member('__call__', supertype, subtype, is_operator=True)
4963:        iter_type = get_proper_type(find_member('__iter__', instance, instance, is_operator=True))

mypy/subtypes.py
292:            call = find_member('__call__', left, left, is_operator=True)
321:                call = find_member('__call__', right, left, is_operator=True)
408:                call = find_member('__call__', right, left, is_operator=True)
560:            supertype = get_proper_type(find_member(member, right, left))
562:            subtype = get_proper_type(find_member(member, left, left))
608:def find_member(name: str,
736:        typ = get_proper_type(find_member(member, instance, instance))
1299:            call = find_member('__call__', left, left, is_operator=True)

All things in checker.py are easy, I will just change find_member to analyze_member_access.

But I have several questions:

  1. What to do with functions that use find_member inside? Like ones in subtypes.py and constrains.py? They will require a lot of context to work properly: chk is the most complex one to pass through
  2. How will it affect performance if is_subtype (which is widely used) will call get_attribute_hook?
  3. Should find_memebr and analyze_member_access be different? Are there any details to keep in mind?

@97littleleaf11 97littleleaf11 added the meta Issues tracking a broad area of work label Dec 1, 2021
ilevkivskyi added a commit that referenced this issue Nov 15, 2022
Ref #12840 
Fixes #11871
Fixes #14089

This is an alternative implementation to two existing PRs:
#11666,
#13133. This PR treats `typing.Self`
as pure syntactic sugar, and transforms it into a type variable early
during semantic analyzis.

This way we can re-use all the existing machinery and handled edge cases
for self-types. The only new thing is self-type for _attributes_ (as
proposed in the PEP). This required handling in several places, since
attribute access is duplicated in several places (see #7724), plus
special forms (like NamedTuples and TypedDicts) and dataclasses plugin
require additional care, since they use attribute annotations in special
ways.

I don't copy all the existing tests for "old style" self-types, but only
some common use cases, possible error conditions, and relevant new edge
cases, such as e.g. special forms mentioned above, and implicit type
variable binding for callable types.
@ilevkivskyi
Copy link
Member Author

Another problem that came to my mind while thinking about this is how to handle deferrals? If e.g. is_subtype() ultimately ends up in protocol check w.r.t. to a class with not ready attribute (e.g. a tricky decorator) then we can't really return neither False nor True since both can cause spurious errors. The only way I see is to raise an error, but then each call to is_subtype() should expect this, making everything insanely complex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues tracking a broad area of work needs discussion priority-0-high refactoring Changing mypy's internals
Projects
None yet
Development

No branches or pull requests

3 participants