Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
6dbaf9c
Add signature for dataclasses.replace
ikonst Mar 7, 2023
4dcbe44
add ClassVar
ikonst Mar 7, 2023
7ed3741
prevent misleading note
ikonst Mar 7, 2023
89257b5
stash in a secret class-private
ikonst Mar 8, 2023
32b1d47
fix typing
ikonst Mar 8, 2023
1f08816
docs and naming
ikonst Mar 8, 2023
c456a5f
inst -> obj
ikonst Mar 8, 2023
8118d29
add the secret symbol to deps.test
ikonst Mar 8, 2023
789cb2b
language
ikonst Mar 8, 2023
9f0974c
nit
ikonst Mar 8, 2023
a37e406
nit
ikonst Mar 8, 2023
367c0e9
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst Mar 9, 2023
0e84c4f
make obj positional-only
ikonst Mar 14, 2023
9cfc081
Merge branch '2023-03-06-dataclasses-replace' of https://github.com/i…
ikonst Mar 14, 2023
7b907cf
add pythoneval test
ikonst Mar 14, 2023
985db60
use py3.7 syntax
ikonst Mar 14, 2023
15dbb7b
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst Mar 17, 2023
3227fde
Fix lint
ikonst Mar 17, 2023
2dbf249
use syntactically invalid name for symbol
ikonst Mar 30, 2023
b32881c
Merge branch '2023-03-06-dataclasses-replace' of https://github.com/i…
ikonst Mar 30, 2023
26056a4
Merge remote-tracking branch 'origin/master' into 2023-03-06-dataclas…
ikonst Mar 30, 2023
40315b7
mypy-replace must use name mangling prefix
ikonst Mar 30, 2023
c005895
Generic dataclass support
ikonst Mar 30, 2023
d71bc21
add fine-grained test
ikonst Mar 30, 2023
d914b94
disable for arbitrary transforms
ikonst Mar 31, 2023
2cb6dee
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst Apr 6, 2023
5735726
fix testDataclassTransformReplace
ikonst Apr 7, 2023
d38897e
better error message for generics
ikonst Apr 7, 2023
04f0ee3
add support for typevars
ikonst Apr 8, 2023
f402b86
streamline
ikonst Apr 8, 2023
306c3f3
self-check
ikonst Apr 8, 2023
9b491f5
Merge remote-tracking branch 'origin/master' into 2023-03-06-dataclas…
ikonst Apr 21, 2023
283fe3d
Add improved union support from #15050
ikonst Apr 21, 2023
29780e9
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst May 1, 2023
9c43ab6
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst May 3, 2023
94024ba
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst May 9, 2023
70240c4
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst May 16, 2023
99bf973
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst May 17, 2023
8fe75a7
add to testReplaceUnion
ikonst May 22, 2023
bea50e8
testReplaceUnion: fix B.y to be int
ikonst May 22, 2023
cd35951
rename testcase replace to testReplace
ikonst May 22, 2023
d71a7c0
Merge remote-tracking branch 'origin/master' into 2023-03-06-dataclas…
ikonst May 22, 2023
957744e
Merge branch 'master' into 2023-03-06-dataclasses-replace
ikonst Jun 2, 2023
6abf6ff
Merge remote-tracking branch 'origin/master' into pr/ikonst/14849
ikonst Jun 12, 2023
4c4fc94
remove get_expression_type hack
ikonst Jun 12, 2023
be4a290
assert isinstance(replace_sig, ProperType)
ikonst Jun 17, 2023
ee0ae21
add a TODO for _meet_replace_sigs
ikonst Jun 17, 2023
3f656f8
Merge remote-tracking branch 'origin/master' into 2023-03-06-dataclas…
ikonst Jun 17, 2023
65d6e89
Merge remote-tracking branch 'origin/master' into 2023-03-06-dataclas…
ikonst Jun 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion mypy/plugins/dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from mypy import errorcodes, message_registry
from mypy.expandtype import expand_type
from mypy.messages import format_type_bare
from mypy.nodes import (
ARG_NAMED,
ARG_NAMED_OPT,
Expand Down Expand Up @@ -35,7 +36,7 @@
TypeVarExpr,
Var,
)
from mypy.plugin import ClassDefContext, SemanticAnalyzerPluginInterface
from mypy.plugin import ClassDefContext, FunctionSigContext, SemanticAnalyzerPluginInterface
from mypy.plugins.common import (
_get_decorator_bool_argument,
add_attribute_to_class,
Expand Down Expand Up @@ -769,3 +770,63 @@ def _is_dataclasses_decorator(node: Node) -> bool:
if isinstance(node, RefExpr):
return node.fullname in dataclass_makers
return False


def replace_function_sig_callback(ctx: FunctionSigContext) -> CallableType:
"""
Generates a signature for the 'dataclasses.replace' function that's specific to the call site
and dependent on the type of the first argument.
"""
if len(ctx.args) != 2:
# Ideally the name and context should be callee's, but we don't have it in FunctionSigContext.
ctx.api.fail(f'"{ctx.default_signature.name}" has unexpected type annotation', ctx.context)
return ctx.default_signature

if len(ctx.args[0]) != 1:
return ctx.default_signature # leave it to the type checker to complain

inst_arg = ctx.args[0][0]

# <hack>
from mypy.checker import TypeChecker

assert isinstance(ctx.api, TypeChecker)
obj_type = ctx.api.expr_checker.accept(inst_arg)
# </hack>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW this kind of hack is quite common (I myself used it several times, for old sqlalchemy plugin, and internally). I would propose to instead add another plugin hook, e.g. get_function_signature_hook_2 (similar to what we have for classes), that would be called after infer_arg_types_in_context() with a context that includes inferred argument types.

cc @JukkaL what do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This proposal still stands. Do we have an issue for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's #14845 and #10216, though not one spelling out the "get_function_signature_hook_2" solution.


obj_type = get_proper_type(obj_type)
if not isinstance(obj_type, Instance):
return ctx.default_signature
inst_type_str = format_type_bare(obj_type)

metadata = obj_type.type.metadata
dataclass = metadata.get("dataclass")
if not dataclass:
ctx.api.fail(
f'Argument 1 to "replace" has incompatible type "{inst_type_str}"; expected a dataclass',
ctx.context,
)
return ctx.default_signature

arg_names = [None]
arg_kinds = [ARG_POS]
arg_types = [obj_type]
for attr in dataclass["attributes"]:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wish I could've deserialized metadata, or even accessed a list of DataclassAttributes that's already been deserialized...

Or maybe the right way to go is to replace dataclasses.replace with an OverloadedFuncDef that the transform (which has full context) will keep adding to.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The OverloadedFuncDef appears to be a non-starter since by the time I can do anything to replace dataclasses.replace with a synthesized OverloadedFuncDef, the call expression has been resolved with the pointer to the generic FunctionDef.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, solved it by creating a "secret" symbol with a signature at semantic analysis time, then using this signature in the function sig callback.

if not attr["is_in_init"]:
continue
arg_names.append(attr["name"])
arg_kinds.append(
ARG_NAMED if not attr["has_default"] and attr["is_init_var"] else ARG_NAMED_OPT
)
arg_types.append(ctx.api.named_type(attr["type"]))

return ctx.default_signature.copy_modified(
arg_names=arg_names,
arg_kinds=arg_kinds,
arg_types=arg_types,
ret_type=obj_type,
name=f"{ctx.default_signature.name} of {inst_type_str}",
# prevent 'dataclasses.pyi:...: note: "replace" of "A" defined here' notes
# since they are misleading: the definition is dynamic, not from a definition
definition=None,
)
4 changes: 3 additions & 1 deletion mypy/plugins/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@ def get_function_hook(self, fullname: str) -> Callable[[FunctionContext], Type]
def get_function_signature_hook(
self, fullname: str
) -> Callable[[FunctionSigContext], FunctionLike] | None:
from mypy.plugins import attrs
from mypy.plugins import attrs, dataclasses

if fullname in ("attr.evolve", "attrs.evolve", "attr.assoc", "attrs.assoc"):
return attrs.evolve_function_sig_callback
elif fullname == "dataclasses.replace":
return dataclasses.replace_function_sig_callback
return None

def get_method_signature_hook(
Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -2001,3 +2001,26 @@ class Bar(Foo): ...
e: Element[Bar]
reveal_type(e.elements) # N: Revealed type is "typing.Sequence[__main__.Element[__main__.Bar]]"
[builtins fixtures/dataclasses.pyi]

[case testReplace]
from dataclasses import dataclass, replace, InitVar
from typing import ClassVar

@dataclass
class A:
x: int
q: InitVar[int]
q2: InitVar[int] = 0
c: ClassVar[int]


a = A(x=42, q=7)
a2 = replace(a) # E: Missing named argument "q" for "replace" of "A"
a2 = replace(a, q=42)
a2 = replace(a, x=42, q=42)
a2 = replace(a, x=42, q=42, c=7) # E: Unexpected keyword argument "c" for "replace" of "A"
a2 = replace(a, x='42', q=42) # E: Argument "x" to "replace" of "A" has incompatible type "str"; expected "int"
a2 = replace(a, q='42') # E: Argument "q" to "replace" of "A" has incompatible type "str"; expected "int"
reveal_type(a2) # N: Revealed type is "__main__.A"

[builtins fixtures/dataclasses.pyi]
2 changes: 2 additions & 0 deletions test-data/unit/lib-stub/dataclasses.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ def field(*,


class Field(Generic[_T]): pass

def replace(obj: _T, **changes: Any) -> _T: ...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is pretty different to typeshed's stub for replace. Would it be worth adding a pythoneval test as well, to check that this machinery all works with the full typeshed stubs for the stdlib, outside the context of mypy's test suite?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The stub has:

def replace(__obj: _DataclassT, **changes: Any) -> _DataclassT: ...

I can rename obj to __obj in this fixture. We can also change the stub to better reflect reality:

if sys.version_info >= (3, 9):
    def replace(obj: _DataclassT, /, **changes: Any) -> _DataclassT: ...
else:
   def replace(obj: _DataclassT, **changes: Any) -> _DataclassT: ...

Can we just use the Python 3.9 syntax even for Python 3.8 targets? Passing obj as kw is deprecated in Python 3.8 so being a checker/linter we might as well just disallow it, no?

Do you think it matters much? For the plugin to function, replace has to:

  1. exist
  2. have a first argument

The **changes are optional, the retval is overwritten, and FWIW it doesn't look for a keyword arg obj since it's deprecated since Python 3.8 and was never a good idea to begin with, so we might as well just not support it, right?

Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood Mar 14, 2023

Choose a reason for hiding this comment

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

We can also change the stub to better reflect reality:

No need: prepending the parameter name with __ is the py37-compatible way of declaring that a parameter is positional-only, as is described in PEP 484: https://peps.python.org/pep-0484/#positional-only-arguments. So the obj parameter is already marked as positional-only on all Python versions in typeshed's stubs.

I don't think you need to change anything in your existing fixtures/tests for this PR, I'm just suggesting adding a single additional test in the pythoneval.test file. Unlike mypy's other tests, the tests in that file don't use fixtures — they run with typeshed's full stdlib stubs. So it's a good sanity check to make sure that the machinery you've added works with typeshed's stubs for this function as well as with mypy's fixtures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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