feat(doc): solar-based rewrite + vocs migration#14568
Conversation
zerosnacks
left a comment
There was a problem hiding this comment.
Overall super nice! Some small points of feedback
I would like to see a differentiation in style between @notice and @dev, the latter I think should be rendered in italic.
Ref: http://localhost:5173/src/utils/g/library.EnumerableSetLib
In forge-std as example a problem with function overrides is visible, maybe we can come up with something that makes it easier to distinguish between them.
|
I am also seeing brittleness with Vocs where just clicking around will cause |
|
From Amp's oracle review, I think these findings are broadly accurate, worth double checking: Blockers
Major
|
|
code side lgtm, will leave iteration on frontend to you |
- Fix UDVT xref prefix mismatch (udvt -> type) so {MyType} links land on
the actual type.MyType.mdx page.
- Build name_to_page from the filtered source set so cross-references
cannot resolve to library/ignored pages that are never emitted.
- Resolve @inheritdoc by parameter-type signature first, falling back to
name match, so overloads pick the right base function.
- Surface render panics: non-library files that panic during render now
fail the build instead of silently producing missing pages.
- Properly YAML-escape frontmatter title and description fields
(handles backslashes, quotes, control chars). Drop the 120-char
description truncation; emit the full first @notice with whitespace
collapsed.
- Use path-slash to_slash_lossy() for any path that ends up in a URL
(vocs links, sidebar paths, GitHub blob/edit URLs) so generated docs
stay correct on Windows.
Co-authored-by: Amp <amp@ampcode.com>
- find_contract_id: require absolute-path equality so contracts that share a file stem across src/ and lib/ no longer collide. - watch_doc: also watch external libraries (when --include-libraries), the homepage README override, <root>/README.md, the deployments dir, and foundry.toml so the live preview no longer goes silently stale. - Prune stale .mdx pages left behind by previous runs (renames, deletes) and remove now-empty directories; the vocs scaffold's own index.mdx is preserved. - Markdown table cells now escape | (column separator) and convert newlines to <br/>, so @param/@return text containing pipes or line breaks no longer shifts columns. - Homepage lookup now also tries <sources>/README.md between the explicit config.homepage override and <root>/README.md, matching the documented precedence comment. - Source links fall back to GitHub's HEAD (default branch agnostic) instead of master; the vocs editLink now uses the detected current branch (or 'main' as last resort) instead of hardcoding 'main'. Co-authored-by: Amp <amp@ampcode.com>
…ar_rewrite Amp-Thread-ID: https://ampcode.com/threads/T-019dfced-b5e9-77f4-9ed1-397907ffa292 Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # crates/doc/Cargo.toml
| .vars | ||
| .iter() | ||
| .map(|v| { | ||
| let raw = sm.span_to_snippet(v.ty.span).unwrap_or_default(); |
There was a problem hiding this comment.
Proposed: open a tracking issue for "derive canonical HIR/ABI parameter types for @inheritdoc overload matching", and land this characterization test now so the gap is recorded and self-corrects when fixed:
// Known limitation: `@inheritdoc` overload matching compares the raw *source text*
// of each parameter's type (only `uint`/`int` aliases are normalized, see
// `normalize_sol_type`). When a valid, compiling override spells a parameter type
// differently from the base declaration — here base `I.Status` vs override
// `Status` — the signatures fail to match and the inherited NatSpec is silently
// dropped. The `uint256` overload (verbatim match) inherits correctly, isolating
// the cause to type spelling rather than `@inheritdoc` resolution in general.
//
// CHARACTERIZATION test: it locks in the *current* (incomplete) behavior. The
// proper fix is to derive canonical HIR/ABI parameter types instead of source
// snippets. When that lands, the `!contains("Sets the account status")` assertion
// below will start failing — flip it to `contains(...)` at that point.
forgetest_init!(inheritdoc_overload_drops_docs_on_type_spelling, |prj, cmd| {
prj.add_source(
"I.sol",
r#"
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
interface I {
enum Status { Inactive, Active }
/// @notice Sets the account status.
/// @param s the new status
function configure(I.Status s) external;
/// @notice Configures by raw id.
/// @param id the raw id
function configure(uint256 id) external;
}
"#,
);
prj.add_source(
"C.sol",
r#"
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "./I.sol";
contract C is I {
/// @inheritdoc I
function configure(Status s) external override {}
/// @inheritdoc I
function configure(uint256 id) external override {}
}
"#,
);
cmd.args(["doc"]).assert_success();
let content =
std::fs::read_to_string(prj.root().join("docs/src/pages/src/contract.C.mdx")).unwrap();
// Control: the `uint256` overload matches verbatim and inherits correctly.
assert!(
content.contains("Configures by raw id"),
"uint256 overload should inherit its notice (control case), found:\n{content}"
);
// Known limitation (see header comment): the `Status` overload's inherited
// notice is currently dropped because `I.Status` != `Status` as raw text.
// FIXME(<issue>): once canonical HIR types are used, this should be
// `assert!(content.contains("Sets the account status"), ...)`.
assert!(
!content.contains("Sets the account status"),
"EXPECTED-FAIL breadcrumb: `@inheritdoc` overload type matching was improved \
to canonicalize types — update this test to assert the notice IS inherited.\n{content}"
);
});
Summary
Rewrite solar-based
forge doc(#14447), migrate tovocs.vocsscafold folder with produced MDX files--servefeature to avoid depending on npm, now user has to runforge doc --watch, and thennpm run devin another terminal.type.prefix.replace_inline_linksnow escapes bare<-><(fixes<email@example.com>parsed as JSX), unresolved{Ident}->`Ident`inline code, bare{->{. Unnamed return parameters rendered as<none>instead of<none>. Inherited natspec content (from@inheritdocresolution) now runs through the same sanitizer.constantandimmutablestate variables are rendered under a## Constantssection, separate from mutable## State Variables.--hostname,--port,--open) moved into a dedicatedServe optionshelp section so they no longer appear underWatch options.Closes #11884
Closes OSS-93
PR Checklist
Tested against vectorized/solady and ithacaxyz/account.