Represent unresolved superclass as Ancestor::Partial#651
Represent unresolved superclass as Ancestor::Partial#651
Conversation
When a superclass reference is unresolved (e.g. `class Foo < Bar` where Bar is never defined), the ancestor chain for Foo was silently falling back to Object's chain with Ancestors::Partial state. Bar was completely absent from the chain. Mixins handle this consistently by pushing Ancestor::Partial(name_id) when a mixin reference is unresolved. Apply the same approach for superclasses. Also update assert_ancestors_eq to compare by name string so it works uniformly for both Complete and Partial ancestor entries.
Adds rdx_class_superclass_name FFI function that walks the ancestor chain looking for an Ancestor::Partial entry and reconstructs its fully-qualified name string (handling ::Foo and Foo::Bar forms). Exposed at the Ruby level as Class#superclass_name, returning nil when the superclass is resolved or absent.
Morriar
force-pushed
the
at-unresolved-superclass-partial-ancestor
branch
from
7a593bf to
8f0b663
Compare
vinistock
left a comment
vinistock
left a comment
There was a problem hiding this comment.
I think the resolution changes can be shipped, but I do think the superclass API will be nicer returning constant reference objects
| // Class#superclass_name: () -> String? | ||
| // Returns the unresolved superclass name as written in source, or nil if the superclass is | ||
| // resolved (i.e. it exists in the indexed workspace) or there is no superclass. | ||
| static VALUE rdxr_class_superclass_name(VALUE self) { |
There was a problem hiding this comment.
I wonder if we should instead return a constant reference, where the concept of it being resolved or not is and what declaration it points to is encapsulated in it.
In fact, this is related to the reference related refactors I'm working on. The type hierarchy request needs to know direct ancestors (mixins and parent classes) and I wanted to make sure we were returning proper references.
What do you think?
We can always remove this part of the PR and ship just the partial ancestors part while we refactor references.
|
|
||
| assert_definition_at!(&context, "3:1-3:54", Class, |def| { | ||
| assert!(def.superclass_ref().is_none(),); | ||
| assert!(def.superclass_ref().is_some(),); |
There was a problem hiding this comment.
There's a weird trailing comma.
| assert!(def.superclass_ref().is_some(),); | |
| assert!(def.superclass_ref().is_some()); |
| .unwrap(); | ||
|
|
||
| match declaration.as_namespace().unwrap().ancestors() { | ||
| let ancestors = match declaration.as_namespace().unwrap().ancestors() { |
There was a problem hiding this comment.
Is this change to allow asserting partial ancestors? I think Stan made this possible in one of his recent PRs.
2 participants
Summary
class Foo < Baris indexed andBaris never defined,Foo's ancestor chain was silently falling back to[Foo, Object, BasicObject](partial) —Barwas completely absent. This is inconsistent with how unresolved mixins are handled, which pushAncestor::Partial(name_id)as a placeholder.linearize_parent_classnow returnsAncestors::Partial([Ancestor::Partial(name_id)])instead of falling back to Object's chain.assert_ancestors_eqto compare by name string, so it works uniformly for bothCompleteandPartialancestor entries (previously panicked onAncestors::Partial).Class#superclass_nameRuby API that returns the unresolved superclass name as written in source, ornilwhen resolved or absent.Test plan
unresolved_superclass_is_represented_as_partial_ancestorverifiesclass Foo < Baryields["Foo", "Bar"]test_superclass_name_returns_nil_when_superclass_is_resolvedandtest_superclass_name_returns_name_when_superclass_is_unresolvedcargo test— all 380 tests passbundle exec ruby -Itest test/declaration_test.rb— all 23 tests passcargo clippy --all-targets -- -D warnings— clean