fix: key inherited metadata dependency cache per class - #1466
Conversation
MetadataDependent.get_inherited_dependencies() memoized into a plain class attribute and read it back with ordinary attribute lookup. That lookup walks the MRO, so resolving a bare CSTTransformer/CSTVisitor cached an empty frozenset onto the base class, and every subclass that had not yet computed its own cache inherited that empty set - raising METADATA_DEPENDENCIES KeyErrors despite declaring the provider. Read the cache from cls.__dict__ instead, so each class only ever sees its own memoized value. Adds a regression test that resolves a bare CSTTransformer first and then asserts the subclass still reports its declared dependency.
|
Hi @Sanjays2402! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
meta-cla
Bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
1 participant
Closes #1459
Summary
MetadataDependent.get_inherited_dependencies()memoized into a plain class attribute and read it back with ordinary attribute lookup. That lookup walks the MRO, so resolving a bareCSTTransformer/CSTVisitorcached an emptyfrozensetonto the base class, and every subclass that had not yet computed its own cache inherited it — raisingMETADATA_DEPENDENCIESKeyErrors even though the subclass declares the provider correctly. Reading the cache fromcls.__dict__instead means each class only ever sees its own memoized value.Test Plan
Added
test_bare_visitor_does_not_poison_subclass_dependenciestolibcst/metadata/tests/test_metadata_provider.py, which resolves a bareCSTTransformerfirst and then asserts the subclass still reports its declared dependency. It fails on the unpatched source (frozenset() != {ProviderA}) and passes with the fix;libcst/metadata/tests/,test_visitor.pyandtest_batched_visitor.pyare green (178 passed, only the pre-existing pyre-binary errors intest_type_inference_provider.py).This change was prepared with AI assistance; the regression test was run locally and fails without the fix.