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

Dataclass with __module__ = "builtins" adds __builtins__.__builtins__ #105203

Closed
The-Compiler opened this issue Jun 1, 2023 · 9 comments
Closed
Labels
pending The issue will be closed if no feedback is provided stdlib Python modules in the Lib dir topic-dataclasses

Comments

@The-Compiler
Copy link
Contributor

Bug report

When a dataclass with __module__ = "builtins" is defined (which seems somewhat common for custom exception classes), this seems to result in a __builtins__ attribute to __builtins__ (which recursively points to the builtins module):

from dataclasses import dataclass

print(hasattr(__builtins__, "__builtins__"))

@dataclass
class Bla(Exception):

    __module__ = "builtins"

print(hasattr(__builtins__, "__builtins__"))

results in:

False
True

This in turn results in an import astroid.nodes causing an endless recursion, in the original issues due to how schemathesis uses dataclasses:

Downstream reports:

Your environment

  • CPython versions tested on: 3.7.16, 3.8.16, 3.9.16, 3.10.11, 3.11.3, 3.12.0b1
  • Operating system and architecture: Archlinux 64-bit
@The-Compiler The-Compiler added the type-bug An unexpected behavior, bug, or error label Jun 1, 2023
@terryjreedy
Copy link
Member

It is normal for modules created from Python to have attribute 'builtins'. I am not surprised that one is added here, so I doubt that this is a bug.

My recursive search of Lib/*.py failed to find __module__ = "builtins" or __module__ = 'builtins'. Dataclasses might never be used in the stdlib for a custom exception. Normal is class CustomError(Exception): pass or similar.

@ericvsmith Is there any point to using dataclasses for custom exceptions?

@The-Compiler
Copy link
Contributor Author

It is normal for modules created from Python to have attribute 'builtins'. I am not surprised that one is added here, so I doubt that this is a bug.

While that's true, it does seem very surprising to me that merely declaring a dataclass ends up dynamically adding a new attribute to an existing module, which wasn't there before.

@The-Compiler
Copy link
Contributor Author

The-Compiler commented Jun 1, 2023

From a (very) quick reading of the dataclasses source, it looks like this is actually happening because exec() gets called with the __module__ as globals:

>>> hasattr(__builtins__, "__builtins__")
False
>>> exec("", __builtins__.__dict__, {})
>>> hasattr(__builtins__, "__builtins__")
True

And this behavior actually seems to be documented:

If the globals dictionary does not contain a value for the key __builtins__, a reference to the dictionary of the built-in module builtins is inserted under that key. That way you can control what builtins are available to the executed code by inserting your own __builtins__ dictionary into globals before passing it to exec().

While that still seems rather surprising, I suppose that's working as documented then?

@ericvsmith
Copy link
Member

Yes, this is because of the exec() behavior. I'm curious why __module__ = "builtins" is being used.

@The-Compiler
Copy link
Contributor Author

The-Compiler commented Jun 1, 2023

From what I can gather, it's a hack to see InvalidSchema in an error message, rather than schemathesis.exceptions.InvalidSchema.

I have a feeling I've seen this hack used elsewhere before as well, but I can't remember where exactly.

edit: Might have been this bit in pytest I'm thinking of.

@terryjreedy terryjreedy added pending The issue will be closed if no feedback is provided and removed type-bug An unexpected behavior, bug, or error labels Jun 1, 2023
@terryjreedy
Copy link
Member

I see no CPython issue here.

@ericvsmith
Copy link
Member

From what I can gather, it's a hack to see InvalidSchema in an error message, rather than schemathesis.exceptions.InvalidSchema.

Irrespective of this particular issue, it seems like a bad idea to hide the type of an exception.

@Stranger6667
Copy link

@The-Compiler

You're right about the reason it was implemented this way. I found this hack exactly in the place in pytest you've mentioned and used it in Schemathesis.

@ericvsmith
Copy link
Member

I don't think there's anything to do here, so I'm going to close this.

@ericvsmith ericvsmith closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending The issue will be closed if no feedback is provided stdlib Python modules in the Lib dir topic-dataclasses
Projects
None yet
Development

No branches or pull requests

6 participants