Skip to content

Follow aliases when they are used in mixins or parent classes #620

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
vinistock merged 1 commit into from
Mar 4, 2026
Merged

Follow aliases when they are used in mixins or parent classes #620

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
206 changes: 135 additions & 71 deletions rust/rubydex/src/resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,32 +286,22 @@ impl<'a> Resolver<'a> {
// `Foo` is undefined). The method is orphaned as a consequence.
continue;
};
self.get_or_create_singleton_class(owner_decl_id)
.expect("Methods with self receiver should always be inside a namespace")
let Some(singleton_id) = self.get_or_create_singleton_class(owner_decl_id) else {
// The enclosing declaration is a constant that can't be promoted to a namespace
// (e.g., `CONST = 1; module CONST; def self.bar; end; end`). The method is
// orphaned as a consequence.
continue;
};
singleton_id
}
Some(Receiver::ConstantReceiver(name_id)) => {
let mut receiver_decl_id = match self.graph.names().get(name_id).unwrap() {
let receiver_decl_id = match self.graph.names().get(name_id).unwrap() {
NameRef::Resolved(resolved) => *resolved.declaration_id(),
NameRef::Unresolved(_) => {
continue;
}
};

let receiver_decl = self.graph.declarations().get(&receiver_decl_id).unwrap();

// If the method receiver is a constant alias, it needs to point to a namespace or else we don't have a place to declare the method
if matches!(receiver_decl, Declaration::ConstantAlias(_)) {
let resolved_ids = self.resolve_alias_chains(receiver_decl_id);
let Some(namespace) = resolved_ids
.iter()
.find(|id| self.graph.declarations().get(id).unwrap().as_namespace().is_some())
else {
continue;
};

receiver_decl_id = *namespace;
}

let Some(singleton_id) = self.get_or_create_singleton_class(receiver_decl_id) else {
continue;
};
Expand Down Expand Up @@ -387,22 +377,7 @@ impl<'a> Resolver<'a> {
continue;
};

let mut receiver_decl_id = *resolved.declaration_id();
let receiver_decl = self.graph.declarations().get(&receiver_decl_id).unwrap();

// If the method receiver is a constant alias, it needs to point to a namespace or else we don't have a place to declare the method
if matches!(receiver_decl, Declaration::ConstantAlias(_)) {
let resolved_ids = self.resolve_alias_chains(receiver_decl_id);
let Some(namespace) = resolved_ids.iter().find(|id| {
self.graph.declarations().get(id).unwrap().as_namespace().is_some()
}) else {
continue;
};

receiver_decl_id = *namespace;
}

receiver_decl_id
*resolved.declaration_id()
}
};

