Add @override and @abstract comment annotations#3
Conversation
Add three persisted fields to RDoc::AnyMethod (@OverRide, @override_target, @abstract) and one to RDoc::ClassModule (@abstract). Bump both Marshal formats to version 4 and load missing fields as +false+/+nil+ so old .ri caches keep working until they're regenerated. Setters and tests cover the round-trip; resolution and rendering land in follow-up commits.
Add the parsing pipeline: - RDoc::Comment::AnnotationRegistry routes @tag names to handler classes - RDoc::Comment::Annotation::{Override,Abstract} are the two built-in handlers, each declaring which code-object types it applies to - RDoc::Comment::AnnotationScanner strips matching @tag lines from comment text and applies their handlers to the comment's owner - Comment#owner is wired through both Ruby parsers (Ripper and Prism) so the scanner can see the surrounding code object when Comment#parse runs Resolution lives on the Store and runs once during Store#complete: - resolve_overrides walks each @override-annotated method's ancestor chain (superclass + recursive includes) and records the matched ancestor's full name on +override_target+; missing matches log a warning - build_abstract_index inverts those edges into reverse hashes so templates can ask "what implements this abstract method?" or "what subclasses this abstract class?" in O(1) Test fixtures under test/rdoc/test_data/annotations exercise the resolver end-to-end across superclasses, mixins, and orphan @OverRide targets.
Add the rendering side for both backends. In the Aliki theme, each annotation appears as a restrained "inheritance ladder" notice in the description: a small diamond sigil, a small-caps title, an italic lede, and a CSS-drawn vertical rail with one rung per concrete implementation. Branch labels strip the host namespace so they stay short, and the rail uses lh-based geometry so it closes cleanly at the last rung regardless of line-height. The class.rhtml template hoists host_ns and the namespace-shorten lambda once per page so the per-method block doesn't recompute them. In the ri formatter, @OverRide emits "(Overrides ...)" or "(Overrides - no matching ancestor)", @abstract emits an "(Abstract - subclasses are expected to implement)" heading with a list of known implementations, and abstract classes get a class-level panel with their concrete subclasses. Includes tests for both renderers and an unobtrusive +annotations.css+ file linked from the Aliki head template.
st0012
temporarily deployed
to
fork-preview-protection
GitHub Actions
Inactive
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad1d89ec2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if klass.type == 'class' | ||
| sc = klass.superclass | ||
| sc = resolve_class_module_name(klass, sc) if sc.is_a?(String) | ||
| result << sc if sc | ||
| end |
There was a problem hiding this comment.
Override resolution currently enqueues a class’s superclass before its included modules, which does not match Ruby method lookup order. If both a superclass and an included module define the same method, @override will be resolved to the superclass method even though dispatch would hit the included module first, so rendered override targets and implementation indexes can point to the wrong ancestor.
Useful? React with 👍 / 👎.
|
|
||
| def force_parse_annotation_comments # :nodoc: | ||
| all_classes_and_modules.each do |klass| | ||
| klass.comment.parse if klass.comment.is_a?(RDoc::Comment) && klass.comment.text |
There was a problem hiding this comment.
Annotation parsing for classes only calls parse on klass.comment, but class docs are accumulated in comment_location and klass.comment may represent only one fragment (e.g., a reopened class across files). In that case an @abstract tag in a non-current fragment is never applied, so class-level abstract annotations can be silently dropped depending on comment ordering.
Useful? React with 👍 / 👎.
| each_recursive_ancestor(klass) do |ancestor| | ||
| next unless ancestor.abstract | ||
| @abstract_subclasses[ancestor.full_name] << klass.full_name | ||
| end |
There was a problem hiding this comment.
The abstract subclass index adds every descendant of an abstract ancestor without filtering out descendants that are themselves abstract. This makes the “Concrete subclasses” output include abstract intermediate classes, which is misleading in both Aliki and ri annotation output and contradicts the method’s own documentation.
Useful? React with 👍 / 👎.
The previous lede `Subclasses are expected to implement this method` reads as a future-tense / unfulfilled requirement, which is awkward when concrete implementations are already listed right below it. `Subclasses provide the implementation` is declarative and reads consistently whether or not the implementations list is empty.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57acfff57c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| out << RDoc::Markup::BlankLine.new | ||
| out << RDoc::Markup::Heading.new(4, | ||
| "(Abstract - subclasses are expected to implement)") | ||
| impls = store.implementations_of(method.full_name) |
There was a problem hiding this comment.
In normal ri usage the driver only calls load_cache during initialization and then loads the requested class/method; it never calls complete or build_abstract_index, and the @abstract_implementations / @abstract_subclasses hashes are not persisted in the cache. As a result, this lookup is empty for saved .ri data, so ri omits the advertised "Implemented by" lists (and the class path similarly omits concrete subclasses) even when the generated docs contain the same annotations.
Useful? React with 👍 / 👎.
| def find_method_named(full_name) | ||
| parent_name, sep, method_name = full_name.rpartition(/[#.]/) | ||
| return nil if sep.empty? | ||
| klass = find_class_or_module(parent_name) or return nil | ||
| klass.method_list.find do |m| | ||
| m.name == method_name && m.singleton == (sep == '.') | ||
| end |
There was a problem hiding this comment.
override_target and implementation entries are stored from AnyMethod#full_name, which uses Foo::bar for singleton methods, but this parser only recognizes # and . separators. For @override or @abstract on class methods, find_method_named("Foo::bar") returns nil, so Aliki renders the target as plain code or href="#" instead of linking to the documented method.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds two new YARD-style comment annotations to RDoc:
@override— declares that a method overrides one inherited from an ancestor; the resolver walks the superclass chain and recursive includes to find a match@abstract— declares a method (or class) as abstract; subclasses/implementors get reverse-indexed so the rendered docs can list themBoth annotations flow through every layer: the comment scanner strips them from the rendered text, the store resolves and reverse-indexes them once per build, and both the ri and Aliki backends surface them in the output.
In Aliki, each annotation appears as a restrained "inheritance ladder" notice — diamond sigil, small-caps title, italic lede, and a CSS-drawn vertical rail with one rung per concrete implementation/subclass. Branch labels strip the host namespace so they stay short.
In ri,
@overrideemits an "(Overrides ...)" heading and@abstractemits a heading plus a list of known implementations or concrete subclasses.Structure
Four commits, each compiles and tests cleanly on its own:
Add @override and @abstract data model with Marshal v4— adds the persisted fields onRDoc::AnyMethodandRDoc::ClassModule, bumps both Marshal formats to v4 with backward-compatible defaultsParse and resolve @override and @abstract annotations— adds the registry/scanner/handler pipeline, wiresComment#ownerthrough both Ruby parsers, and runsresolve_overrides+build_abstract_indexfromStore#completeRender @override and @abstract annotations in ri and Aliki output— both renderers, including the Aliki CSSAdd end-to-end test for annotation rendering through full RDoc run— exercises the full pipeline against fixtures undertest/rdoc/test_data/annotationsNotes
false/nilso old.ricaches keep working until they're regenerated.RDoc.warnhelper for@overridetargets that don't resolve — these are silent in--quiet.