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

Stop regenerating all value constants for reopened types #1385

Merged
merged 2 commits into from
Feb 9, 2023

Conversation

paracycle
Copy link
Member

@paracycle paracycle commented Feb 7, 2023

Motivation

Fixes #1326

Implementation

When we are generating value constants (i.e. constants that have a value that is not a module), we didn't have a good way to filter them based on where they were defined, since we couldn't ask them before.

However, since Ruby 2.7, we have access to const_source_location so we can check to see if a value constant has been defined by the current gem or not. So the implementation uses that.

Tests

Updated existing tests to test for this specific case where the bar gem is reopening a constant defined by the foo gem and adds a value constant under it. The expectation is that only the subconstant added in bar gem is generated in bar.rbi and none of the other subconstants defined in foo.

I also tested this against Core and it works without problems (and gets rid of a lot of unnecessary constant definitions on gem RBI files).

@paracycle paracycle requested a review from a team as a code owner February 7, 2023 21:48
@paracycle paracycle force-pushed the uk-const-source-location branch 2 times, most recently from b16c56f to c8d8f31 Compare February 7, 2023 22:22
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.

Nice!

@paracycle paracycle merged commit eac27ac into main Feb 9, 2023
@paracycle paracycle deleted the uk-const-source-location branch February 9, 2023 19:35
@shopify-shipit shopify-shipit bot temporarily deployed to production February 21, 2023 12:01 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better subconstant tracking when a gem reopens an external constant
2 participants