Skip to content

Index methods#633

Draft
soutaro wants to merge 2 commits intomainfrom
index-methods
Draft

Index methods#633
soutaro wants to merge 2 commits intomainfrom
index-methods

Conversation

@soutaro
Copy link
Contributor

@soutaro soutaro commented Mar 4, 2026

  • Index method
  • Index method parameters
  • Add test to confirm resolution works

@soutaro soutaro self-assigned this Mar 4, 2026
@soutaro soutaro added the enhancement New feature or request label Mar 4, 2026
let line_text = if i == 0 {
line.to_string()
} else {
line[indent_len..].to_string()
Copy link
Contributor

Choose a reason for hiding this comment

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

This can panic if a subsequent comment line has fewer leading spaces than indent_len. For example, if the first line has 4 spaces of indentation but a later line accidentally starts at column 0, line[4..] would panic with an out-of-bounds string slice.

Might be safer to use line.get(indent_len..).unwrap_or(line) here:

Suggested change
line[indent_len..].to_string()
line.get(indent_len..).unwrap_or(line).to_string()

}

fn visit_method_definition_node(&mut self, def_node: &node::MethodDefinitionNode) {
// Singleton and singleton_instance methods are not indexed for now.
Copy link
Member

Choose a reason for hiding this comment

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

We should have the mechanisms for indexing singleton methods. Not sure what singleton_instance method means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve been working on instance method support because it's easier to finish.

I’ll work on singleton method support once I finish indexing instance methods.
singleton_instance is for module_function. (Yeah, that makes things even more complicated. 😄)

comments,
flags,
lexical_nesting_id,
Vec::new(),
Copy link
Member

Choose a reason for hiding this comment

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

I was reflecting on this for the parameters. I think we have a few options, but my suggestion is that we make parameters in MethodDefinition an enum.

enum MethodParameters {
  Simple(Signature),
  Overloaded(Vec<Signature>),
}

// where Signature just holds the vector of parameters for convenience

The reason I feel like this is the best option is because a lot of methods in RBS will not have overloads. If we were to create a separate RBSMethodDefinition that always accepted a vector of signatures, we would pay the price of allocating vectors for a bunch of RBS methods that don't have overloads.

I believe we'll reduce our memory usage and keep the implementation simple by simply allowing method defs to accept a single signature or overloads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants