-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Refactor all member access to go through checkmember.py (meta issue) #7724
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
Comments
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).
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`).
Plugin management is also really important here, because right now protocols do not support objects with custom # 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. 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
|
At the moment I am trying to refactor
All things in But I have several questions:
|
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.
Another problem that came to my mind while thinking about this is how to handle deferrals? If e.g. |
Fixes #5803 Fixes #18695 Fixes #17513 Fixes #13194 Fixes #12126 This is the first PR towards #7724, some notes: * I add a new generic `suppress_errors` flag to `MemberContext` mostly as a performance optimization, but it should also be handy in the following PRs * I noticed some inconsistencies with how we handle variable inference (e.g. we don't infer type at all if rvalue type is not compatible with superclass type). After all I decided to remove some of them, as it makes implementation of this PR simpler. * I added a bunch of TODOs, most of those will be addressed in following PRs. * A while ago we agreed that an explicit `Callable[...]` annotation in class body means how the type looks on an _instance_, but the override check used to handle this inconsistently (I add few `reveal_type()`s to tests to illustrate this).
This is a second "large" PR towards #7724. Here I actually expect a smaller fallout than for variables, since methods are usually less tricky, but let's see.
This is the third "major" PR towards #7724 This one is mostly straightforward. I tried to preserve the existing logic about mutable overrides (to minimize fallout), as currently we e.g. don't use the covariant mutable override error code here. In future we can separately "synchronize" mutable override logic across variable override, method override, and multiple inheritance code paths (as currently all three are subtly different).
Fixes #18024 Fixes #18706 Fixes #17734 Fixes #15097 Fixes #14814 Fixes #14806 Fixes #14259 Fixes #13041 Fixes #11993 Fixes #9585 Fixes #9266 Fixes #9202 Fixes #5481 This is a fourth "major" PR toward #7724. This is one is watershed/crux of the whole series (but to set correct expectations, there are almost a dozen smaller follow-up/clean-up PRs in the pipeline). The core of the idea is to set current type-checker as part of the global state. There are however some details: * There are cases where we call `is_subtype()` before type-checking. For now, I fall back to old logic in this cases. In follow up PRs we may switch to using type-checker instances before type checking phase (this requires some care). * This increases typeops import cycle by a few modules, but unfortunately this is inevitable. * This PR increases potential for infinite recursion in protocols. To mitigate I add: one legitimate fix for `__call__`, and one temporary hack for `freshen_all_functions_type_vars` (to reduce performance impact). * Finally I change semantics for method access on class objects to match the one in old `find_member()`. Now we will expand type by instance, so we have something like this: ```python class B(Generic[T]): def foo(self, x: T) -> T: ... class C(B[str]): ... reveal_type(C.foo) # def (self: B[str], x: str) -> str ``` FWIW, I am not even 100% sure this is correct, it seems to me we _may_ keep the method generic. But in any case what we do currently is definitely wrong (we infer a _non-generic_ `def (x: T) -> T`). --------- Co-authored-by: hauntsaninja <hauntsaninja@gmail.com> Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Implements a check to disallow assignments to self.__class__, as these can cause unsafe runtime behavior. Includes tests for __class__ and related dunder attributes. Fixes python#7724
Fixes python#18024 Fixes python#18706 Fixes python#17734 Fixes python#15097 Fixes python#14814 Fixes python#14806 Fixes python#14259 Fixes python#13041 Fixes python#11993 Fixes python#9585 Fixes python#9266 Fixes python#9202 Fixes python#5481 This is a fourth "major" PR toward python#7724. This is one is watershed/crux of the whole series (but to set correct expectations, there are almost a dozen smaller follow-up/clean-up PRs in the pipeline). The core of the idea is to set current type-checker as part of the global state. There are however some details: * There are cases where we call `is_subtype()` before type-checking. For now, I fall back to old logic in this cases. In follow up PRs we may switch to using type-checker instances before type checking phase (this requires some care). * This increases typeops import cycle by a few modules, but unfortunately this is inevitable. * This PR increases potential for infinite recursion in protocols. To mitigate I add: one legitimate fix for `__call__`, and one temporary hack for `freshen_all_functions_type_vars` (to reduce performance impact). * Finally I change semantics for method access on class objects to match the one in old `find_member()`. Now we will expand type by instance, so we have something like this: ```python class B(Generic[T]): def foo(self, x: T) -> T: ... class C(B[str]): ... reveal_type(C.foo) # def (self: B[str], x: str) -> str ``` FWIW, I am not even 100% sure this is correct, it seems to me we _may_ keep the method generic. But in any case what we do currently is definitely wrong (we infer a _non-generic_ `def (x: T) -> T`). --------- Co-authored-by: hauntsaninja <hauntsaninja@gmail.com> Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
…Visitor (Fixes python#7724) Replaces previous incorrect call to self.chk.fail() with self.errors.report(), resolving AttributeError. Ensures assignment to __class__ is properly reported as an error during instance attribute assignments.
Added a check inside TypeChecker.visit_assignment_stmt to disallow assignments to '__class__', as such assignments are unsafe and may lead to unpredictable behavior. The new check raises an error using self.fail when '__class__' is assigned, matching the style used in other TypeChecker checks. This change addresses issue python#7724.
Implemented the check inside TypeChecker.visit_assignment_stmt to disallow assignments to '__class__', as such assignments are unsafe. The check correctly uses self.fail() following mypy's error reporting conventions, and is located after initial assignment processing to align with the design of TypeChecker. Previous redundant logic inside VarAssignVisitor.visit_assignment_stmt has been removed. This change addresses issue python#7724.
Fixes #5136 Fixes #5491 This is a fifth "major" PR toward #7724. Although it would be impractical to move all the operator special-casing to `checkmember.py`, this does two things: * Removes known inconsistencies in operator handling * Adds a much more complete `has_operator()` helper that can be a starting point for future performance optimizations
Fixes #3832 Fixes #5723 Fixes #17174 Improves #7217 This is a sixth "major" PR toward #7724. Previously access to "static" attributes (like type aliases, class objects) was duplicated in four places: * In `analyze_ref_expr()` * In `determine_type_of_member()` (for modules as subtypes of protocols) * In instance attribute access logic * In class attribute logic Most of these were somewhat incomplete and/or inconsistent, this PR unifies all four (there is still tiny duplication because I decided to limit the number of deferrals, i.e. preserve the existing logic in this respect). Some notable things that are not pure refactoring: * Previously we disabled access to type variables as class attributes. This was inconsistent with plain references and instance attributes that just return `Instance("typing.TypeVar")`. * Instance access plugins were only applied on `TypeInfo`s and `TypeAlias`es, now they are applied always. * Previously arguments kinds were sometimes not correct for TypedDict class objects with non-required keys. * I tweaked `TypeOfAny` in couple places to be more logical.
Uh oh!
There was an error while loading. Please reload this page.
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):checkmember.py
itselfcheck_op()
andhas_member()
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).
The text was updated successfully, but these errors were encountered: