Add support for lower bounds in type parameters #45

allcre · 2025-05-13T15:59:43Z

Ticket: https://github.com/Shopify/team-ruby-dx/issues/1453
This change implements lower bound constraints for generic type parameters in RBS, allowing declarations like [T > SomeType].

The implementation includes:

Parser and lexer modifications to handle ">" token in type params
Relevant changes to the Ruby API, like Locator and Validator
Schema updates to typeParam.json
Documentation updates

To review

My approach was to find how upper_bound is handled and duplicate the logic for lower_bound. Did I miss anything?
Any documentation updates missing?
Any missing tests?

note: CI is currently failing but this will be fixed by ruby#2482

allcre · 2025-05-13T16:11:08Z

src/parser.c

-        upper_bound_range.end = parser->current_token.range.end;
+      rbs_range_t lower_bound_range = NULL_RANGE;
+
+      for(int bound_parse_attempt = 0; bound_parse_attempt < 2; bound_parse_attempt++) {


This loop is to parse both upper bound (<) and lower bound (>) constraints for type parameters. The loop runs at most twice to allow both bound types in either order: [T < UpperBound > LowerBound] or [T > LowerBound < UpperBound]. It detects and prevents duplicate bounds of the same type and exits early if no bounds are present.

allcre · 2025-05-13T16:30:53Z

src/parser.c

+      } else if (parser->next_token.type == pRBRACKET) {
        break;
+      } else {
+        rbs_parser_set_error(parser, parser->next_token, true, "expected ',' or ']' after type parameter, got %s", rbs_token_type_str(parser->next_token.type));
+        return false;


This change ensures the parser correctly validates syntax after type parameter bounds. Previously, expressions like [T < String Integer] would be silently accepted despite being invalid.

amomchilov

This is a really impressive piece of work!

amomchilov · 2025-05-13T22:49:49Z

docs/syntax.md

+You can also specify the _lower bound_ of the type parameter using `>`. This constrains the type parameter to be a supertype of the specified bound. Both an upper and a lower bound can be specified for the same type parameter.
+
+```rbs
+class FlexibleProcessor[T > Integer < Numeric]


Hmmmm I do not like this syntax... Integer < T < Numeric would be much better. 🤔 Let's discuss with @Morriar

Agreed that would be nicer. If only a lower bound was specified, would we want to accept Integer < T and not T > Integer?

While it works well with one letter names mixed with core types, it can become more confusing when using semantic names:

SUV < Car < Vehicle

Which one is the type param?

Not that the original syntax is much better though...

Car > SUV < Vehicle

I'm not against the proposed Integer < T < Numeric but it's still confusing when using a default type:

Integer < T < Numeric = Integer

In practice, lower bounds are not as used as upper bound and using both would be a really rare occasion. So I'm not sure any syntax matters much?

We can ask Soutaro once we open the PR against ruby/rbs

lib/rbs/validator.rb

src/parser.c

docs/syntax.md

sig/type_param.rbs

Morriar · 2025-05-14T15:55:05Z

src/parser.c

-        upper_bound_range.end = parser->current_token.range.end;
+      rbs_range_t lower_bound_range = NULL_RANGE;
+
+      for (int bound_parse_attempt = 0; bound_parse_attempt < 2; bound_parse_attempt++) {


st0012 · 2025-05-14T22:18:41Z

lib/rbs/cli/validate.rb

+              void_type_context_validator(lb)
+              no_self_type_validator(lb)
+              no_classish_type_validator(lb)
+              @validator.validate_type(lb, context: nil)


Not directly related to this PR, but it feels like these 4 lines can be extracted into a helper method as it's repeated a lot 🤔

st0012 · 2025-05-14T22:25:27Z

lib/rbs/validator.rb

+          [param.upper_bound_type, param.lower_bound_type].compact.each do |bound|
+            bound.free_variables.each do |tv|
              block[tv]
            end


These are apparently not the same! With the suggested change, tests start failing, but I'm not exactly sure why.

Morriar · 2025-05-15T13:33:59Z

For the CI errors I think you need to bundle exec rake lexer and commits the changes

st0012 · 2025-05-15T13:44:39Z

For the CI errors I think you need to bundle exec rake lexer and commits the changes

I think it's caused by mismatches on the r2e versions. RBS currently uses v3.1 but we usually install 4.* now.

@allcre you can follow the same commands used on CI to install that specific version:

          cd /tmp
          curl -L https://github.com/skvadrik/re2c/archive/refs/tags/3.1.tar.gz > re2c-3.1.tar.gz
          tar xf re2c-3.1.tar.gz
          cd re2c-3.1
          autoreconf -i -W all
          ./configure
          make
          sudo make install

We can look at this together in our pairing later.

This change implements lower bound constraints for generic type parameters in RBS, allowing declarations like `[T > SomeType]`. The implementation includes: - Parser and lexer modifications to handle ">" token in type params - Relevant changes to the Ruby API, like Locator and Validator - Schema updates to typeParam.json - Documentation updates

allcre commented May 13, 2025

View reviewed changes

allcre requested review from Morriar, amomchilov and st0012 May 13, 2025 17:00

amomchilov reviewed May 13, 2025

View reviewed changes

allcre force-pushed the support-lower-bound branch from f5e12ef to 1a2750a Compare May 14, 2025 14:51

allcre requested a review from amomchilov May 14, 2025 14:56

allcre force-pushed the support-lower-bound branch from 1a2750a to 8abac1e Compare May 14, 2025 15:22

Morriar reviewed May 14, 2025

View reviewed changes

allcre force-pushed the support-lower-bound branch 2 times, most recently from d047a16 to 2e4b7bc Compare May 14, 2025 17:04

allcre requested a review from Morriar May 14, 2025 17:21

st0012 approved these changes May 14, 2025

View reviewed changes

Morriar approved these changes May 15, 2025

View reviewed changes

allcre force-pushed the support-lower-bound branch 4 times, most recently from 487f79c to c63e115 Compare May 15, 2025 16:32

allcre force-pushed the support-lower-bound branch from c63e115 to ac8348a Compare May 20, 2025 16:03

alexcrocha merged commit 2a73da6 into master Jul 8, 2025
22 of 24 checks passed

Add support for lower bounds in type parameters #45

Add support for lower bounds in type parameters #45

Uh oh!

Conversation

allcre commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amomchilov left a comment

Choose a reason for hiding this comment

Uh oh!

amomchilov May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Morriar commented May 15, 2025

Uh oh!

st0012 commented May 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

allcre commented May 13, 2025 •

edited

Loading

amomchilov May 13, 2025 •

edited

Loading