Skip to content

Clear last_sigs when returning early on abstract #755

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 9, 2025
Merged

Clear last_sigs when returning early on abstract #755

merged 2 commits into from
Jun 9, 2025

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Jun 9, 2025

Before this fix, when encountering a signature with abstract we were not clearing the list, meaning that subsequent signatures were not translated:

sig { abstract.void }
def foo; end

sig { void } # list still contains the abstract one and will return early
def bar; end

After this change, the second signature is properly translated:

sig { abstract.void }
def foo; end

#: -> void
def bar; end

@Morriar Morriar requested a review from a team as a code owner June 9, 2025 19:29
@Morriar Morriar self-assigned this Jun 9, 2025
@Morriar Morriar added the bugfix Fix a bug label Jun 9, 2025
jesse-shopify
jesse-shopify approved these changes Jun 9, 2025
View reviewed changes
Copy link
Contributor

@jesse-shopify jesse-shopify left a comment

Choose a reason for hiding this comment

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

I like the change from instance variable to local variable for this. This is easier to reason about.

KaanOzkan
KaanOzkan approved these changes Jun 9, 2025
View reviewed changes
Morriar added 2 commits June 9, 2025 15:53
@Morriar
Clear last_sigs when returning early on abstract
1e3c3b2 
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar
Update exported RBI
a04c7d4 
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar force-pushed the at-fix-abstract branch from 7e2da32 to a04c7d4 Compare June 9, 2025 19:54
@Morriar Morriar merged commit 0c1edb8 into main Jun 9, 2025
8 checks passed
@Morriar Morriar deleted the at-fix-abstract branch June 9, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaanOzkan KaanOzkan KaanOzkan approved these changes

@jesse-shopify jesse-shopify jesse-shopify approved these changes

Assignees

@Morriar Morriar

Labels
bugfix Fix a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Morriar @KaanOzkan @jesse-shopify