Expand Down Expand Up @@ -616,6 +591,14 @@ impl<'a> Resolver<'a> {
fn get_or_create_singleton_class(&mut self, attached_id: DeclarationId) -> Option<DeclarationId> {
let attached_decl = self.graph.declarations().get(&attached_id).unwrap();

// If the attached object is a constant alias, follow the alias chain to find the actual namespace
if matches!(attached_decl, Declaration::ConstantAlias(_)) {
return match self.resolve_to_namespace(attached_id) {
Some(id) => self.get_or_create_singleton_class(id),
None => None,
};
}

if matches!(attached_decl, Declaration::Constant(_)) {
if self.graph.all_definitions_promotable(attached_decl) {
self.graph.promote_constant_to_namespace(attached_id, |name, owner_id| {
Expand Down Expand Up @@ -852,15 +835,11 @@ impl<'a> Resolver<'a> {
Mixin::Prepend(_) => {
match self.graph.names().get(constant_reference.name_id()).unwrap() {
NameRef::Resolved(resolved) => {
let declaration_id = resolved.declaration_id();

// Skip if the mixin is not a namespace (e.g.: dynamically defined class or module that we
// misidentified as a constant)
if !self.graph.is_namespace(declaration_id) {
let Some(module_id) = self.resolve_to_namespace(*resolved.declaration_id()) else {
continue;
Copy link
Contributor

@Morriar Morriar Mar 4, 2026

Choose a reason for hiding this comment

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

I'm wondering if we should set context.partial = true here when resolve_to_namespace returns None because the alias target isn't resolved yet (as opposed to being a non-namespace constant).

What would happen with this test?

  #[test]
  fn include_via_alias_when_target_resolves_later() {
      let mut context = GraphTest::new();
      context.index_uri("file:///a.rb", {
          r"
          AliasedModule = SomeModule

          class Foo
            include AliasedModule
          end

          module SomeModule
          end
          "
      });
      context.resolve();

      assert_ancestors_eq!(context, "Foo", ["SomeModule", "Foo", "Object"]);
  }

Copy link
Member Author

@vinistock vinistock Mar 5, 2026

Choose a reason for hiding this comment

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

This specific example already works as we figure out what the alias points to. I prototyped making the ancestors partial in this case, but I'm not 100% sure it's worth it to be honest.

Making the ancestors partial means we enqueue retries for linearization, which in the case of an unresolved alias will never complete and only slows down resolution.

Also, it makes the code a bit more complicated because partial ancestors store a NameId, so then we need to follow aliases to trace back what NameId are associated with them.

Considering that we will add a diagnostic pointing to the fact that the alias was not successfully resolved, I would vote to not do this. At least not for now.

}
};

let ids = match self.linearize_ancestors(*declaration_id, context) {
let ids = match self.linearize_ancestors(module_id, context) {
Ancestors::Complete(ids) => ids,
Ancestors::Cyclic(ids) => {
context.cyclic = true;
Expand Down Expand Up @@ -895,15 +874,11 @@ impl<'a> Resolver<'a> {
Mixin::Include(_) | Mixin::Extend(_) => {
match self.graph.names().get(constant_reference.name_id()).unwrap() {
NameRef::Resolved(resolved) => {
let declaration_id = resolved.declaration_id();

// Skip if the mixin is not a namespace (e.g.: dynamically defined class or module that we
// misidentified as a constant)
if !self.graph.is_namespace(declaration_id) {
let Some(module_id) = self.resolve_to_namespace(*resolved.declaration_id()) else {
continue;
}
};

let mut ids = match self.linearize_ancestors(*declaration_id, context) {
let mut ids = match self.linearize_ancestors(module_id, context) {
Ancestors::Complete(ids) => ids,
Ancestors::Cyclic(ids) => {
context.cyclic = true;
Expand Down Expand Up @@ -1200,6 +1175,20 @@ impl<'a> Resolver<'a> {
}
}

/// If `declaration_id` is already a namespace, returns it directly. If it's a `ConstantAlias`, follows the alias
/// chain and returns the first namespace found. Returns `None` for all other declaration types or unresolved alias
/// chains.
fn resolve_to_namespace(&self, declaration_id: DeclarationId) -> Option<DeclarationId> {
match self.graph.declarations().get(&declaration_id)? {
Declaration::Namespace(_) => Some(declaration_id),
Declaration::ConstantAlias(_) => self
.resolve_alias_chains(declaration_id)
.into_iter()
.find(|id| self.graph.is_namespace(id)),
_ => None,
}
}

/// Resolves an alias chain to get all possible final target declarations.
/// Returns the original ID if it's not an alias or if the target hasn't been resolved yet.
///
Expand Down Expand Up @@ -1253,10 +1242,27 @@ impl<'a> Resolver<'a> {
return scope_outcome;
}

// Search inheritance chain
// Search inheritance chain, following alias chains to find the actual namespace
let ancestor_outcome = match self.graph.names().get(nesting).unwrap() {
NameRef::Resolved(nesting_name_ref) => {
self.search_ancestors(*nesting_name_ref.declaration_id(), str_id)
let resolved_ids = self.resolve_alias_chains(*nesting_name_ref.declaration_id());
let mut result = Outcome::Unresolved(None);

for &id in &resolved_ids {
match self.graph.declarations().get(&id) {
Some(Declaration::ConstantAlias(_)) => {
result = Outcome::Retry(None);
break;
}
Some(Declaration::Namespace(_)) => {
result = self.search_ancestors(id, str_id);
break;
}
_ => {}
}
}

result
}
NameRef::Unresolved(_) => Outcome::Retry(None),
};
Expand Down Expand Up @@ -1344,23 +1350,11 @@ impl<'a> Resolver<'a> {
if let NameRef::Resolved(nesting_name_ref) = self.graph.names().get(nesting_id).unwrap() {
let declaration_id = *nesting_name_ref.declaration_id();

match self.graph.declarations().get(&declaration_id) {
Some(Declaration::Namespace(namespace)) => {
if let Some(member) = namespace.member(&str_id) {
return Outcome::Resolved(*member, None);
}
}
Some(Declaration::ConstantAlias(_)) => {
for id in &self.resolve_alias_chains(declaration_id) {
if let Some(declaration) = self.graph.declarations().get(id)
&& let Some(namespace) = declaration.as_namespace()
&& let Some(member) = namespace.member(&str_id)
{
return Outcome::Resolved(*member, None);
}
}
}
_ => {}
if let Some(namespace_id) = self.resolve_to_namespace(declaration_id)
&& let Some(namespace) = self.graph.declarations().get(&namespace_id).unwrap().as_namespace()
&& let Some(member) = namespace.member(&str_id)
{
return Outcome::Resolved(*member, None);
}

current_name = nesting_name_ref.name();
Expand Down Expand Up @@ -1583,10 +1577,8 @@ impl<'a> Resolver<'a> {

match name {
NameRef::Resolved(resolved) => {
let declaration_id = resolved.declaration_id();

if self.graph.is_namespace(declaration_id) {
explicit_parents.push(*declaration_id);
if let Some(parent_id) = self.resolve_to_namespace(*resolved.declaration_id()) {
explicit_parents.push(parent_id);
}
}
NameRef::Unresolved(_) => {
Expand Down Expand Up @@ -4967,4 +4959,76 @@ mod tests {
assert_declaration_exists!(context, "Baz");
assert_constant_reference_to!(context, "CONST", "file:///reopen.rb:5:5-5:10");
}

#[test]
fn constant_alias_reopened_as_class_with_nested_inheritance() {
let mut context = GraphTest::new();
context.index_uri("file:///a.rb", {
r"
module Foo
Bar = ::Object
end

module Foo
class Bar
class Baz < Something
end
end
end
"
});
context.resolve();

assert_declaration_exists!(context, "Foo::Bar");
}

#[test]
fn superclass_through_alias() {
let mut context = GraphTest::new();
context.index_uri("file:///a.rb", {
r"
class Base; end
AliasedBase = Base
class Foo < AliasedBase; end
"
});
context.resolve();
assert_no_diagnostics!(&context);
assert_ancestors_eq!(context, "Foo", ["Foo", "Base", "Object"]);
}

#[test]
fn mixin_through_alias() {
let mut context = GraphTest::new();
context.index_uri("file:///a.rb", {
r"
module M; end
AliasM = M
class Foo
include AliasM
end
"
});
context.resolve();
assert_no_diagnostics!(&context);
assert_ancestors_eq!(context, "Foo", ["Foo", "M", "Object"]);
}

#[test]
fn self_method_inside_non_promotable_constant() {
let mut context = GraphTest::new();
context.index_uri("file:///a.rb", {
r"
CONST = 1
module CONST
def self.bar
end
end
"
});
// Should not panic when a `def self.` method is inside a constant that can't be promoted to a namespace (e.g.,
// `CONST = 1` is non-promotable).
context.resolve();
assert_declaration_exists!(context, "CONST");
}
}
Loading