Skip to content

Introduce Receiver enum to distinguish self vs constant method receivers #573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
st0012 merged 1 commit into from
Feb 26, 2026
Merged

Introduce Receiver enum to distinguish self vs constant method receivers #573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 51 additions & 28 deletions rust/rubydex/src/indexing/ruby_indexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::model::definitions::{
ConstantAliasDefinition, ConstantDefinition, Definition, DefinitionFlags, ExtendDefinition,
GlobalVariableAliasDefinition, GlobalVariableDefinition, IncludeDefinition, InstanceVariableDefinition,
MethodAliasDefinition, MethodDefinition, Mixin, ModuleDefinition, Parameter, ParameterStruct, PrependDefinition,
SingletonClassDefinition,
Receiver, SingletonClassDefinition,
};
use crate::model::document::Document;
use crate::model::ids::{DefinitionId, NameId, StringId, UriId};
Expand Down Expand Up @@ -1042,9 +1042,17 @@ impl<'a> RubyIndexer<'a> {
unreachable!("method definition for nesting should exist")
};

if let Some(method_def_receiver) = definition.receiver() {
if let Some(receiver) = definition.receiver() {
is_singleton_name = true;
Some(*method_def_receiver)
match receiver {
Receiver::SelfReceiver(def_id) => self
.local_graph
.definitions()
.get(def_id)
.and_then(Definition::name_id)
.copied(),
Receiver::ConstantReceiver(name_id) => Some(*name_id),
}
} else {
self.current_owner_name_id()
}
Expand Down Expand Up @@ -1416,12 +1424,12 @@ impl Visit<'_> for RubyIndexer<'_> {

let receiver = if let Some(recv_node) = node.receiver() {
match recv_node {
// def self.foo - receiver is the current owner's NameId (includes Class.new/Module.new)
ruby_prism::Node::SelfNode { .. } => self.current_owner_name_id(),
// def self.foo - receiver is the enclosing definition's DefinitionId
ruby_prism::Node::SelfNode { .. } => self.current_nesting_definition_id().map(Receiver::SelfReceiver),
// def Foo.bar or def Foo::Bar.baz - receiver is the constant's NameId
ruby_prism::Node::ConstantPathNode { .. } | ruby_prism::Node::ConstantReadNode { .. } => {
self.index_constant_reference(&recv_node, true)
}
ruby_prism::Node::ConstantPathNode { .. } | ruby_prism::Node::ConstantReadNode { .. } => self
.index_constant_reference(&recv_node, true)
.map(Receiver::ConstantReceiver),
// Dynamic receiver (def foo.bar) - visit and then skip
// We still want to visit because it could be a variable reference
_ => {
Expand Down Expand Up @@ -1451,7 +1459,7 @@ impl Visit<'_> for RubyIndexer<'_> {
parent_nesting_id,
parameters.clone(),
Visibility::Public,
self.current_owner_name_id(),
self.current_nesting_definition_id().map(Receiver::SelfReceiver),
)));
let definition_id = self.local_graph.add_definition(method);

Expand Down Expand Up @@ -2007,7 +2015,7 @@ mod tests {
assert_def_name_eq, assert_def_name_offset_eq, assert_def_str_eq, assert_definition_at,
assert_local_diagnostics_eq, assert_name_path_eq, assert_no_local_diagnostics, assert_string_eq,
model::{
definitions::{Definition, Mixin, Parameter},
definitions::{Definition, Mixin, Parameter, Receiver},
ids::{StringId, UriId},
visibility::Visibility,
},
Expand Down Expand Up @@ -2114,20 +2122,27 @@ mod tests {
/// - `assert_method_has_receiver!(ctx, method, "<Bar>")`
macro_rules! assert_method_has_receiver {
($context:expr, $method:expr, $expected_receiver:expr) => {{
if let Some(receiver_name_id) = $method.receiver() {
let name = $context.graph().names().get(receiver_name_id).unwrap();
let actual_name = $context.graph().strings().get(name.str()).unwrap().as_str();
assert_eq!(
$expected_receiver, actual_name,
"method receiver mismatch: expected `{}`, got `{}`",
$expected_receiver, actual_name
);
} else {
panic!(
"Method receiver mismatch: expected `{}`, got `None`",
$expected_receiver
);
}
let name_id = match $method.receiver() {
Some(Receiver::SelfReceiver(def_id)) => {
let def = $context.graph().definitions().get(def_id).unwrap();
*def.name_id().expect("SelfReceiver definition should have a name_id")
}
Some(Receiver::ConstantReceiver(name_id)) => *name_id,
None => {
panic!(
"Method receiver mismatch: expected `{}`, got `None`",
$expected_receiver
);
}
};

let name = $context.graph().names().get(&name_id).unwrap();
let actual_name = $context.graph().strings().get(name.str()).unwrap().as_str();
assert_eq!(
$expected_receiver, actual_name,
"method receiver mismatch: expected `{}`, got `{}`",
$expected_receiver, actual_name
);
}};
}

Expand Down Expand Up @@ -5429,8 +5444,12 @@ mod tests {
assert_no_local_diagnostics!(&context);

assert_definition_at!(&context, "3:5-4:8", Method, |bar| {
let receiver = bar.receiver().unwrap();
let name_ref = context.graph().names().get(&receiver).unwrap();
let Receiver::SelfReceiver(def_id) = bar.receiver().as_ref().unwrap() else {
panic!("Expected SelfReceiver for def self.bar in Class.new");
};
let def = context.graph().definitions().get(def_id).unwrap();
let name_id = def.name_id().expect("Owner definition should have a name_id");
let name_ref = context.graph().names().get(name_id).unwrap();
assert_eq!(StringId::from("A"), *name_ref.str());

let nesting_name = context.graph().names().get(&name_ref.nesting().unwrap()).unwrap();
Expand Down Expand Up @@ -5475,8 +5494,12 @@ mod tests {
assert_no_local_diagnostics!(&context);

assert_definition_at!(&context, "3:5-4:8", Method, |bar| {
let receiver = bar.receiver().unwrap();
let name_ref = context.graph().names().get(&receiver).unwrap();
let Receiver::SelfReceiver(def_id) = bar.receiver().as_ref().unwrap() else {
panic!("Expected SelfReceiver for def self.bar in anonymous Class.new");
};
let def = context.graph().definitions().get(def_id).unwrap();
let name_id = def.name_id().expect("Owner definition should have a name_id");
let name_ref = context.graph().names().get(name_id).unwrap();
let uri_id = UriId::from("file:///foo.rb");
assert_eq!(StringId::from(&format!("{uri_id}:13<anonymous>")), *name_ref.str());
assert!(name_ref.nesting().is_none());
Expand Down
27 changes: 22 additions & 5 deletions rust/rubydex/src/model/definitions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,10 +712,22 @@ pub struct MethodDefinition {
lexical_nesting_id: Option<DefinitionId>,
parameters: Vec<Parameter>,
visibility: Visibility,
receiver: Option<NameId>,
receiver: Option<Receiver>,
}

assert_mem_size!(MethodDefinition, 112);

/// The receiver of a singleton method definition.
#[derive(Debug)]
pub enum Receiver {
/// `def self.foo` - receiver is the enclosing definition (class, module, singleton class or DSL)
SelfReceiver(DefinitionId),
/// `def Foo.bar` - receiver is an explicit constant that needs resolution
ConstantReceiver(NameId),
}

assert_mem_size!(Receiver, 16);

impl MethodDefinition {
#[allow(clippy::too_many_arguments)]
#[must_use]
Expand All @@ -728,7 +740,7 @@ impl MethodDefinition {
lexical_nesting_id: Option<DefinitionId>,
parameters: Vec<Parameter>,
visibility: Visibility,
receiver: Option<NameId>,
receiver: Option<Receiver>,
) -> Self {
Self {
str_id,
Expand All @@ -747,8 +759,13 @@ impl MethodDefinition {
pub fn id(&self) -> DefinitionId {
let mut formatted_id = format!("{}{}{}", *self.uri_id, self.offset.start(), *self.str_id);

if let Some(receiver) = self.receiver {
formatted_id.push_str(&receiver.to_string());
if let Some(receiver) = &self.receiver {
match receiver {
Receiver::SelfReceiver(def_id) => formatted_id.push_str(&def_id.to_string()),
Receiver::ConstantReceiver(name_id) => {
formatted_id.push_str(&name_id.to_string());
}
}
}

DefinitionId::from(&formatted_id)
Expand Down Expand Up @@ -785,7 +802,7 @@ impl MethodDefinition {
}

#[must_use]
pub fn receiver(&self) -> &Option<NameId> {
pub fn receiver(&self) -> &Option<Receiver> {
&self.receiver
}

Expand Down
70 changes: 38 additions & 32 deletions rust/rubydex/src/resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::model::{
Declaration, GlobalVariableDeclaration, InstanceVariableDeclaration, MethodDeclaration, ModuleDeclaration,
Namespace, SingletonClassDeclaration,
},
definitions::{Definition, Mixin},
definitions::{Definition, Mixin, Receiver},
graph::{CLASS_ID, Graph, MODULE_ID, OBJECT_ID},
identity_maps::{IdentityHashMap, IdentityHashSet},
ids::{DeclarationId, DefinitionId, NameId, ReferenceId, StringId},
Expand Down Expand Up @@ -272,23 +272,26 @@ impl<'a> Resolver<'a> {
match self.graph.definitions().get(&id).unwrap() {
Definition::Method(method_definition) => {
let str_id = *method_definition.str_id();
let owner_id = if let Some(receiver) = method_definition.receiver() {
let receiver_decl_id = match self.graph.names().get(receiver).unwrap() {
NameRef::Resolved(resolved) => *resolved.declaration_id(),
NameRef::Unresolved(_) => {
// Error diagnostic: if we couldn't resolve the constant being used, then we don't know
// where this method is being defined. For example:
//
// def Foo.bar; end
//
// where Foo is undefined
let owner_id = match method_definition.receiver() {
Some(Receiver::SelfReceiver(def_id)) => {
let Some(&owner_decl_id) = self.graph.definition_id_to_declaration_id(*def_id) else {
// The enclosing class/module couldn't be resolved (e.g., `class Foo::Bar` where
// `Foo` is undefined). The method is orphaned as a consequence.
continue;
}
};
};
self.get_or_create_singleton_class(owner_decl_id)
}
Some(Receiver::ConstantReceiver(name_id)) => {
let receiver_decl_id = match self.graph.names().get(name_id).unwrap() {
NameRef::Resolved(resolved) => *resolved.declaration_id(),
NameRef::Unresolved(_) => {
continue;
}
};

self.get_or_create_singleton_class(receiver_decl_id)
} else {
self.resolve_lexical_owner(*method_definition.lexical_nesting_id())
self.get_or_create_singleton_class(receiver_decl_id)
}
None => self.resolve_lexical_owner(*method_definition.lexical_nesting_id()),
};

self.create_declaration(str_id, id, owner_id, |name| {
Expand Down Expand Up @@ -346,25 +349,28 @@ impl<'a> Resolver<'a> {
match nesting_def {
// When the instance variable is inside a method body, we determine the owner based on the method's receiver
Definition::Method(method) => {
// Method has explicit receiver (def self.foo or def Foo.bar)
if let Some(receiver_name_id) = method.receiver() {
let Some(NameRef::Resolved(resolved)) = self.graph.names().get(receiver_name_id) else {
// TODO: add diagnostic for unresolved receiver
continue;
if let Some(receiver) = method.receiver() {
let receiver_decl_id = match receiver {
Receiver::SelfReceiver(def_id) => *self
.graph
.definition_id_to_declaration_id(*def_id)
.expect("SelfReceiver definition should have a declaration"),
Receiver::ConstantReceiver(name_id) => {
let Some(NameRef::Resolved(resolved)) = self.graph.names().get(name_id) else {
continue;
};
*resolved.declaration_id()
}
};
let receiver_decl_id = *resolved.declaration_id();

// Instance variable in singleton method - owned by the receiver's singleton class
let owner_id = self.get_or_create_singleton_class(receiver_decl_id);
{
debug_assert!(
matches!(
self.graph.declarations().get(&owner_id),
Some(Declaration::Namespace(Namespace::SingletonClass(_)))
),
"Instance variable in singleton method should be owned by a SingletonClass"
);
}
debug_assert!(
matches!(
self.graph.declarations().get(&owner_id),
Some(Declaration::Namespace(Namespace::SingletonClass(_)))
),
"Instance variable in singleton method should be owned by a SingletonClass"
);
self.create_declaration(str_id, id, owner_id, |name| {
Declaration::InstanceVariable(Box::new(InstanceVariableDeclaration::new(
name, owner_id,
Expand Down
Loading