Skip to content

Commit

Permalink
Warn about buggy type resolution
Browse files Browse the repository at this point in the history
This type annotation resolution is a flawed implementation -- ideally a
proper fix can come later, but for now it's worth warning about the
present behavior.

`dataclasses-json` does not behave correctly when using string-style
annotations (a frequent tactic for annotating types that have not yet
been defined at that point in the file).

The core problem comes from the assumption that type annotations in a
module will match `sys.modules()` -- `maybe_resolved` will frequently
resolve to a different value than a typechecker would!

A short example of the error
============================

`example_types.py`:
```python
from dataclasses import dataclass

from dataclasses_json import dataclass_json

@dataclass_json
@DataClass
class Config:
    options: list["Option"]

@dataclass_json
@DataClass
class Option:
    label: str
```

Expected behavior
-----------------
`_decode_items` correctly identifies `Option` as coming from
`example_types`:

```python
>>> from example_types import Config
>>> print(Config.from_dict({"options": [{"label": "repro"}]}).options)
[Option(label='repro')]
```

Unexpected behavior
-------------------
`_decode_items()` incorrectly identifies `"Option"` as from a third
party module that is not in scope at time of annotation:

`repro.py`:
```python
import click.parser  # Has `Option`
from example_types import Config

print(Config.from_dict({"options": [{"label": "repro"}]}).options)
```

```bash
$ python3.10 repro.py
[{'label': 'repro'}]
```

Related fix -- truthiness
=========================
The conditional `if maybe_resolved` is meant to handle the case of an
attribute not being found in the module (it could basically be written
as `if maybe_resolved is not None`). However, that doesn't cover the
case of types that are falsy at runtime!

```python
CustomNoneType = None

values: list['CustomNoneType'] = [None, None]
```

We can just use `hasattr()` instead.
  • Loading branch information
DavidCain committed May 17, 2024
1 parent 669ee33 commit 10abfc0
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions dataclasses_json/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,11 @@ def _decode_items(type_args, xs, infer_missing):
hence the check of `is_dataclass(vs)`
"""
def handle_pep0673(pre_0673_hint: str) -> Union[Type, str]:
for module in sys.modules:
maybe_resolved = getattr(sys.modules[module], type_args, None)
if maybe_resolved:
for module in sys.modules.values():
if hasattr(module, type_args):
maybe_resolved = getattr(module, type_args)
warnings.warn(f"Assuming hint {pre_0673_hint} resolves to {maybe_resolved} "
"This is not necessarily the value that is in-scope.")
return maybe_resolved

warnings.warn(f"Could not resolve self-reference for type {pre_0673_hint}, "
Expand Down

0 comments on commit 10abfc0

Please sign in to comment.