reject arbitrary attribute assignment on bound methods#4201
Conversation
A bound method is a types.MethodType, which does not override __setattr__/__delattr__, so assigning an arbitrary attribute to it fails at runtime. Pyrefly permitted it: lookup_magic_dunder_attr1 had no BoundMethod arm, so the base fell through to the general lookup and resolved __setattr__/__delattr__ via MethodType.__getattr__ (which returns Any), making the assignment look valid. Add a BoundMethod arm mirroring the ClassInstance handling: when the mutating dunder is inherited from object, report the attribute as not-found so the set/delete is rejected. Attribute reads are unchanged. Closes facebook#4163
|
Diff from mypy_primer, showing the effect of this PR on open source code: dragonchain (https://github.com/dragonchain/dragonchain)
+ ERROR dragonchain/transaction_processor/level_5_actions_utest.py:256:9-81: Object of class `FunctionType` has no attribute `return_value` [missing-attribute]
+ ERROR dragonchain/transaction_processor/level_5_actions_utest.py:275:9-80: Object of class `FunctionType` has no attribute `side_effect` [missing-attribute]
+ ERROR dragonchain/transaction_processor/level_5_actions_utest.py:294:9-81: Object of class `FunctionType` has no attribute `return_value` [missing-attribute]
+ ERROR dragonchain/transaction_processor/level_5_actions_utest.py:312:9-81: Object of class `FunctionType` has no attribute `return_value` [missing-attribute]
|
|
heads up on the mypy_primer diff: the 4 new errors on dragonchain are all the |
|
I think this is probably fine. Thanks for the PR! |
|
@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D113405966. |
connernilsen
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
@yangdanny97 merged this pull request in a950986. |
a bound method is a
types.MethodType, which doesn't override__setattr__/__delattr__, so assigning an arbitrary attribute to it fails at runtime. pyrefly allowed it:lookup_magic_dunder_attr1had noBoundMethodarm, so the base fell through to the general lookup and resolved the dunder viaMethodType.__getattr__(returnsAny), which made the assignment look validadd a
BoundMethodarm mirroring theClassInstancehandling: when the mutating dunder is inherited fromobject, report it as not-found so the set/delete is rejected. attribute reads are unchangedcloses #4163