Skip to content

Support constant assign parsed before class/module definition - #1621

Merged
tompng merged 1 commit into
ruby:masterfrom
tompng:const_alias_reverse_order
Feb 22, 2026
Merged

Support constant assign parsed before class/module definition#1621
tompng merged 1 commit into
ruby:masterfrom
tompng:const_alias_reverse_order

Conversation

@tompng

@tompng tompng commented Feb 21, 2026

Copy link
Copy Markdown
Member

Support Const = RHS parsed before class RHS;end defined in another file.

This will fix one document-coverage check failure in ruby/ruby#16194

@tompng
tompng temporarily deployed to fork-preview-protection February 21, 2026 20:13 — with GitHub Actions Inactive
@matzbot

matzbot commented Feb 21, 2026

Copy link
Copy Markdown
Collaborator

🚀 Preview deployment available at: https://2f6c154e.rdoc-6cd.pages.dev (commit: bc71285)

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Comment thread lib/rdoc/code_object/constant.rb Outdated
# The module or class this constant is an alias for

def is_alias_for
@is_alias_for ||= find_alias_for

@kou kou Feb 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May find_alias_for return a String? If find_alias_for must not return a String, how about doing this in else?

diff --git a/lib/rdoc/code_object/constant.rb b/lib/rdoc/code_object/constant.rb
index 8823b0bd..69c3133a 100644
--- a/lib/rdoc/code_object/constant.rb
+++ b/lib/rdoc/code_object/constant.rb
@@ -92,7 +92,7 @@ class RDoc::Constant < RDoc::CodeObject
       @is_alias_for = found if found
       @is_alias_for
     else
-      @is_alias_for
+      @is_alias_for ||= find_alias_for
     end
   end

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That looks much better, thank you. Updated

Support `Const = RHS` parsed before `class RHS;end` defined in another file.
@tompng
tompng force-pushed the const_alias_reverse_order branch from ab711b7 to bc71285 Compare February 22, 2026 08:04
@tompng
tompng temporarily deployed to fork-preview-protection February 22, 2026 08:04 — with GitHub Actions Inactive
@tompng
tompng merged commit 84b0900 into ruby:master Feb 22, 2026
36 checks passed
@tompng
tompng deleted the const_alias_reverse_order branch February 22, 2026 10:35
st0012 pushed a commit that referenced this pull request Apr 30, 2026
Fixes #1662.

PR #1621 made RDoc::Constant#is_alias_for fall through to a lazy
find_alias_for lookup that returned whatever class the constant's value
named in the current store. The lazy result then flowed into
RDoc::ClassModule#update_aliases, which unconditionally wrote a dup'd
alias copy into @store.classes_hash, with two safeguards present
elsewhere bypassed:

* Context#add_module_alias refuses to clobber an existing class entry
  (the historic BasicObject = BlankSlate guard), but update_aliases
  did not.
* The prism parser only registers an alias when the constant has
  document_self set (so :nodoc: is honored), but the lazy resolver
  ignored it.

In combination these meant `Foo = Bar # :nodoc:`, where a real `Foo`
class was parsed elsewhere, would silently replace the real class's
documentation with an alias copy of `Bar` -- the literal failure mode
in #1662 (the prism shim's `Ripper = Prism::Translation::Ripper`
clobbering the real Ripper class docs).

Architectural fix
-----------------

Split RDoc::Constant#is_alias_for back into a pure accessor and a
separate Constant#resolved_alias_target lookup. is_alias_for now
returns only what was recorded explicitly (by Context#add_module_alias,
by ClassModule#update_aliases, or by ri marshal load) and never mutates
state. resolved_alias_target is the opportunistic forward-reference
lookup, used only by update_aliases as a fallback when no explicit
alias is recorded; it honors document_self so :nodoc: constants
don't lazy-resolve.

The lazy lookup itself uses a new Constant#is_alias_for_path attribute
populated by the parsers, instead of regex-matching #value at lookup
time. Both the prism parser (via ConstantReadNode/ConstantPathNode
detection in constant_path_string) and the legacy ripper parser (via
on_const-only token accumulation in parse_constant_body) already know
whether the RHS is a constant reference at parse time; we now propagate
that explicitly rather than rediscovering it from a stringly-typed
value.

Mechanical fix
--------------

ClassModule#update_aliases falls back to const.resolved_alias_target
when const.is_alias_for is unset, and gates that lazy-resolved path
behind a collision check that mirrors the one in
Context#add_module_alias. Explicit aliases (already vetted by
add_module_alias) keep their existing path so the two compose.

Tests
-----

Parser-level regressions cover :nodoc: assignment, real-class
collision, the combined :nodoc:-plus-collision scenario from #1662,
and the :stopdoc:/:startdoc: workaround path. Each asserts both the
negative (the would-be alias didn't clobber) and the positive (the
alias target keeps its own methods and aliases list). Unit tests on
update_aliases assert the same. The two existing PR #1621 tests
(test_constant_alias_reverse_order, test_repeated_constant_alias) are
updated to exercise the renamed resolved_alias_target API; the
underlying forward-reference behavior is preserved.
st0012 pushed a commit that referenced this pull request Apr 30, 2026
Fixes #1662.

PR #1621 made RDoc::Constant#is_alias_for fall through to a lazy
find_alias_for lookup that returned whatever class the constant's value
named in the current store. The lazy result then flowed into
RDoc::ClassModule#update_aliases, which unconditionally wrote a dup'd
alias copy into @store.classes_hash, with two safeguards present
elsewhere bypassed:

* Context#add_module_alias refuses to clobber an existing class entry
  (the historic BasicObject = BlankSlate guard), but update_aliases
  did not.
* The prism parser only registers an alias when the constant has
  document_self set (so :nodoc: is honored), but the lazy resolver
  ignored it.

In combination these meant `Foo = Bar # :nodoc:`, where a real `Foo`
class was parsed elsewhere, would silently replace the real class's
documentation with an alias copy of `Bar` -- the literal failure mode
in #1662 (the prism shim's `Ripper = Prism::Translation::Ripper`
clobbering the real Ripper class docs).

Architectural fix
-----------------

Split RDoc::Constant#is_alias_for back into a pure accessor and a
separate Constant#resolved_alias_target lookup. is_alias_for now
returns only what was recorded explicitly (by Context#add_module_alias,
by ClassModule#update_aliases, or by ri marshal load) and never mutates
state. resolved_alias_target is the opportunistic forward-reference
lookup, used only by update_aliases as a fallback when no explicit
alias is recorded; it honors document_self so :nodoc: constants
don't lazy-resolve.

The lazy lookup itself uses a new Constant#is_alias_for_path attribute
populated by the parsers, instead of regex-matching #value at lookup
time. Both the prism parser (via ConstantReadNode/ConstantPathNode
detection in constant_path_string) and the legacy ripper parser (via
on_const-only token accumulation in parse_constant_body) already know
whether the RHS is a constant reference at parse time; we now propagate
that explicitly rather than rediscovering it from a stringly-typed
value.

Mechanical fix
--------------

ClassModule#update_aliases falls back to const.resolved_alias_target
when const.is_alias_for is unset, and gates the destination write
behind a collision check that mirrors the one in
Context#add_module_alias: skip if classes_hash/modules_hash already
holds a real (non-alias) class at the destination name. The guard now
applies uniformly to both the explicit and lazy paths.

For that guard to compose with add_module_alias's pre-registration
flow (which already places an alias copy at the destination expecting
update_aliases to overwrite it with the properly-marked version),
add_module_alias now sets is_alias_for on the alias copy at creation
time. Existing alias copies are therefore distinguishable from real
classes by the unconditional guard.

Tests
-----

Parser-level regressions cover :nodoc: assignment, real-class
collision, the combined :nodoc:-plus-collision scenario from #1662,
the :stopdoc:/:startdoc: workaround path, and an explicit alias
followed by a same-named class re-open (which under the new guard
preserves both the alias target's methods and the methods added by
the re-open). Each asserts both the negative (the would-be alias
didn't clobber) and the positive (the alias target keeps its own
methods and aliases list). Unit tests on update_aliases assert the
same. The two existing PR #1621 tests
(test_constant_alias_reverse_order, test_repeated_constant_alias) are
updated to exercise the renamed resolved_alias_target API; the
underlying forward-reference behavior is preserved.
st0012 pushed a commit to st0012/rdoc that referenced this pull request May 1, 2026
ClassModule#update_aliases falls back to const.resolved_alias_target
when const.is_alias_for is unset, but never wrote the resolved target
back to the constant. As a result, forward-reference aliases (Foo = Bar
parsed before Bar exists) stayed "unaliased" on the constant after
Store#complete, regressing two downstream consumers that already worked
under the lazy lookup ruby#1621 introduced:

* RDoc::Stats#report_constants skips when constant.is_alias_for is set;
  forward-ref aliases were getting reported as undocumented.
* RDoc::Constant#marshal_dump serializes the alias target via
  is_alias_for; ri data lost the alias relationship.

Have update_aliases write the immediate resolved_alias_target back to
const.is_alias_for when the fallback path produces it (before the
chained-alias resolution loop, so the recorded target matches what
add_module_alias would have stored at parse time). Add a regression
test that exercises the after-Store#complete state.
st0012 pushed a commit to st0012/rdoc that referenced this pull request May 1, 2026
Fixes ruby#1662.

PR ruby#1621 made RDoc::Constant#is_alias_for fall through to a lazy
find_alias_for lookup that returned whatever class the constant's value
named in the current store. The lazy result then flowed into
RDoc::ClassModule#update_aliases, which unconditionally wrote a dup'd
alias copy into @store.classes_hash, with two safeguards present
elsewhere bypassed:

* Context#add_module_alias refuses to clobber an existing class entry
  (the historic BasicObject = BlankSlate guard), but update_aliases
  did not.
* The prism parser only registers an alias when the constant has
  document_self set (so :nodoc: is honored), but the lazy resolver
  ignored it.

In combination these meant `Foo = Bar # :nodoc:`, where a real `Foo`
class was parsed elsewhere, would silently replace the real class's
documentation with an alias copy of `Bar` -- the literal failure mode
in ruby#1662 (the prism shim's `Ripper = Prism::Translation::Ripper`
clobbering the real Ripper class docs).

Architectural fix
-----------------

Split RDoc::Constant#is_alias_for back into a pure accessor and a
separate Constant#resolved_alias_target lookup. is_alias_for now
returns only what was recorded explicitly (by Context#add_module_alias,
by ClassModule#update_aliases, or by ri marshal load) and never mutates
state. resolved_alias_target is the opportunistic forward-reference
lookup, used only by update_aliases as a fallback when no explicit
alias is recorded; it honors document_self so :nodoc: constants
don't lazy-resolve.

The lazy lookup itself uses a new Constant#is_alias_for_path attribute
populated by the parsers, instead of regex-matching #value at lookup
time. Both the prism parser (via ConstantReadNode/ConstantPathNode
detection in constant_path_string) and the legacy ripper parser (via
on_const-only token accumulation in parse_constant_body) already know
whether the RHS is a constant reference at parse time; we now propagate
that explicitly rather than rediscovering it from a stringly-typed
value.

Mechanical fix
--------------

ClassModule#update_aliases falls back to const.resolved_alias_target
when const.is_alias_for is unset, and gates the destination write
behind a collision check that mirrors the one in
Context#add_module_alias: skip if classes_hash/modules_hash already
holds a real (non-alias) class at the destination name. The guard now
applies uniformly to both the explicit and lazy paths.

When the fallback resolves a forward-reference target, the resolved
class is also written back to const.is_alias_for so downstream
consumers (RDoc::Stats#report_constants, RDoc::Constant#marshal_dump)
observe the alias relationship the lazy lookup found -- otherwise
forward-ref aliases get reported as undocumented and the alias slot is
serialized as nil into ri data.

For that guard to compose with add_module_alias's pre-registration
flow (which already places an alias copy at the destination expecting
update_aliases to overwrite it with the properly-marked version),
add_module_alias now sets is_alias_for on the alias copy at creation
time. Existing alias copies are therefore distinguishable from real
classes by the unconditional guard.

Tests
-----

Parser-level regressions cover :nodoc: assignment, real-class
collision, the combined :nodoc:-plus-collision scenario from ruby#1662,
the :stopdoc:/:startdoc: workaround path, an explicit alias followed
by a same-named class re-open (which under the new guard preserves
both the alias target's methods and the methods added by the re-open),
and the post-Store#complete is_alias_for persistence on a
forward-reference alias. Each asserts both the negative (the would-be
alias didn't clobber) and the positive (the alias target keeps its
own methods and aliases list). Unit tests on update_aliases assert
the same. The two existing PR ruby#1621 tests
(test_constant_alias_reverse_order, test_repeated_constant_alias) are
updated to exercise the renamed resolved_alias_target API; the
underlying forward-reference behavior is preserved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants