Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better mixin generation for gem RBIs #890

Closed
egiurleo opened this issue Apr 8, 2022 · 0 comments · Fixed by #1012
Closed

Better mixin generation for gem RBIs #890

egiurleo opened this issue Apr 8, 2022 · 0 comments · Fixed by #1012
Assignees

Comments

@egiurleo
Copy link
Contributor

egiurleo commented Apr 8, 2022

This issue encompasses multiple problems:

  1. 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.

An initial attempt was made in #569 where mixin tracking was introduced with the intention of storing the backtraces of calls to mix in modules. However, this resulted in some RBIs missing critical mixins because it would overmatch mixins to external gems, thus filtering them out and not generating RBIs for them.

  1. Another problem is that mixins can be added dynamically (e.g. Foo.prepend(Bar)):
  • Imagine Foo is defined in a gem called foo
  • Bar is defined in a gem called bar
  • In the bar gem, the line Foo.prepend(Bar) is executed.
  • When generating RBIs for the bar gem, tapioca will not be aware of Bar being prepended to Foo because it is not generating RBIs for Foo, which is defined in a different gem.

This is especially true if Foo is a standard library constant. Tapioca only ever generates RBIs for gems that are in the Gemfile, and the standard library does not need to be explicitly added to the gemfile.

There was an attempt to address the second issue in the uk-eg-mixin-fix branch, but it was never finalized. We have since reverted the use of the MixinTracker (#668) and refactored a lot, making the changes on this branch incompatible with what is currently on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant