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

Do not mark constants as seen before generating RBI for them #1040

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Jul 6, 2022

Motivation

Fix foreign constants mixin for already seen ancestors

Consider the following example when running tapioca gem gemB:

# gemA code
class Bar; end
# gemB code
module Foo; end
class Baz < Bar; end
Bar.prepend Foo

While visiting the constants of GemB we would:

  • visit constant Baz
  • visit the superclass constant Bar
  • mark Bar as seen
  • skip Bar since it belongs to GemA
  • visit the foreign constant Bar for the prepend
  • skip Bar because previously marked as seen

Implementation

Only mark a constant as seen if we generate something for it.

Tests

See included test.

Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar added bug Something isn't working bugfix labels Jul 6, 2022
@Morriar Morriar self-assigned this Jul 6, 2022
@Morriar Morriar requested a review from a team July 6, 2022 16:51
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thank you

@Morriar Morriar merged commit 062e409 into main Jul 6, 2022
@Morriar Morriar deleted the at-fix-foreign-mixins branch July 6, 2022 17:42
Copy link
Contributor

@egiurleo egiurleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏🏻

@shopify-shipit shopify-shipit bot temporarily deployed to production July 7, 2022 17:53 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-9-stable August 19, 2022 20:37 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants