Skip to content

Commit 2c4dcbb

Browse files
vinistockvinistock
committed
Follow aliases when they are used in mixins or parent classes
1 parent fbed256 commit 2c4dcbb

1 file changed

Lines changed: 135 additions & 71 deletions

File tree

‎rust/rubydex/src/resolution.rs‎

Lines changed: 135 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -286,32 +286,22 @@ impl<'a> Resolver<'a> {
286286
// `Foo` is undefined). The method is orphaned as a consequence.
287287
continue;
288288
};
289-
self.get_or_create_singleton_class(owner_decl_id)
290-
.expect("Methods with self receiver should always be inside a namespace")
289+
let Some(singleton_id) = self.get_or_create_singleton_class(owner_decl_id) else {
290+
// The enclosing declaration is a constant that can't be promoted to a namespace
291+
// (e.g., `CONST = 1; module CONST; def self.bar; end; end`). The method is
292+
// orphaned as a consequence.
293+
continue;
294+
};
295+
singleton_id
291296
}
292297
Some(Receiver::ConstantReceiver(name_id)) => {
293-
let mut receiver_decl_id = match self.graph.names().get(name_id).unwrap() {
298+
let receiver_decl_id = match self.graph.names().get(name_id).unwrap() {
294299
NameRef::Resolved(resolved) => *resolved.declaration_id(),
295300
NameRef::Unresolved(_) => {
296301
continue;
297302
}
298303
};
299304

300-
let receiver_decl = self.graph.declarations().get(&receiver_decl_id).unwrap();
301-
302-
// 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
303-
if matches!(receiver_decl, Declaration::ConstantAlias(_)) {
304-
let resolved_ids = self.resolve_alias_chains(receiver_decl_id);
305-
let Some(namespace) = resolved_ids
306-
.iter()
307-
.find(|id| self.graph.declarations().get(id).unwrap().as_namespace().is_some())
308-
else {
309-
continue;
310-
};
311-
312-
receiver_decl_id = *namespace;
313-
}
314-
315305
let Some(singleton_id) = self.get_or_create_singleton_class(receiver_decl_id) else {
316306
continue;
317307
};
@@ -387,22 +377,7 @@ impl<'a> Resolver<'a> {
387377
continue;
388378
};
389379

390-
let mut receiver_decl_id = *resolved.declaration_id();
391-
let receiver_decl = self.graph.declarations().get(&receiver_decl_id).unwrap();
392-
393-
// 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
394-
if matches!(receiver_decl, Declaration::ConstantAlias(_)) {
395-
let resolved_ids = self.resolve_alias_chains(receiver_decl_id);
396-
let Some(namespace) = resolved_ids.iter().find(|id| {
397-
self.graph.declarations().get(id).unwrap().as_namespace().is_some()
398-
}) else {
399-
continue;
400-
};
401-
402-
receiver_decl_id = *namespace;
403-
}
404-
405-
receiver_decl_id
380+
*resolved.declaration_id()
406381
}
407382
};
408383

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

