Conversation
Align RBS keyword parameter offsets with Ruby indexer behavior. The RBS indexer now includes the trailing colon in the offset for required and optional keyword parameters (e.g. `name:` instead of `name`). Adds `Offset::extend_end` helper to adjust byte ranges. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Expose method signature information through the Ruby C extension. Each signature contains parameters as [kind, name, location] tuples, following the same format as Method#parameters with an additional location element. - Add Parameter::inner() to access ParameterStruct from any variant - Add Rust C API: ParameterKind, SignatureEntry, SignatureArray structs and rdx_definition_signatures/rdx_definition_signatures_free functions - Add Rubydex::Signature Ruby class with #parameters and #method_definition - Add rdxi_signatures_to_ruby shared C helper for SignatureArray conversion - Register MethodDefinition#signatures in the C extension Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add signatures method to method declarations (Rubydex::Method) that aggregates signatures from all definitions, resolving method aliases to the original method's signatures. - Add query::method_definitions() in Rust for alias-aware lookup - Add rdx_declaration_method_signatures C API function - Register Method#signatures in the C extension Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
soutaro
force-pushed
the
soutaro-signature_ruby_api
branch
from
8a49bfb to
d1f4139
Compare
rust/rubydex/src/query.rs
Outdated
| /// - the declaration is not a method declaration | ||
| /// - any definition or owner declaration referenced by the method is missing from the graph | ||
| #[must_use] | ||
| pub fn method_definitions(graph: &Graph, declaration_id: DeclarationId) -> Vec<(DefinitionId, &MethodDefinition)> { |
There was a problem hiding this comment.
The name may be confusing, what do you think of something like dealias_method_declaration_to_definitions
There was a problem hiding this comment.
Agree that method_definitions is too ambiguous. I feel dealias_ is a bit too implementation-specific though — the intent is "get the underlying MethodDefinitions from a declaration", not specifically about alias resolution.
How about resolve_method_declaration_to_definitions, or without the resolve_ prefix?
There was a problem hiding this comment.
I like resolve_method_declaration_to_definitions
Actually, we generally use resolve to go from a name to a declaration, it may be confusing to "resolve a declaration".
Let's go with method_declaration_to_definitions
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
vinistock
left a comment
vinistock
left a comment
There was a problem hiding this comment.
This PR will likely be quicker to ship if we split it into smaller steps.
For example, the creation of the Signature class and all parameters can probably be shipped on its own
| } | ||
|
|
||
| // Method#signatures -> [Rubydex::Signature] | ||
| static VALUE rdxr_method_declaration_signatures(VALUE self) { |
There was a problem hiding this comment.
Do we need to expose Declaration#signature? Signatures are associated to the specific Definition that creates them, so I'd expect us to access it like this:
method_decl = graph["Foo#bar()"]
signatures = method_decl.definitions.map(&:signature)I think we can remove this one.
There was a problem hiding this comment.
I'd like to keep #signatures as a utility API. #definitions can contain method aliases, so definitions.flat_map(&:signatures) wouldn't work transparently — users would need to handle alias resolution themselves.
We could add a separate API for alias normalization, but if we're adding an API anyway, I think #signatures that does it all in one shot is the better choice.
| # Each parameter is a 3-element array of `[kind, name, location]`, | ||
| # following the same format as `Method#parameters` with an additional location element. | ||
| # | ||
| # The kind symbol is one of: | ||
| # - `:req` — required positional parameter (`a`) or post-rest positional parameter (`d` in `def foo(*c, d)`) | ||
| # - `:opt` — optional positional parameter (`b = 1`) | ||
| # - `:rest` — rest positional parameter (`*c`) | ||
| # - `:keyreq` — required keyword parameter (`e:`) | ||
| # - `:key` — optional keyword parameter (`f: 1`) | ||
| # - `:keyrest` — rest keyword parameter (`**g`) | ||
| # - `:block` — block parameter (`&h`) | ||
| # - `:forward` — forward parameter (`...`), Rubydex-specific (Ruby expands this to `:rest`, `:keyrest`, `:block`) | ||
| # | ||
| #: Array[[Symbol, Symbol, Location]] |
There was a problem hiding this comment.
Let's please create Ruby objects to represent each parameter type instead of using symbols.
I suspect you only need a parent class and then a bunch of empty definitions:
class Parameter
def initialize(name, location)
# ...
end
end
class PositionalParameter < Parameter; end
class OptionalPositionalParameter < Parameter; end
class KeywordParameter < Parameter; end
class OptionalKeywordParameter < Parameter; end
# ...
|
@vinistock Yeah, splitting sounds good to me. (Once I started working on this, everything came together as a single PR.) I'll extract the |
3 participants
Towards #668
(?) ->returns empty parameters, which should be fixed later.