Skip to content
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

Pyright doesn't reject uses of Field for non-dataclasses #4980

Closed
adampauls opened this issue Apr 20, 2023 · 3 comments
Closed

Pyright doesn't reject uses of Field for non-dataclasses #4980

adampauls opened this issue Apr 20, 2023 · 3 comments
Labels
as designed Not a bug, working as intended

Comments

@adampauls
Copy link
Contributor

adampauls commented Apr 20, 2023

Describe the bug pyright does not complain when a dataclasses.field is used as the default value of an attribute on a non-dataclass.

To Reproduce

import dataclasses

class NonDataClass:
    my_list: list[int] = dataclasses.field(default_factory=list)

    def add_item(self, item: int) -> None:
        self.my_list.append(item)

instance = NonDataClass()
instance.add_item(1)
assert instance.my_list == [1]

pyright issues no error.

Expected behavior
I should get the same error as if I used a literal of the wrong type (like "5"):

error: Expression of type "Field" cannot be assigned to declared type "list[int]"

VS Code extension or command-line
On command-line using pyright 1.1.302 and 1.1.303.

Additional context
Perhaps this behavior is by design? Maybe it's idiomatic in python to use dataclasses.field as a default value and do a manual isinstance(self.my_list, Field) check in __init__ or something?

@adampauls
Copy link
Contributor Author

adampauls commented Apr 20, 2023

It appears that pyright is smart enough to narrow self.my_list in with this code:

    import dataclasses

    class NonDataClass:
        my_list: list[int] | dataclasses.Field[list[int]] = dataclasses.field(default_factory=list)

        def add_item(self, item: int) -> None:
            if isinstance(self.my_list, dataclasses.Field):
                assert isinstance(self.my_list.default_factory, Callable)
                self.my_list = self.my_list.default_factory()
            reveal_type(self.my_list)
            self.my_list.append(item)

    instance = NonDataClass()
    instance.add_item(1)
    assert instance.my_list == [1]

So I think it's reasonable to expect the user to explicitly declare the union type if they want to uses a dataclass.field on a non-dataclass?

@erictraut
Copy link
Collaborator

Pyright is doing the right thing here from a type checking standpoint. Pyright does apply special-case logic for dataclass.field if it is called within a dataclass field definition, but it otherwise doesn't apply any special meaning to it. It's a function, and it can be called anywhere you want. It's defined with overloads, and the applicable overload in this case returns an Any type, so its return type is compatible with list[int].

Not surprisingly, mypy also does not generate an error in this case.

@erictraut erictraut added the as designed Not a bug, working as intended label Apr 20, 2023
@adampauls
Copy link
Contributor Author

adampauls commented Apr 20, 2023

Oh my, I think it's even worse than that -- field just lies and says it returns list[int]

@overload
    def field(
        *,
        default_factory: Callable[[], _T],
        init: bool = True,
        repr: bool = True,
        hash: bool | None = None,
        compare: bool = True,
        metadata: Mapping[Any, Any] | None = None,
        kw_only: bool = ...,
    ) -> _T: ...

with the helpful comment:

# NOTE: Actual return type is 'Field[_T]', but we want to help type checkers
# to understand the magic that happens at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

2 participants