Skip to content

Adopt params and sig from other when merging a Method with no sig#585

Merged
KaanOzkan merged 1 commit intomainfrom
ko-fix-sig-param-mismatch
Apr 10, 2026
Merged

Adopt params and sig from other when merging a Method with no sig#585
KaanOzkan merged 1 commit intomainfrom
ko-fix-sig-param-mismatch

Conversation

@KaanOzkan
Copy link
Copy Markdown
Contributor

Resolves Shopify/tapioca#2589

When merging two method definitions where the left has no sig but the right does, adopt the right's param names alongside its sigs. Previously only the sigs were adopted, causing a mismatch between the method's param names and the sig's param names.

@KaanOzkan KaanOzkan requested a review from a team as a code owner April 9, 2026 17:57
assert_equal(<<~RBI, res.string)
class Foo
sig { params(_expand: T.nilable(T::Array[String])).returns(T.nilable(T::Array[String])) }
def expand=(_expand); end
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before this change we'd have def expand=(value); end instead.

Copy link
Copy Markdown
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.

Thanks for the fix, and sorry I missed this in the first place.

@KaanOzkan KaanOzkan force-pushed the ko-fix-sig-param-mismatch branch from db3379b to d9e7b81 Compare April 10, 2026 13:46
@KaanOzkan KaanOzkan force-pushed the ko-fix-sig-param-mismatch branch from d9e7b81 to b5c95b3 Compare April 10, 2026 13:49
@KaanOzkan KaanOzkan merged commit dfecb86 into main Apr 10, 2026
11 checks passed
@KaanOzkan KaanOzkan deleted the ko-fix-sig-param-mismatch branch April 10, 2026 13:58
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 this pull request may close these issues.

Tapioca v0.18.0 generates invalid gem RBI for stripe gem

3 participants