Prohibit assignment to __class__ (Fixes #7724) #19180
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements a check to disallow assignments to self.__class__, as these can cause unsafe runtime behavior. Includes tests for __class__ and related dunder attributes. Fixes python#7724
Adds a dummy comment to force GitHub Actions to run CI on the updated branch.
This comment has been minimized.
This comment has been minimized.
Move __class__ assignment check to use self.chk.fail() in VarAssignVisitor, fixing AttributeError. Also remove temporary test comment used for triggering GitHub Actions.
This comment has been minimized.
This comment has been minimized.
…Visitor (Fixes python#7724) Replaces previous incorrect call to self.chk.fail() with self.errors.report(), resolving AttributeError. Ensures assignment to __class__ is properly reported as an error during instance attribute assignments.
This comment has been minimized.
This comment has been minimized.
Added a check inside TypeChecker.visit_assignment_stmt to disallow assignments to '__class__', as such assignments are unsafe and may lead to unpredictable behavior. The new check raises an error using self.fail when '__class__' is assigned, matching the style used in other TypeChecker checks. This change addresses issue python#7724.
This comment has been minimized.
This comment has been minimized.
Implemented the check inside TypeChecker.visit_assignment_stmt to disallow assignments to '__class__', as such assignments are unsafe. The check correctly uses self.fail() following mypy's error reporting conventions, and is located after initial assignment processing to align with the design of TypeChecker. Previous redundant logic inside VarAssignVisitor.visit_assignment_stmt has been removed. This change addresses issue python#7724.
for more information, see https://pre-commit.ci
|
Diff from mypy_primer, showing the effect of this PR on open source code: zipp (https://github.com/jaraco/zipp)
+ zipp/__init__.py:163: error: Assignment to '__class__' is unsafe and not allowed [misc]
prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/_internal/_logging.py:30: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ src/prefect/_internal/_logging.py:38: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ src/prefect/client/base.py:174: error: Assignment to '__class__' is unsafe and not allowed [misc]
porcupine (https://github.com/Akuli/porcupine)
+ porcupine/utils.py:475: error: Assignment to '__class__' is unsafe and not allowed [misc]
hydpy (https://github.com/hydpy-dev/hydpy)
+ hydpy/docs/sphinx/conf.py:166: error: Assignment to '__class__' is unsafe and not allowed [misc]
scipy (https://github.com/scipy/scipy)
+ scipy/interpolate/_fitpack2.py:326: error: Assignment to '__class__' is unsafe and not allowed [misc]
alerta (https://github.com/alerta/alerta)
+ alerta/models/alarms/__init__.py:34: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ alerta/database/base.py:56: error: Assignment to '__class__' is unsafe and not allowed [misc]
werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/wrappers/response.py:239: error: Assignment to '__class__' is unsafe and not allowed [misc]
spack (https://github.com/spack/spack)
+ lib/spack/llnl/util/lang.py:745: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ lib/spack/llnl/util/lang.py:747: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ lib/spack/spack/builder.py:133: error: Assignment to '__class__' is unsafe and not allowed [misc]
discord.py (https://github.com/Rapptz/discord.py): 1.06x faster (297.0s -> 279.3s in single noisy sample)
steam.py (https://github.com/Gobot1234/steam.py)
+ steam/protobufs/msg.py:161: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ steam/protobufs/msg.py:212: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ steam/protobufs/msg.py:297: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ steam/protobufs/msg.py:322: error: Assignment to '__class__' is unsafe and not allowed [misc]
sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/logging.py: note: In member "filter" of class "SphinxLogRecordTranslator":
+ sphinx/util/logging.py:505:13: error: Assignment to '__class__' is unsafe and not allowed [misc]
+ sphinx/util/logging.py:505:13: note: Error code "misc" not covered by "type: ignore" comment
|
sterliakov
suggested changes
Jun 3, 2025
There was a problem hiding this comment.
I' not sure rejecting __class__ assignments altogether is the most user-friendly approach. While it is indeed unsafe and is not supported by mypy, it is important for many projects (see primer fallback). This isn't a typing error per se, just a declaration "we do not support it", which probably shouldn't be a diagnostic?
| @@ -0,0 +1,22 @@ | |||
| [case test_assign_to_dunder_class] | |||
There was a problem hiding this comment.
For some historical reason, all our testcases use camelCase naming scheme. Also I don't think this deserves its own file, probably just appending to check-statements.test would be more reasonable.
| class D: | ||
| pass | ||
|
|
||
| [case test_assign_to_regular_attribute] |
There was a problem hiding this comment.
We certainly already have tests checking that.
| [case test_assign_to_dunder_class] | ||
| class A: | ||
| def foo(self): | ||
| self.__class__ = B # E: Assignment to '__class__' is unsafe and not allowed |
There was a problem hiding this comment.
Please also add testcases with non-self assignments and assignments to self.__class__ in __init__ (they're slightly different AFAIC).
I'd also love to see tests where this is assigned to various other destinations:
b = object()
b.__class__ = something
class A:
__class__ = something
# etc.
| for lv in s.lvalues: | ||
| if isinstance(lv, MemberExpr): | ||
| if lv.name == "__class__": | ||
| self.fail("Assignment to '__class__' is unsafe and not allowed", lv) |
There was a problem hiding this comment.
This wording is too strong - assignments are fine themselves, there's nothing inherently unsafe with doing this, it's not a security vuln. Probably something along the lines of
Suggested change
| self.fail("Assignment to '__class__' is unsafe and not allowed", lv) | |
| self.fail("Assignment to '__class__' is not supported", lv) |
(probably accompanied by a self.note explaining it further) would be less intrusive.
|
And why on earth is this PR listed as fixing #7724? That one is much broader and doesn't seem to concern |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a type check that disallows assignments to class, which can lead to unsafe runtime behavior.
Includes tests for class assignments and verifies that other dunder and regular attributes remain assignable.
Fixes #7724