594+
// If the attached object is a constant alias, follow the alias chain to find the actual namespace
595+
if matches!(attached_decl, Declaration::ConstantAlias(_)) {
596+
return match self.resolve_to_namespace(attached_id) {
597+
Some(id) => self.get_or_create_singleton_class(id),
598+
None => None,
599+
};
600+
}
601+
619602
if matches!(attached_decl, Declaration::Constant(_)) {
620603
if self.graph.all_definitions_promotable(attached_decl) {
621604
self.graph.promote_constant_to_namespace(attached_id, |name, owner_id| {
@@ -852,15 +835,11 @@ impl<'a> Resolver<'a> {
852835
Mixin::Prepend(_) => {
853836
match self.graph.names().get(constant_reference.name_id()).unwrap() {
854837
NameRef::Resolved(resolved) => {
855-
let declaration_id = resolved.declaration_id();
856-
857-
// Skip if the mixin is not a namespace (e.g.: dynamically defined class or module that we
858-
// misidentified as a constant)
859-
if !self.graph.is_namespace(declaration_id) {
838+
let Some(module_id) = self.resolve_to_namespace(*resolved.declaration_id()) else {
860839
continue;
861-
}
840+
};
862841

863-
let ids = match self.linearize_ancestors(*declaration_id, context) {
842+
let ids = match self.linearize_ancestors(module_id, context) {
864843
Ancestors::Complete(ids) => ids,
865844
Ancestors::Cyclic(ids) => {
866845
context.cyclic = true;
@@ -895,15 +874,11 @@ impl<'a> Resolver<'a> {
895874
Mixin::Include(_) | Mixin::Extend(_) => {
896875
match self.graph.names().get(constant_reference.name_id()).unwrap() {
897876
NameRef::Resolved(resolved) => {
898-
let declaration_id = resolved.declaration_id();
899-
900-
// Skip if the mixin is not a namespace (e.g.: dynamically defined class or module that we
901-
// misidentified as a constant)
902-
if !self.graph.is_namespace(declaration_id) {
877+
let Some(module_id) = self.resolve_to_namespace(*resolved.declaration_id()) else {
903878
continue;
904-
}
879+
};
905880

906-
let mut ids = match self.linearize_ancestors(*declaration_id, context) {
881+
let mut ids = match self.linearize_ancestors(module_id, context) {
907882
Ancestors::Complete(ids) => ids,
908883
Ancestors::Cyclic(ids) => {
909884
context.cyclic = true;
@@ -1200,6 +1175,20 @@ impl<'a> Resolver<'a> {
12001175
}
12011176
}
12021177

1178+
/// If `declaration_id` is already a namespace, returns it directly. If it's a `ConstantAlias`, follows the alias
1179+
/// chain and returns the first namespace found. Returns `None` for all other declaration types or unresolved alias
1180+
/// chains.
1181+
fn resolve_to_namespace(&self, declaration_id: DeclarationId) -> Option<DeclarationId> {
1182+
match self.graph.declarations().get(&declaration_id)? {
1183+
Declaration::Namespace(_) => Some(declaration_id),
1184+
Declaration::ConstantAlias(_) => self
1185+
.resolve_alias_chains(declaration_id)
1186+
.into_iter()
1187+
.find(|id| self.graph.is_namespace(id)),
1188+
_ => None,
1189+
}
1190+
}
1191+
12031192
/// Resolves an alias chain to get all possible final target declarations.
12041193
/// Returns the original ID if it's not an alias or if the target hasn't been resolved yet.
12051194
///
@@ -1253,10 +1242,27 @@ impl<'a> Resolver<'a> {
12531242
return scope_outcome;
12541243
}
12551244

1256-
// Search inheritance chain
1245+
// Search inheritance chain, following alias chains to find the actual namespace
12571246
let ancestor_outcome = match self.graph.names().get(nesting).unwrap() {
12581247
NameRef::Resolved(nesting_name_ref) => {
1259-
self.search_ancestors(*nesting_name_ref.declaration_id(), str_id)
1248+
let resolved_ids = self.resolve_alias_chains(*nesting_name_ref.declaration_id());
1249+
let mut result = Outcome::Unresolved(None);
1250+
1251+
for &id in &resolved_ids {
1252+
match self.graph.declarations().get(&id) {
1253+
Some(Declaration::ConstantAlias(_)) => {
1254+
result = Outcome::Retry(None);
1255+
break;
1256+
}
1257+
Some(Declaration::Namespace(_)) => {
1258+
result = self.search_ancestors(id, str_id);
1259+
break;
1260+
}
1261+
_ => {}
1262+
}
1263+
}
1264+
1265+
result
12601266
}
12611267
NameRef::Unresolved(_) => Outcome::Retry(None),
12621268
};
@@ -1344,23 +1350,11 @@ impl<'a> Resolver<'a> {
13441350
if let NameRef::Resolved(nesting_name_ref) = self.graph.names().get(nesting_id).unwrap() {
13451351
let declaration_id = *nesting_name_ref.declaration_id();
13461352

1347-
match self.graph.declarations().get(&declaration_id) {
1348-
Some(Declaration::Namespace(namespace)) => {
1349-
if let Some(member) = namespace.member(&str_id) {
1350-
return Outcome::Resolved(*member, None);
1351-
}
1352-
}
1353-
Some(Declaration::ConstantAlias(_)) => {
1354-
for id in &self.resolve_alias_chains(declaration_id) {
1355-
if let Some(declaration) = self.graph.declarations().get(id)
1356-
&& let Some(namespace) = declaration.as_namespace()
1357-
&& let Some(member) = namespace.member(&str_id)
1358-
{
1359-
return Outcome::Resolved(*member, None);
1360-
}
1361-
}
1362-
}
1363-
_ => {}
1353+
if let Some(namespace_id) = self.resolve_to_namespace(declaration_id)
1354+
&& let Some(namespace) = self.graph.declarations().get(&namespace_id).unwrap().as_namespace()
1355+
&& let Some(member) = namespace.member(&str_id)
1356+
{
1357+
return Outcome::Resolved(*member, None);
13641358
}
13651359

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

15841578
match name {
15851579
NameRef::Resolved(resolved) => {
1586-
let declaration_id = resolved.declaration_id();
1587-
1588-
if self.graph.is_namespace(declaration_id) {
1589-
explicit_parents.push(*declaration_id);
1580+
if let Some(parent_id) = self.resolve_to_namespace(*resolved.declaration_id()) {
1581+
explicit_parents.push(parent_id);
15901582
}
15911583
}
15921584
NameRef::Unresolved(_) => {
@@ -4901,4 +4893,76 @@ mod tests {
49014893
assert_declaration_exists!(context, "Baz");
49024894
assert_constant_reference_to!(context, "CONST", "file:///reopen.rb:5:5-5:10");
49034895
}
4896+
4897+
#[test]
4898+
fn constant_alias_reopened_as_class_with_nested_inheritance() {
4899+
let mut context = GraphTest::new();
4900+
context.index_uri("file:///a.rb", {
4901+
r"
4902+
module Foo
4903+
Bar = ::Object
4904+
end
4905+
4906+
module Foo
4907+
class Bar
4908+
class Baz < Something
4909+
end
4910+
end
4911+
end
4912+
"
4913+
});
4914+
context.resolve();
4915+
4916+
assert_declaration_exists!(context, "Foo::Bar");
4917+
}
4918+
4919+
#[test]
4920+
fn superclass_through_alias() {
4921+
let mut context = GraphTest::new();
4922+
context.index_uri("file:///a.rb", {
4923+
r"
4924+
class Base; end
4925+
AliasedBase = Base
4926+
class Foo < AliasedBase; end
4927+
"
4928+
});
4929+
context.resolve();
4930+
assert_no_diagnostics!(&context);
4931+
assert_ancestors_eq!(context, "Foo", ["Foo", "Base", "Object"]);
4932+
}
4933+
4934+
#[test]
4935+
fn mixin_through_alias() {
4936+
let mut context = GraphTest::new();
4937+
context.index_uri("file:///a.rb", {
4938+
r"
4939+
module M; end
4940+
AliasM = M
4941+
class Foo
4942+
include AliasM
4943+
end
4944+
"
4945+
});
4946+
context.resolve();
4947+
assert_no_diagnostics!(&context);
4948+
assert_ancestors_eq!(context, "Foo", ["Foo", "M", "Object"]);
4949+
}
4950+
4951+
#[test]
4952+
fn self_method_inside_non_promotable_constant() {
4953+
let mut context = GraphTest::new();
4954+
context.index_uri("file:///a.rb", {
4955+
r"
4956+
CONST = 1
4957+
module CONST
4958+
def self.bar
4959+
end
4960+
end
4961+
"
4962+
});
4963+
// Should not panic when a `def self.` method is inside a constant that can't be promoted to a namespace (e.g.,
4964+
// `CONST = 1` is non-promotable).
4965+
context.resolve();
4966+
assert_declaration_exists!(context, "CONST");
4967+
}
49044968
}

0 commit comments

Comments
 (0)