python / python/mypy

Misbehavior of `expand_type` in pydantic mypy plugin

Open
#16,087 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

topic-plugins
Dominant language
Python
Stars
20.6k
Forks
3.3k
PR merge metrics
PR metrics pending

Description

In writing the pydantic mypy plugin, I have leaned heavily on the code from the dataclasses plugin. Recently, a couple issues have been reported with misbehavior that I was able to track down to being caused by a call to expand_type changing the type from Union[str, None] to just str.

In particular, the following code snippet is misbehaving while using the pydantic mypy plugin (reported in https://github.com/pydantic/pydantic/issues/7399):

import pydantic

class Model(pydantic.BaseModel):
    model_config = {'frozen': True}
    my_var: str | None = pydantic.Field(default=None)

class SubModel(Model):
    pass

reveal_type(SubModel.my_var)
#> note: Revealed type is "builtins.str"

(The revealed type should have been "Union[builtins.str, None]".)

If I change

class Model(pydantic.BaseModel):
    model_config = {'frozen': True}
    ...

to

import dataclasses
@dataclasses.dataclass(frozen=True)
class Model:
    ...

I do get the expected reveal_type result of "Union[builtins.str, None]".

After a bit of digging, I was able to trace this down to the code that freezes fields, and in particular, if I comment out this block, the issue goes away. Digging further, I found the issue was that, in this line, I would sometimes end up getting str as the "expanded type" for Union[str, None].

Of course, I also noticed the verbatim warning comment, which was taken from the dataclasses plugin here. It seems that the caveats:

however this plugin is called very late...

only apply to the dataclasses plugin, as I do not get the same (correct) behavior in the pydantic plugin. I was able to find a hacky work-around for this specific failure (included in this PR), where, if I passed in a Union and got out a non-Union from expand_type I defer analysis, and this did resolve one of the reported issues, but it feels to me like there are probably many other cases that that won't handle correctly, and I'm not sure what exactly I can/should do to handle this more correctly.

Any suggestions about how to ensure that the pydantic plugin behaves the same as the dataclasses plugin with regard to expand_type would be appreciated! (Or other suggestions about how to improve the plugin logic, of course.)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reproducer in the issue and inspect pydantic/mypy.py at the field-freezing block around lines 876-880 and the expand_type call around line 354. Compare the behavior with mypy/plugins/dataclasses.py, then verify that reveal_type for the inherited optional field remains Union[builtins.str, None] without breaking the plugin's other field-freezing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.