Apply generic class fix also to non-callable types

[ilevkivskyi]

This is a follow up for #8021, that applies the fix also to non-callable types. Currently non-callable types are still wrong:

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).

[JukkaL]

Thanks for fixing this special case! Some additional comments would be helpful here, since it wasn't obvious to me how the fix works exactly.

Maybe also update the commit message with a code example that this fixes, since it's not entirely obvious from the explanation (or create an issue and refer to it in the commit message).

[JukkaL]

What about testing if there are two levels of generic subclasses, such as class F(G[Tuple[int]]): ...?

What if the derived class adds a type variable? There may be two interesting cases -- 1) derived class adds a type variable that becomes the first type variable 2) derived class adds a type variable the becomes the second type variable.

[ilevkivskyi]

I can add few more tests, but essentially this will be testing map_instance_to_supertype() which is already quite well tested.

[JukkaL]

Yes, the extra tests don't seem useful.

[JukkaL]

It would be nice to have a comment here, or maybe add a case to the above comment where the type is not callable.

Also, I have trouble convincing myself how we can be sure that the type arguments from isuper can always be applied to t. Maybe explain this is briefly (in the above comment perhaps, since this doesn't seem specific to this case).

[ilevkivskyi]

Also, I have trouble convincing myself how we can be sure that the type arguments from isuper can always be applied to t.

I actually have trouble understanding how this can not work :-) My guess is confusion comes from missing docstring entry for isuper, which is current instance mapped to the method definition superclass. I was also thinking about an assert, but this would be probably too much.

[ilevkivskyi]

Btw, while adding docstring items I noticed there are two unused arguments in this function.

[JukkaL]

Yeah, I was confused about isuper. Now it all makes sense.

[JukkaL]

Looks good now after the docstring update!

[JukkaL]

Style nit: We don't use ; after each description of an argument in docstrings.

[JukkaL]

Yeah, I was confused about isuper. Now it all makes sense.

[JukkaL]

Yes, the extra tests don't seem useful.