Add opt-in flag for method hook defining-class fullnames#21650
Add opt-in flag for method hook defining-class fullnames#21650itxsamad1 wants to merge 3 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
|
I think we should fix this instead of documenting errata, and that seems to be consensus in the issue too. |
method_fullname() now resolves the class where a method was defined via get_containing_type_info(), matching the documented plugin hook semantics (Base.method rather than Derived.method for inherited calls). Fixes python#19181
|
Updated per @A5rocks feedback — this is now a code fix, not a docstring errata update. What changed: Tests: Added Note: plugins that registered hooks on derived-class fullnames only (without also hooking the base class) may need updating — this aligns with the original documented intent per the issue discussion. |
|
Updated per @A5rocks feedback — this is now a code fix, not docs-only. method_fullname() uses TypeInfo.get_containing_type_info() so inherited calls like Derived().method() invoke plugin hooks with Base.method (the defining class), matching the documented/intended semantics discussed in #19181. Added regression test estMethodSignatureHookUsesDefiningClass and ran 276 plugin-related tests locally. |
|
Sorry but the issue also describes a backward compatibility approach to take. Anyways if this PR is purely LLM generated then we will close it. |
This comment has been minimized.
This comment has been minimized.
Keep call-site class names as the default for plugin method hooks, and add an opt-in flag to use defining-class names so plugins can migrate gradually. Fixes python#19181
|
Sorry about the duplicate comments earlier — that was my mistake. Re the backward-compat point: I reworked this so the old behavior stays the default. Hooks still get the call-site class unless you opt in with \use_method_hook_defining_class = True\ (or --use-method-hook-defining-class). The new test only enables that flag, so existing plugin tests should behave the same as before. Happy to adjust naming or rollout if you'd prefer a different migration path. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
This adds an opt-in flag for the defining-class behavior discussed in #19181.
By default, plugin method hooks still receive the call-site class name (e.g. Derived.method), so existing plugins keep working. If you set use_method_hook_defining_class = True in config (or pass --use-method-hook-defining-class), hooks get the class where the method was defined instead (Base.method).
Fixes #19181
Test plan