Skip to content

Include type_parameters declarations in signatures#2550

Merged
amomchilov merged 1 commit intomainfrom
jb-current-attributes-compiler
Mar 26, 2026
Merged

Include type_parameters declarations in signatures#2550
amomchilov merged 1 commit intomainfrom
jb-current-attributes-compiler

Conversation

@dejmedus
Copy link
Copy Markdown
Contributor

@dejmedus dejmedus commented Mar 24, 2026

Motivation

Previously, when a compiler used create_method_from_def (ex. ActiveSupportCurrentAttributes) any method signatures would be generated without type_parameters declarations. This created invalid signatures when type params were used in the method sig

- sig { params(a: T.type_parameter(:U)).returns(T.type_parameter(:U)) }
+ sig { type_parameters(:U).params(a: T.type_parameter(:U)).returns(T.type_parameter(:U)) }

Implementation

This PR moves out TYPE_PARAMETER_MATCHER and the type param extraction logic from gem sorbet_signatures.rb file to rbi_helper.rb so that they can also be used by dsl. This way create_method_from_def can pass any type params to create_method so that RBI::Sig.new will be able to include them in the generated signature

Tests

  • in compiler_spec.rb compiles a class with methods that have signatures:
    • updated a case to add type_parameters declaration
    • added new case to test more complex type param use
  • added a method with type params to active_support_current_attributes_spec.rb copies types from instance methods to class methods

Previously, when a compiler `used create_method_from_def`
the generated signature did not include type parameters
declarations. This created invalid signatures when
type params were used in the method sig

This commit moves out type param scanning logic from
gem signature listener to rbi helpers and uses it to
pass type params declarations through to `create_method_from_def`
and `create_method` so that `RBI::Sig.new` will know
about them and include them in the generated signature
@dejmedus dejmedus marked this pull request as ready for review March 24, 2026 20:53
@dejmedus dejmedus requested a review from a team as a code owner March 24, 2026 20:53
@amomchilov amomchilov merged commit 333df08 into main Mar 26, 2026
19 of 20 checks passed
@amomchilov amomchilov deleted the jb-current-attributes-compiler branch March 26, 2026 23:37
Comment on lines +100 to +102
def extract_type_parameters(type_strings)
type_strings.join(", ").scan(TYPE_PARAMETER_MATCHER).flatten.uniq
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd missed this PR earlier, so sorry for the delayed review, but it seems like this functionality should be a part of the inner working of create_method or RBI::Sig construction, since all the inputs to this method seem to always be params and return_type.

I suggest:

  1. Reverting the signature of create_method to remove the type_params parameter
  2. Adding a public method to RBI::Sig for doing this calculation and for setting type_params. Something like set_type_params_from_params. Btw, we don't need to pass return_type to that calculation since there is no way that we can have a type param only in the out position, that means the signature is broken in the first place.
  3. Making create_method method call this method on the created sig after constructing it.

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.

Just saw you beat me to this, thank you! (+ thank you for the detailed feedback)

Btw, we don't need to pass return_type to that calculation since there is no way that we can have a type param only in the out position, that means the signature is broken in the first place.

Ah, that makes sense

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still left the return type in, just in case, so that we don't end up generating broken signatures, even if the signature itself would be meaningless like that.

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.

Generated RBI signatures missing type_parameters declaration

4 participants