[cherry-pick] Fix mypy plugin for 1.1.0 (#5077)

[cdce8p]

(cherry picked from commit 6267ae3)

Manual backport to avoid and resolve merge conflict. Alternative to #5110.

[KotlinIsland]

I couldn't test this with V2 because mypy just ran forever, but I think this is still broken on this branch (at least with dataclasses).

[tool.poetry]
name = "test-project"
version = "1"
description = ""
authors = []

[tool.poetry.dependencies]
python = "~3.11"
mypy = { git = "https://github.com/python/mypy.git", rev = "release-1.1" }
pydantic = { git = "https://github.com/cdce8p/pydantic", rev = "backport-plugin-fix"}

[tool.mypy]
plugins = ["pydantic.mypy"]

import pydantic

@pydantic.dataclasses.dataclass
class A:
    a: str

A("")  # error: Too many positional arguments for "A"  [misc]

[cdce8p]

I couldn't test this with V2 because mypy just ran forever, but I think this is still broken on this branch (at least with dataclasses).

See #5077 (comment)

[cdce8p]

@samuelcolvin This PR is ready. I guess the change-file-checks failure is because of the manual backport.

[samuelcolvin]

we'll need to rename the change file so the link is right in the changelog, I'll do that when I get time.

[cdce8p]

we'll need to rename the change file so the link is right in the changelog, I'll do that when I get time.

I can do it, no problem. However, as it isn't renamed by the backport bot, I though it would be good here. It's basically the same PR as #5110, just without the merge conflict.
https://github.com/pydantic/pydantic/pull/5110/files#diff-7346c71569c422ee0520a5f2de506f6e959d587acb20aa72f90be6db4dfa7d82R1

[samuelcolvin]

I guess we could either extend the backport bot to rename the change file, or change hooky to recognise the change file, but for now it, I think it's easiest just to rename it manually.

[dmontagu]

I have run the mypy tests with mypy==1.1.1; here is a breakdown of the failures:

• In all plugin-using tests, the only new errors have code empty-body — in 1.1.1, empty bodies don't pass type checking. (Might make sense to update the tests so that they don't actually have empty bodies..)
• In non-plugin-using tests, we have the following other categories of new errors:
  • Mypy errors are emitted if keyword arguments are not of the annotated type. (I.e., it doesn't allow for validation to change the provided type.)
  • Mypy errors are emitted for non-type-annotated fields.
  • Mypy does not treat fields with value Field(xyz) as having default value xyz. I have confirmed that this can be resolved by changing to the form Field(default=xyz).
  • Passing "extra" keyword arguments that are not annotated fields cause mypy errors.

I consider all of the above new kinds of errors to be acceptable, especially considering that, with the exception of empty-body (which is not pydantic-specific), everything behaves the same as before when using the plugin.

@samuelcolvin the only thing I'm not sure of is if there are any concerns with adding the import dataclasses line, as discussed here: https://github.com/pydantic/pydantic/pull/5120/files#r1118839536. It seems maybe this approach was agreed on as acceptable but I just want to confirm before merging.

If the above is acceptable, I think we can merge this and cut a new 1.10.X release.

[dmontagu]

Oh actually, @cdce8p could you check and confirm that, with this branch, there isn't anything else that could/should be fixed that I haven't already discussed in my previous comment?

[cdce8p]

Oh actually, @cdce8p could you check and confirm that, with this branch, there isn't anything else that could/should be fixed that I haven't already discussed in my previous comment?

AFAICT those are the same observations I saw during testing. So it should be good.
I saw one other issue with the mypy plugin, in particular with allow_population_by_field_name #5070. This can probably be address at a later point though as it's not related to mypy 1.1.1.

the only thing I'm not sure of is if there are any concerns with adding the import dataclasses line, as discussed here: https://github.com/pydantic/pydantic/pull/5120/files#r1118839536. It seems maybe this approach was agreed on as acceptable but I just want to confirm before merging.

Some background, this was still an open question in #5120. However, I noticed that other files already import dataclasses, so importing it in pydantic.dataclasses shouldn't effect the import time as it's already loaded. At least that's the case for v2. Searching v1.10.x it looks as if it would indeed be an additional import unfortunately. Not sure though it's worth changing it (and if we do, how).

[dmontagu]

Okay, in the interest of keeping things simple how about I just change it back to the imports being the same. I think in that case it will be non-controversial.

[dmontagu]

Okay actually I take it back, I see we are actually using it in a more meaningful way than I expected.

Considering how fast dataclasses is to import, I think it should be okay, but I'll let @samuelcolvin make the final call.

[cdce8p]

@dmontagu Thanks for the quick release! Just confirmed with Home Assistant, everything is working as expected (when using the plugin).