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 print type parameters as TYPE_ when translating to RBS #396

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Jan 14, 2025

We actually don't need to translate these types with the TYPE_ prefix to differentiate from actual types.

Since all type parameters need to be declared in the signature preamble, we can easily figure which types are parameters and which ones are actual types.

@Morriar Morriar added the bugfix Fix a bug label Jan 14, 2025
@Morriar Morriar self-assigned this Jan 14, 2025
@Morriar Morriar requested a review from a team as a code owner January 14, 2025 21:26
@@ -385,7 +385,7 @@ def foo(a); end

# To avoid conflict with existing constants, we prefix type parameters with `TYPE_`
assert_equal(<<~RBI, rbi.rbs_string)
def foo: [TYPE_U, TYPE_V] (TYPE_U a) -> TYPE_V
def foo: [U, V] (U a) -> V
Copy link
Member

Choose a reason for hiding this comment

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

We need to remove the comment above this assertion

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, fixed 👍

Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar force-pushed the at-rbs-method-type-params branch from c239e18 to 9216a9d Compare January 15, 2025 15:09
@Morriar Morriar merged commit 0297249 into main Jan 15, 2025
8 checks passed
@Morriar Morriar deleted the at-rbs-method-type-params branch January 15, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants