Skip to content

BUILTINS is not a valid field name for a frozen dataclass #96151

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

Closed
hauntsaninja opened this issue Aug 21, 2022 · 1 comment
Closed

BUILTINS is not a valid field name for a frozen dataclass #96151

hauntsaninja opened this issue Aug 21, 2022 · 1 comment
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Aug 21, 2022

This is not a bug that I encountered in the wild, but I happened to notice the possibility for this while reading the source.

from dataclasses import dataclass
@dataclass(frozen=True)
class X:
    BUILTINS: str
X(5)

will give you:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 3, in __init__
AttributeError: 'int' object has no attribute 'object'

There are other field names that you can use to trick dataclass into bad behaviour, but all of the other ones are prefixed by an underscore.

So one "fix" is pretty simple: just rename BUILTINS to _BUILTINS in dataclasses.py to bring it in line with the others.

If a single underscore doesn't feel like sufficient "this is the user's fault", we could use __dataclass_* (like we do for __dataclass_self__ param to __init__)

For what it's worth, the only thing we need BUILTINS for is so we can access object inside __init__, so an alternative fix could be to use ().__class__.__base__ instead of BUILTINS.object (and skip the locals dance needed to define BUILTINS altogether)

If any of that sounds worth doing, I'd be happy to open a PR

@hauntsaninja hauntsaninja added the type-bug An unexpected behavior, bug, or error label Aug 21, 2022
@AlexWaygood AlexWaygood added the stdlib Python modules in the Lib dir label Aug 21, 2022
hauntsaninja added a commit to hauntsaninja/cpython that referenced this issue Oct 10, 2022
There's no indication that BUILTINS is a special name. Other names that
are special to dataclass are all prefixed by an underscore.

As mentioned in the issue, we can also avoid this locals dance
altogether by using `().__class__.__base__` instead of
`BUILTINS.object`.
ericvsmith pushed a commit that referenced this issue Oct 31, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 31, 2022
… This now allows for a field named BUILTIN (pythongh-98143)

(cherry picked from commit 29f98b4)

Co-authored-by: Shantanu <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 31, 2022
… This now allows for a field named BUILTIN (pythongh-98143)

(cherry picked from commit 29f98b4)

Co-authored-by: Shantanu <[email protected]>
ericvsmith pushed a commit that referenced this issue Oct 31, 2022
…. This now allows for a field named BUILTIN (gh-98143) (gh-98899)

gh-96151: Use a private name for passing builtins to dataclass. This now allows for a field named BUILTIN (gh-98143)
(cherry picked from commit 29f98b4)

Co-authored-by: Shantanu <[email protected]>

Co-authored-by: Shantanu <[email protected]>
ericvsmith pushed a commit that referenced this issue Oct 31, 2022
…. This now allows for a field named BUILTIN (gh-98143) (gh-98900)

gh-96151: Use a private name for passing builtins to dataclass. This now allows for a field named BUILTIN (gh-98143)
(cherry picked from commit 29f98b4)

Co-authored-by: Shantanu <[email protected]>

Co-authored-by: Shantanu <[email protected]>
@ericvsmith
Copy link
Member

Merge and backports complete. Thanks, @hauntsaninja !

hauntsaninja added a commit to hauntsaninja/cpython that referenced this issue Feb 18, 2023
This commit prefixes `__dataclass` to several things in the locals dict:
- Names like _dflt_ (which cause trouble, see first test)
- Names like _type_ (not known to be able to cause trouble)
- _return_type (not known to able to cause trouble)
- _HAS_DEFAULT_FACTORY (which causes trouble, see second test)

In addition, this removes `MISSING` from the locals dict. As far as I
can tell, this wasn't needed even in the initial implementation of
dataclasses.py (and tests on that version passed with it removed).

This is basically a continuation of python#96151, where fixing this was
welcomed in python#98143 (comment)
hauntsaninja added a commit that referenced this issue Mar 25, 2023
…mes (#102032)

This commit prefixes `__dataclass` to several things in the locals dict:
- Names like `_dflt_` (which cause trouble, see first test)
- Names like `_type_` (not known to be able to cause trouble)
- `_return_type` (not known to able to cause trouble)
- `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test)

In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-)

This is basically a continuation of #96151, where fixing this was welcomed in #98143 (comment)
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
…ore names (python#102032)

This commit prefixes `__dataclass` to several things in the locals dict:
- Names like `_dflt_` (which cause trouble, see first test)
- Names like `_type_` (not known to be able to cause trouble)
- `_return_type` (not known to able to cause trouble)
- `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test)

In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-)

This is basically a continuation of python#96151, where fixing this was welcomed in python#98143 (comment)
warsaw pushed a commit to warsaw/cpython that referenced this issue Apr 11, 2023
…ore names (python#102032)

This commit prefixes `__dataclass` to several things in the locals dict:
- Names like `_dflt_` (which cause trouble, see first test)
- Names like `_type_` (not known to be able to cause trouble)
- `_return_type` (not known to able to cause trouble)
- `_HAS_DEFAULT_FACTORY` (which causes trouble, see second test)

In addition, this removes `MISSING` from the locals dict. As far as I can tell, this wasn't needed even in the initial implementation of dataclasses.py (and tests on that version passed with it removed). This makes me wary :-)

This is basically a continuation of python#96151, where fixing this was welcomed in python#98143 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants