Skip to content

Fix __init__ in Dataclasses inheriting from Any #11966

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

Merged
merged 3 commits into from
Jan 17, 2022

Conversation

joey-laminar
Copy link
Contributor

Description

Fixes #11921

Dataclasses that inherit from Any (actually another type that we don't
know) should allow any constructor.

The way this bug was noticed was while importing a class that mypy didn't know
about with the --ignore-missing-imports flag:

import dataclasses
from mypackage import MyClass

# @dataclasses.dataclass
# class MyClass:
#     bar: str

@dataclasses.dataclass
class Inherits(MyClass):
    foo: str

i = Inherits(foo="hi", bar="bye")

Test Plan

Added appropriate test case

Dataclasses that inherit from Any (actually another type that we don't
know) should allow any constructor.

Fixes python#11921

if info.fallback_to_any:
args = [Argument(Var('args'), AnyType(TypeOfAny.explicit), None, ARG_STAR),
Argument(Var('kwargs'), AnyType(TypeOfAny.explicit), None, ARG_STAR2),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really want to give the variables names (args, kwargs) but I couldn't find a better way to do it without writing a lot of code to duplicate add_method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the child dataclass has a field named args or kwargs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still works fine because we completely overwrite the args to init with these args. The original fields won't appear

Copy link
Contributor Author

@joey-laminar joey-laminar Jan 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a workaround without using variable names. Using an empty var for both arguments won't work unless it is the same empty var, that way mypy won't throw an error.

The main difference is in the result of reveal_type(A) where A is a dataclass inheriting from any. When using args and kwargs it appeared as:

a.py:9: note: Revealed type is "def (*args: Any, **kwargs: Any) -> a.A"

and now it appears as

a.py:9: note: Revealed type is "def (*Any, **Any) -> a.A"

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this! We should still complain about A(a="asdf", b=2) and A(b=2) (if a doesn't have a default). This could be done by forcing argument kinds to be ARG_NAMED_OPT or ARG_NAMED.

@joey-laminar
Copy link
Contributor Author

joey-laminar commented Jan 11, 2022

Thanks for catching this! We should still complain about A(a="asdf", b=2) and A(b=2) (if a doesn't have a default). This could be done by forcing argument kinds to be ARG_NAMED_OPT or ARG_NAMED.

Hmm, but we also want to accept A(2). And it is hard to type check at all because if we call A("A", False, 3) the 3 is presumably the int argument.

I think the best we can do it to force the arguments to be ARG_OPT, so that we will at least catch a wrong type when called with keywords but will miss the case where the argument is not present at all. I think this is a reasonable fallback since we're pretty much falling back to Any since we're missing the base class

@joey-laminar
Copy link
Contributor Author

Updated type of positional arguments to be ARG_OPT. Now the function signature (is illegal in python but) does what we want:

Revealed type is "def (*Any, a: builtins.int =, **Any) -> a.A"

Meaning if there is an a sent it has to be an int, otherwise types can be whatever.

The only thing this doesn't catch is not sending any arguments, but I don't think that we can handle that while not knowing previous arguments. What we really want would be something like def (*Any, a: builtins.int, **Any) -> a.A meaning any number of positional arguments followed by an int, but python does not let us do that.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for thinking this through! Looks good to me, but I'll leave open for a couple days in case others see something I don't :-)

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@JukkaL JukkaL merged commit 1aa9cf9 into python:master Jan 17, 2022
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
Dataclasses that inherit from Any (actually another type that we don't
know) should assume an (almost) arbitrary constructor.

Fixes python#11921
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inheriting from Any with dataclass
4 participants