Better mixin generation for gem RBIs - #569
Merged
Merged
Conversation
Mixin tracker tracks all `prepend`, `include` and `extend` calls and keeps a map of modules and the locations where they were mixed into each constant. Co-authored-by: Alexandre Terrasa <[email protected]>
The logic was duplicated for `prepends`, `includes` and `extends`, so we factored out the common logic to a method. Co-authored-by: Alexandre Terrasa <[email protected]>
We should generate a mixin in the RBI file only if we can be sure that the mixin location was coming from a gem file or if we could not track the mixin location at all. In all other cases, we can safely drop the mixin, since it should be generated in the RBI file of the gem that is actually performing the mixin in the first place. Co-authored-by: Alexandre Terrasa <[email protected]>
Stop expecting that unrelated mixins should be generated on the core types we have to reopen them in RBI files. Co-authored-by: Alexandre Terrasa <[email protected]>
Co-authored-by: Alexandre Terrasa <[email protected]>
Not sure if this is going to work properly Co-authored-by: Alexandre Terrasa <[email protected]>
Since some code checks or relies on the result of `callers` or `caller_locations` during a mixin, if we patch mixin methods, we might get spurious failures. A safer way is to patch `append_features` to catch `include`s, `prepend_features` to catch `prepend`s and `extend_object`s to catch `extend`s. Co-authored-by: Alan Wu <[email protected]>
Morriar
approved these changes
Oct 27, 2021
vinistock
approved these changes
Oct 28, 2021
Morriar
approved these changes
Oct 28, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
We have not had a way of filtering mixins on a constant to generate only the ones that are actually mixed in by the gem we are generating an RBI for. This has been causing issues of attribution and was resulting in Tapioca generating RBIs that would look different in the presence of other gems.
Implementation
The implementation relies on detecting when mixins happen on constants and recording the unique file based backtrace of the mixin location. At RBI mixin generation time, we filter out all mixins on a constant that have no backtrace that has ever touched the gem we are working on.
Tests
Updated existing tests to work well with the new reality.
Co-authored-by: Alexandre Terrasa [email protected]