Add MethodDefinition#signatures to Ruby API by soutaro · Pull Request #697 · Shopify/rubydex

soutaro · 2026-03-26T05:34:04Z

Summary

Expose method signature information through MethodDefinition#signatures, returning an array of Rubydex::Signature objects
Each Signature holds an array of typed Parameter subclasses (PositionalParameter, KeywordParameter, etc.) with name and location attributes
Add Rust C API (ParameterKind, SignatureEntry, SignatureArray) and C extension glue for conversion

Extracted from #686.

Test plan

Tests cover all 8 parameter kinds (positional, optional, rest, keyword, optional keyword, rest keyword, block, forward)
Tests cover RBS-defined signatures, overloads, untyped parameters, and no-parameter methods

🤖 Generated with Claude Code

ext/rubydex/signature.c

rust/rubydex-sys/src/definition_api.rs

lib/rubydex/signature.rb

- Rename ParameterKind variants to full names (e.g. Req -> RequiredPositional) - Merge two let-else bindings into one in rdx_definition_signatures - Make collect_method_signatures private - Change Signature#initialize from keyword arguments to positional arguments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vinistock · 2026-03-31T15:00:39Z

ext/rubydex/signature.c

+}
+
+VALUE rdxi_signatures_to_ruby(SignatureArray *arr, VALUE graph_obj, VALUE default_method_def) {
+    if (arr == NULL || arr->len == 0) {


How about we raise in rdxr_method_definition_signatures and rdxr_method_alias_definition_signatures if the pointer is NULL?

This should never really happen, so I'm trying to avoid having it fail silently.

Yeah, I think the design is weird. Will take a look at this tonight.

Fixed the implementation. rdx_method_definition_signatures and rdx_method_alias_definition_signatures always return an array. rdxi_signatures_to_ruby can safely skips checking if arr is NULL.

ext/rubydex/signature.c

lib/rubydex/signature.rb

rust/rubydex-sys/src/definition_api.rs

vinistock · 2026-03-31T15:05:44Z

rust/rubydex-sys/src/definition_api.rs

+        let mut boxed = sig_entries.into_boxed_slice();
+        let len = boxed.len();
+        let items_ptr = boxed.as_mut_ptr();
+        std::mem::forget(boxed);


Do we need to forget here if we're ultimately passing this to Box::into_raw?

If we don't use forget, the contents of the Box will be deallocated by the destructor.
We could use Box::into_raw instead, but I don't think it's worth the rewrite since the mem::forget pattern is already used elsewhere in the codebase.

let items_ptr = Box::into_raw(boxed) as *mut SignatureEntry;

rust/rubydex-sys/src/definition_api.rs

rust/rubydex/src/query.rs

vinistock · 2026-04-01T14:54:17Z

ext/rubydex/signature.c

+            ParameterEntry param_entry = sig_entry.parameters[j];
+
+            VALUE param_class = parameter_class_for_kind(param_entry.kind);
+            VALUE name_sym = ID2SYM(rb_intern(param_entry.name));


Does rb_intern make a copy of the string we pass? I'm asking because we free the signature array object further below. If Ruby interning just keeps a pointer, it could lead to a seg fault.

I believe rb_intern copies the string, but I couldn't find any documentation that explicitly says so. Changed to rb_str_intern with rb_utf8_str_new_cstr, which creates dynamic symbols (subject to GC) from a copied C string.

test/definition_test.rb

rust/rubydex-sys/src/signature_api.rs

rust/rubydex/src/query.rs

Morriar · 2026-04-02T14:54:10Z

rust/rubydex/src/test_utils/graph_test.rs

+    ///
+    /// Panics if the definition or its document does not exist.
+    #[must_use]
+    pub fn source_at(&self, definition_id: &DefinitionId) -> &str {


I think we can move this to Offset and pass the document as we do with https://github.com/Shopify/rubydex/blob/main/rust/rubydex/src/offset.rs#L90-L103

st0012 · 2026-04-08T11:00:57Z

rust/rubydex-sys/src/signature_api.rs

+}
+
+/// Returns a newly allocated array of signatures for the given method definition id.
+/// Returns NULL if the definition is not a method definition.


I think it panics in this case?

rust/skills.json

st0012 · 2026-04-08T11:02:19Z

test/definition_test.rb

    end
  end

+  def test_method_definition_signatures_with_various_parameter_kinds


Can we include test cases for singleton methods?

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

soutaro requested a review from a team as a code owner March 26, 2026 05:34

soutaro self-assigned this Mar 26, 2026

soutaro mentioned this pull request Mar 26, 2026

Expose method signature to Ruby #686

Open

vinistock reviewed Mar 26, 2026

View reviewed changes

soutaro commented Mar 27, 2026

View reviewed changes

lib/rubydex/signature.rb Outdated Show resolved Hide resolved

soutaro force-pushed the soutaro-definition-signatures branch from 79877ae to 39084db Compare March 31, 2026 08:58

vinistock reviewed Mar 31, 2026

View reviewed changes

soutaro commented Apr 1, 2026

View reviewed changes

rust/rubydex/src/query.rs Outdated Show resolved Hide resolved

vinistock reviewed Apr 1, 2026

View reviewed changes

Morriar reviewed Apr 2, 2026

View reviewed changes

soutaro force-pushed the soutaro-definition-signatures branch from b33e8c6 to e0b3e8a Compare April 8, 2026 04:48

st0012 reviewed Apr 8, 2026

View reviewed changes

soutaro and others added 17 commits April 14, 2026 06:34

Add Rubydex::Signature class and Rust C API for method signatures

b32b692

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add MethodDefinition#signatures to Ruby API

3203c85

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Implement MethodAliasDefinition#signatures

8379cc7

Find inherited methods

d7681eb

Delete Signature#method_definition

a8f4c85

Rename parameter subclasses

7d9657a

Extract signature_api.rs

555cbed

Refactor rdx_declaration_find_member

d625d55

Fix dealias_method

a2aedb6

Add test for find_member_in_ancestors

4066306

Add test of deep_dealias_method

783402a

👮

d8520b7

lint

3762ce3

Create *dynamic* symbols, which are garbage collected

b73cb99

Drop … comment

f5e6d8e

Add use

cc54708

Get owner_id through declaration

9fb0de4

soutaro added 10 commits April 14, 2026 06:34

Fixup tests

fe2e52d

dedup deep_dealias_method result

9a0fbad

cargo fmt

7f41cb3

Add GraphTest.source_at_offset

e9942f7

Add panics section

c5c9d07

Move source_at to Offset

4d495ef

Add test for singleton classes

fce9732

It panics instead of returning NULL

536102e

delete skills.json

4494f5c

Simpler deep_dealias_method return value

aeeebb3

soutaro force-pushed the soutaro-definition-signatures branch from eef4e19 to aeeebb3 Compare April 13, 2026 21:34

Conversation

soutaro commented Mar 26, 2026

Summary

Test plan

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soutaro Apr 1, 2026 • 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!

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

soutaro Apr 1, 2026 •

edited

Loading