Repository navigation
Prohibit assignment to __class__ (Fixes #7724) - #19180
lsrafael13 wants to merge 7 commits into
Conversation
Sorry, something went wrong.
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.
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.
…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.
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.
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
|
Sorry, something went wrong.
sterliakov
left a comment
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?
Sorry, something went wrong.
| @@ -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.
Sorry, something went wrong.
| class D: | ||
| pass | ||
|
|
||
| [case test_assign_to_regular_attribute] |
There was a problem hiding this comment.
We certainly already have tests checking that.
Sorry, something went wrong.
| [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.
Sorry, something went wrong.
| 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
| 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.
Sorry, something went wrong.
|
And why on earth is this PR listed as fixing #7724? That one is much broader and doesn't seem to concern __class__ assignments at all? |
Sorry, something went wrong.
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