Skip to content

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

Merged
vinistock merged 1 commit intomainfrom
03-02-follow_aliases_when_they_are_used_in_mixins_or_parent_classes
Mar 4, 2026
Merged

Follow aliases when they are used in mixins or parent classes#620
vinistock merged 1 commit intomainfrom
03-02-follow_aliases_when_they_are_used_in_mixins_or_parent_classes

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Mar 2, 2026

When a constant alias is used in a mixin operation or a parent class, we need to follow the alias to figure out what namespace it points to.

@vinistock vinistock self-assigned this Mar 2, 2026
@vinistock vinistock added the bugfix A change that fixes an existing bug label Mar 2, 2026 — with Graphite App
@vinistock vinistock marked this pull request as ready for review March 2, 2026 19:05
@vinistock vinistock requested a review from a team as a code owner March 2, 2026 19:05
@vinistock vinistock force-pushed the 03-02-follow_aliases_when_they_are_used_in_mixins_or_parent_classes branch 2 times, most recently from 2c4dcbb to c043f4d Compare March 2, 2026 19:59
@vinistock vinistock force-pushed the 03-02-follow_aliases_when_they_are_used_in_mixins_or_parent_classes branch from c043f4d to adc0cd9 Compare March 4, 2026 14:49
Copy link
Member Author

vinistock commented Mar 4, 2026

Merge activity

  • Mar 4, 8:32 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 4, 8:32 PM UTC: @vinistock merged this pull request with Graphite.

@vinistock vinistock merged commit 2a5e113 into main Mar 4, 2026
36 of 37 checks passed
@vinistock vinistock deleted the 03-02-follow_aliases_when_they_are_used_in_mixins_or_parent_classes branch March 4, 2026 20:32
// 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

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

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.

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

Labels

bugfix A change that fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants