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

Function argument rejected as a base class #5865

Closed
JukkaL opened this issue Nov 2, 2018 · 9 comments · Fixed by #14135
Closed

Function argument rejected as a base class #5865

JukkaL opened this issue Nov 2, 2018 · 9 comments · Fixed by #14135
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 2, 2018

Class D in the example below is rejected, even though class E, which looks very similar, is accepted:

from typing import Any

a: Any
class C(a): pass  # Ok

def f(b: Any) -> None:
    class D(b): pass  # Errors: Invalid type "b"; Invalid base class

def g(c: Any) -> None:
    d: Any = c
    class E(d): pass  # Ok

The above example shouldn't generate errors.

(Originally reported by @germaniumhq in #2477 (comment).)

@JukkaL JukkaL added bug mypy got something wrong false-positive mypy gave an error on correct code labels Nov 2, 2018
@Victor-Savu
Copy link

I am having this issue as well. I am not sure why an object of type Any should automatically be accepted as a base class. For example, calling f(3) satisfies f's type definition, but 3 would not make for a valid base class. So, I would say that both f and g should error (maybe not for the reason that f errors today).

In my example, I use Type[T]:

from typing import Type, TypeVar

T = TypeVar("T")

def h(tp: Type[T]) -> Type[T]:
    class Foo(tp):
        pass
    return Foo

I would expect h to be accepted here because tp is declared as a type.

@JelleZijlstra
Copy link
Member

@Victor-Savu Any should be accepted as a base class because that's the point of Any; it's an object on which any operation is valid. It's essentially a way to tell the type checker to trust the programmer. Of course f(3) will fail at runtime and the type checker won't catch it, but that's the cost of using Any.

Your example also seems like a bug to me, but it's not quite the same as this one. Feel free to report a new issue.

@Victor-Savu
Copy link

Victor-Savu commented May 5, 2019

Hi @JelleZijlstra! Thank you for the explanation! I went back to the documentation and I am convinced you and @JukkaL are right that the code in the first comment should produce not errors in mypy. I found this stack overflow response very helpful as well.

I would still like to make the point that not any object that is taken in as an argument should be accepted by mypy:

import pytest

def subobject(an_object: object) -> type:
    class A(an_object): pass  # error: Invalid type "an_object", Invalid base class
    return A

def test_subobject() -> None:
    assert isinstance(Exception, type)
    assert isinstance(subobject(Exception), type)
    assert issubclass(subobject(Exception), Exception)
    
    assert isinstance(1, object)
    with pytest.raises(TypeError):
        subobject(1)

The mypy error in the comment above is useful and should be raised even after this issue is fixed. It should probably get a better message. Something like: error: "an_object" cannot be used as a base class because its type is "object", which is not a subclass of "type"

On the other hand, if we fix the problem, the runtime error will be detected ahead of time by mypy as well.

def subclass(a_type: type) -> type:
    class A(a_type): pass  # error: Invalid type "a_type", Invalid base class
    return A

def test_subclass() -> None:
    assert isinstance(Exception, type)
    assert isinstance(subclass(Exception), type)
    assert issubclass(subclass(Exception), Exception)

    assert isinstance(1, object)
    with pytest.raises(TypeError):
        subclass(1)  # error: Argument 1 to "subclass" has incompatible type "int"; expected "type"

The code above should only have one type error (the last line) as the error on the second line is caused by the current issue.

@frankie567
Copy link

frankie567 commented Oct 30, 2019

I'll add here another case, which I believe is related, even though I'm not sure to understand all the whys and wherefores.

from typing import Type

class A: pass

a = A
a2: Type[A] = A  # Ok

class B(a): pass  # Ok

class B2(a2): pass  # Errors: not valid as a type; Invalid base class

def f(cv: Type[A]):
    class B3(cv): pass  # Errors: not valid as a type; Invalid base class
    return B3

f(a)  # Ok
f(a2)  # Ok

I would expect that if I promise the type checker that my variable is of the class type, it would accept it as base class. Is there some subtleties I don't get?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 30, 2019

@frankie567 a = A creates a type alias. mypy doesn't allow these to rebound and they are special in some other ways. a2: Type[A] = A creates a variable, and it could be assigned a new value elsewhere. In particular, it could be bound to a subclass of A that mypy doesn't know anything about.

That's why mypy complains about using a2 as a base class -- for example, mypy can't bind super().__init__ correctly since a2 could refer to a subclass of A that changes the signature of __init__. Also, method overrides can't be checked, since there may be additional methods mypy doesn't see.

@frankie567
Copy link

frankie567 commented Oct 30, 2019

@JukkaL Thank you for this explanation, I understand now 🙂Sorry for the noise on the ticket!

@blueyed
Copy link
Contributor

blueyed commented Nov 7, 2019

This does not only affect bases from args, but also the local scope / function body.

I've looked a bit into why the following is not working, and it appears to be due to the fact that configure_base_classes does not know about base:

mypy/mypy/semanal.py

Lines 1472 to 1477 in a94e649

msg = 'Invalid base class'
name = self.get_name_repr_of_expr(base_expr)
if name:
msg += ' "{}"'.format(name)
self.fail(msg, base_expr)
info.fallback_to_any = True

There is some code in analyze_function_body already to bind things into the local scope, but it appears to not work here correctly / as expected?:

mypy/mypy/semanal.py

Lines 897 to 917 in a94e649

def analyze_function_body(self, defn: FuncItem) -> None:
is_method = self.is_class_scope()
with self.tvar_scope_frame(self.tvar_scope.method_frame()):
# Bind the type variables again to visit the body.
if defn.type:
a = self.type_analyzer()
a.bind_function_type_variables(cast(CallableType, defn.type), defn)
self.function_stack.append(defn)
self.enter(defn)
for arg in defn.arguments:
self.add_local(arg.variable, defn)
# The first argument of a non-static, non-class method is like 'self'
# (though the name could be different), having the enclosing class's
# instance type.
if is_method and not defn.is_static and not defn.is_class and defn.arguments:
defn.arguments[0].variable.is_self = True
defn.body.accept(self)
self.leave()
self.function_stack.pop()

import typing


# works
base = object
class C(base):
    pass


def f1() -> None:
    base = object
    class C(base):  # Variable "base" is not valid as a type / Invalid base class "base"
        pass


def f2(base: typing.Any) -> None:
    class C(base):  # Variable "base" is not valid as a type / Invalid base class "base"
        pass


# works
def f3(base: typing.Any) -> None:
    base2: typing.Any = base
    class C(base2):
        pass

mypy 0.750+dev.a94e649de794ecd169a5bcc42274e5205ffbb1fb

@youtux
Copy link

youtux commented May 21, 2022

Any update on this issue? It's quite a big one IMO.

@maxbane
Copy link

maxbane commented Nov 14, 2022

One more case, very similar to @blueyed's, but in the context of a classmethod -- and I also found a third incorrect error message (name-defined) that I don't think was mentioned yet in this thread: even though cls has type type[C] where C is bounded by MyClass, mypy is unable to resolve the reference to cls.InnerClass.

from typing import TypeVar

C = TypeVar("C", bound="MyClass")


class MyClass:
    class InnerClass:
        pass

    @classmethod
    def dynamic_subclass(cls: type[C]) -> type[C]:
        # Mypy emits two errors on the following line:
        # error: Variable "cls" is not valid as a type  [valid-type]
        # error: Invalid base class "cls"  [misc]
        class MySubClass(cls):
            # Mypy emits one error on the following line:
            # Name "cls.InnerClass" is not defined  [name-defined]
            class MySubInnerClass(cls.InnerClass):
                pass

            pass
        return MySubClass


# to show that everything works as expected at runtime:
SubClass = MyClass.dynamic_subclass()
my_subclass_instance = SubClass()

ilevkivskyi added a commit that referenced this issue Nov 21, 2022
Fixes #5865 

Looks quite easy and safe, unless I am missing something. Most changes
in the diff are just moving stuff around.

Previously we only applied argument types before type checking, but it
looks like we can totally do this in semantic analyzer. I also enable
variable annotated as `type` (or equivalently `Type[Any]`), this use
case was mentioned in the comments.

This PR also accidentally fixes two additional bugs, one related to type
variables with values vs walrus operator, another one for type variables
with values vs self types. I include test cases for those as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants