Skip to content

Conversation

@st0012
Copy link
Member

@st0012 st0012 commented Dec 24, 2025

Fixes #1523

The issue is likely caused by RDoc generating documentation using data from marshal load (used by ri), which has different types than if the generation happen directly.

This code can reproduce the same error:

require 'rdoc'
require 'tmpdir'

# Setup store
options = RDoc::Options.new
store = RDoc::Store.new(options, path: Dir.tmpdir)

# Create a class with a comment
tl = store.add_file 'file.rb'
ns = tl.add_module RDoc::NormalModule, 'Namespace'
cm = ns.add_class RDoc::NormalClass, 'Klass', 'Super'
cm.add_comment 'This is a class comment', tl

# Before marshalling - works fine
puts "Before marshal: #{cm.search_snippet.inspect}"

# Marshal round-trip (simulates loading from .ri cache)
loaded = Marshal.load(Marshal.dump(cm))
loaded.store = store

# After marshalling - fails with NoMethodError
puts "After marshal: #{loaded.search_snippet.inspect}"

Similar to other code objects' marshalling behaviour, ClassModule#comment_location's content type would change before/after marshalling. But it should still be an array of pairs that respond to the same set of methods.

This commit also adds documentation about the marshalling behaviour of ClassModule#comment_location.

I don't like the fact that data types & shapes could change before/after marshalling. But changing this design will be a breaking change and require some careful planning. So for now the change it to make sure the type change is less surprising

Similar to other code objects' marshalling behaviour, `ClassModule#comment_location`'s content
type would change before/after marshalling. But it should still be an array of pairs that respond
to the same set of methods.

This commit also adds documentation about the marshalling behaviour of `ClassModule#comment_location`.
@st0012 st0012 added the bug label Dec 24, 2025
@matzbot
Copy link
Collaborator

matzbot commented Dec 24, 2025

🚀 Preview deployment available at: https://30718c1a.rdoc-6cd.pages.dev (commit: 99eef47)

document.parts.map { |doc| [doc, doc.file] }
else
RDoc::Markup::Document.new document
[[document, document.file]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We were creating a brand new document before. Was that intentional?

For example, should the updated code be this?

new_doc = RDoc::Markup::Document.new document
[[new_doc, new_doc.file]]

Copy link
Member

Choose a reason for hiding this comment

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

document is not a comment, so I think it should ideally be [[comment, document.file]] for both branch.

@comment_locaation = [[comment, document.file]]

parse([[@comment, document.file]]) will return a parsed document cached in @comment (as attr_reader :document)

Although not fully investigated the effect of doing this.

Copy link
Member

Choose a reason for hiding this comment

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

Since parse @comment_location seems working, I'm OK with the less-impact change: [[document, location]] for now 👍

@st0012 st0012 merged commit 825d6e9 into master Dec 24, 2025
62 checks passed
@st0012 st0012 deleted the fix-1523 branch December 24, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gem install --document=ri,rdoc causes error with rdoc 7.0.2 with undefined method 'empty?' for an instance of RDoc::Markup::Paragraph

5 participants