Fixit
Fixit copied to clipboard
Instagram
Supports passing no providers to FullRepoMetadataConfig.
Summary
https://github.com/Instagram/Fixit/issues/181
Allowing passing no providers to FullRepoMetadata. There is already a fallback if the provider is broken where a placeholder cache is used, so I used the same codepath to treat empty providers and exceptions the same way.
There is currently only one placeholder cache type so I hardcoded TypeInferenceProvider, but if we want this to be more flexible I can try to build support for multiple.
Test Plan
Added a unit test. Tested manually and exception no longer fires and linters work correctly.
Seems reasonable; check pyre and is there any other kind of full repo metadata other than TypeInferenceProvider that we might need to support?
I don't know why pyre is failing, I cannot get it to repro locally :(
I did some digging to try to figure out what the plan was with the PLACEHOLDER_CACHES and looks like it exists mostly because there is a bug in TypeInferenceProviders that triggers if cache is empty: https://github.com/Instagram/Fixit/pull/31#discussion_r452948963
I'm going to take a look at fixing that and reconfiguring this to not use placeholders at all if possible.
Pyre failed due to a type error was found in fixit/rules/cls_in_classmethod.py:286:26 and that's related to the new LibCST release. https://github.com/Instagram/Fixit/runs/2271833508 The same issue happens on master branch and it's better to fix it before merging new PRs. FlattenSentinel is added in LibCST 0.3.18 and we need to update the type annotation of report(). https://github.com/Instagram/Fixit/blob/1d1cfe938fbc6f348a7666504982ec8ebd46cc87/fixit/common/base.py#L120
Thanks @jimmylai! No rush, I want to wait on this PR until after https://github.com/Instagram/LibCST/pull/476 is merged and released. I think it will clean up the code here nicely.
@zsol do you know when LibCST will be released again? I'd like to get this fix in :) Happy to help if I can!
There has been a new LibCST release so I am back looking at this. The bug in TypeInferenceProviders has been fixed but there is still a problem here: https://github.com/Instagram/LibCST/blob/e759ca8290585cf3badcd22ac5e1fdbc5e4f9e13/libcst/metadata/base_provider.py#L69
When there is a TimeoutError from using Pyre we trigger this exception that says Cache is required for initializing TypeInferenceProvider.
Hey there! We appreciate your contributions, but we're in the process of making some large changes to the core of Fixit. We will try to have more info about the direction we're heading soon, but in the mean time, we are closing all outstanding PR's from before we started this work. Thank you for your understanding.
