Skip to content

Commit bd4ef2a

Browse files
mattssedecofe
andauthored
fix(doc): resolve same-contract references to anchor links (#15672)
* fix(doc): resolve same-contract references to anchor links References naming a member of the contract being rendered, like {toEthSignedMessageHash}, degraded to inline code because the global name index only contains top-level items, and {Contract-member} self-references were routed through the ambiguous name index even though the target page is the one being rendered. Both now resolve lexically to anchor-only links on the current page, before any global name lookup. * fix(doc): validate qualified local reference members --------- Co-authored-by: Derek Cofausper <256792747+decofe@users.noreply.github.com>
1 parent 7f1931b commit bd4ef2a

3 files changed

Lines changed: 370 additions & 21 deletions

File tree

crates/doc/src/hir_ext.rs

Lines changed: 150 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,46 @@ pub(crate) fn clean_block_doc_content(raw: &str) -> String {
642642

643643
// ── inline link replacement ───────────────────────────────────────────────────
644644

645+
/// Members of the contract page currently being rendered.
646+
///
647+
/// Used to resolve `{member}` and `{Contract-member}` references lexically:
648+
/// a name that belongs to the current contract links to its heading anchor on
649+
/// the same page instead of going through the global name index (which only
650+
/// contains top-level items and could otherwise resolve to an unrelated page).
651+
#[derive(Debug)]
652+
pub struct LocalMembers {
653+
/// The current contract's name.
654+
name: String,
655+
/// Member names with a heading (and thus an anchor) on the current page.
656+
members: HashSet<String>,
657+
}
658+
659+
impl LocalMembers {
660+
/// Create an empty member set for the contract `name`.
661+
pub fn new(name: &str) -> Self {
662+
Self { name: name.to_string(), members: HashSet::new() }
663+
}
664+
665+
/// Record a member that is rendered as a `### member` heading on the page.
666+
pub fn insert(&mut self, member: &str) {
667+
self.members.insert(member.to_string());
668+
}
669+
670+
/// Anchor for a bare `{member}` reference, if `member` is documented on this page.
671+
///
672+
/// Overloads share the base heading slug; the first heading owns it.
673+
fn member_anchor(&self, member: &str) -> Option<String> {
674+
self.members.contains(member).then(|| slug_anchor_segment(member))
675+
}
676+
677+
/// Anchor for a qualified `{Contract-member[-params...]}` reference, if `member` is
678+
/// documented on this page.
679+
fn xref_member_anchor(&self, part: &str) -> Option<String> {
680+
let member = part.split('-').find(|piece| !piece.is_empty())?;
681+
self.members.contains(member).then(|| xref_part_anchor(part))
682+
}
683+
}
684+
645685
/// Escape a string for use as a markdown link label.
646686
///
647687
/// Prevents MDX from treating user-controlled NatSpec label text as JSX or
@@ -654,7 +694,17 @@ fn escape_link_label(s: &str) -> String {
654694
///
655695
/// Matches the legacy pattern: `{[xref-]Ident[-part]}[label]` where `label` defaults
656696
/// to `Ident`.
657-
pub fn replace_inline_links(text: &str, name_to_page: &NameToPage, current_page: &Path) -> String {
697+
///
698+
/// Resolution prefers lexical proximity: a reference naming a member of the current
699+
/// contract (`{member}`, or `{Contract-member}` where `Contract` is the current
700+
/// contract) becomes an anchor-only link within the page; everything else goes
701+
/// through the global `name_to_page` index.
702+
pub fn replace_inline_links(
703+
text: &str,
704+
name_to_page: &NameToPage,
705+
current_page: &Path,
706+
local: Option<&LocalMembers>,
707+
) -> String {
658708
let mut out = String::with_capacity(text.len());
659709
let bytes = text.as_bytes();
660710
let mut i = 0;
@@ -671,6 +721,31 @@ pub fn replace_inline_links(text: &str, name_to_page: &NameToPage, current_page:
671721
lookup_name
672722
};
673723

724+
// Same-contract references resolve to anchor-only links: a bare
725+
// `{member}` documented on this page, or `{Contract-member}` where
726+
// `Contract` is the contract being rendered.
727+
if let Some(local) = local {
728+
let anchor = match part {
729+
None => local.member_anchor(lookup_name),
730+
Some(member) if lookup_name == local.name => {
731+
local.xref_member_anchor(member)
732+
}
733+
Some(_) => None,
734+
};
735+
if let Some(anchor) = anchor
736+
&& !anchor.is_empty()
737+
{
738+
let default_display = match part {
739+
Some(member) => format!("{lookup_name}.{member}"),
740+
None => lookup_name.to_string(),
741+
};
742+
let display = escape_link_label(label.unwrap_or(&default_display));
743+
out.push_str(&format!("[{display}](#{anchor})"));
744+
i += end;
745+
continue;
746+
}
747+
}
748+
674749
if let Some(candidates) = name_to_page.get(lookup_name) {
675750
let page = resolve_page(candidates, current_page);
676751
let mut link = page_link(page, current_page);
@@ -845,11 +920,85 @@ mod tests {
845920
"See {xref-ERC721-_safeMint-address-uint256-}.",
846921
&name_to_page,
847922
Path::new("src/contract.Child.mdx"),
923+
None,
848924
);
849925

850926
assert_eq!(
851927
out,
852928
"See [ERC721._safeMint-address-uint256-](/src/contract.ERC721#_safemint-address-uint256)."
853929
);
854930
}
931+
932+
#[test]
933+
fn same_contract_member_links_anchor_only() {
934+
let name_to_page = NameToPage::new();
935+
let mut local = LocalMembers::new("ECDSA");
936+
local.insert("toEthSignedMessageHash");
937+
local.insert("tryRecover");
938+
939+
// Bare member reference -> anchor-only link.
940+
let out = replace_inline_links(
941+
"then calling {toEthSignedMessageHash} on it.",
942+
&name_to_page,
943+
Path::new("src/library.ECDSA.mdx"),
944+
Some(&local),
945+
);
946+
assert_eq!(out, "then calling [toEthSignedMessageHash](#toethsignedmessagehash) on it.");
947+
948+
// `{Contract-member}` self-reference -> anchor-only link.
949+
let out = replace_inline_links(
950+
"Overload of {ECDSA-tryRecover} that ...",
951+
&name_to_page,
952+
Path::new("src/library.ECDSA.mdx"),
953+
Some(&local),
954+
);
955+
assert_eq!(out, "Overload of [ECDSA.tryRecover](#tryrecover) that ...");
956+
957+
// Unknown member still falls back to inline code.
958+
let out = replace_inline_links(
959+
"See {unknownMember}.",
960+
&name_to_page,
961+
Path::new("src/library.ECDSA.mdx"),
962+
Some(&local),
963+
);
964+
assert_eq!(out, "See `unknownMember`.");
965+
966+
// Unknown qualified self-reference should not create a broken same-page anchor.
967+
let out = replace_inline_links(
968+
"See {ECDSA-doesNotExist}.",
969+
&name_to_page,
970+
Path::new("src/library.ECDSA.mdx"),
971+
Some(&local),
972+
);
973+
assert_eq!(out, "See `ECDSA`.");
974+
}
975+
976+
#[test]
977+
fn local_member_wins_over_global_name() {
978+
// A top-level item elsewhere shares the member's name; lexical
979+
// proximity resolves to the same-page anchor, not the other page.
980+
let mut name_to_page = NameToPage::new();
981+
name_to_page
982+
.by_name
983+
.insert("transfer".to_string(), vec![PathBuf::from("src/other/contract.transfer.mdx")]);
984+
let mut local = LocalMembers::new("Token");
985+
local.insert("transfer");
986+
987+
let out = replace_inline_links(
988+
"Calls {transfer}.",
989+
&name_to_page,
990+
Path::new("src/contract.Token.mdx"),
991+
Some(&local),
992+
);
993+
assert_eq!(out, "Calls [transfer](#transfer).");
994+
995+
// Without local context the global index still resolves.
996+
let out = replace_inline_links(
997+
"Calls {transfer}.",
998+
&name_to_page,
999+
Path::new("src/contract.Token.mdx"),
1000+
None,
1001+
);
1002+
assert_eq!(out, "Calls [transfer](/src/other/contract.transfer).");
1003+
}
8551004
}

crates/doc/src/render.rs

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,29 @@ fn render_contract<'ast, 'gcx>(
196196
deployments: &[Deployment],
197197
) -> String {
198198
let name = c.name.as_str();
199-
let comments = collect_comments(docs, name_to_page, page_path);
199+
200+
// Index the members rendered as headings on this page so `{member}` and
201+
// `{Contract-member}` self-references resolve to anchor-only links.
202+
let mut local = hir_ext::LocalMembers::new(name);
203+
for member in c.body.iter() {
204+
match &member.kind {
205+
ItemKind::Variable(v) => {
206+
if let Some(n) = v.name {
207+
local.insert(n.as_str());
208+
}
209+
}
210+
ItemKind::Function(f) => local.insert(&function_heading(f)),
211+
ItemKind::Event(e) => local.insert(e.name.as_str()),
212+
ItemKind::Error(e) => local.insert(e.name.as_str()),
213+
ItemKind::Struct(s) => local.insert(s.name.as_str()),
214+
ItemKind::Enum(e) => local.insert(e.name.as_str()),
215+
ItemKind::Udvt(u) => local.insert(u.name.as_str()),
216+
_ => {}
217+
}
218+
}
219+
let local = Some(&local);
220+
221+
let comments = collect_comments(docs, name_to_page, page_path, local);
200222
let mut out = String::new();
201223
write_frontmatter(&mut out, name, first_notice(&comments).as_deref());
202224
writeln!(out, "# {name}").unwrap();
@@ -251,14 +273,14 @@ fn render_contract<'ast, 'gcx>(
251273
let vname = v.name.map(|n| n.as_str().to_string()).unwrap_or_default();
252274
writeln!(out, "### {vname}").unwrap();
253275
writeln!(out).unwrap();
254-
let mut c = collect_comments(docs, name_to_page, page_path);
276+
let mut c = collect_comments(docs, name_to_page, page_path, local);
255277
// Attempt @inheritdoc resolution for public state variables.
256278
let inherited = inheritdoc_base(docs).and_then(|base| {
257279
hir_id.and_then(|cid| hir_ext::resolve_inheritdoc_var(gcx, cid, &vname, &base))
258280
});
259281
if let Some(ref base_doc) = inherited {
260282
let sanitize =
261-
|s: &str| hir_ext::replace_inline_links(s, name_to_page, page_path);
283+
|s: &str| hir_ext::replace_inline_links(s, name_to_page, page_path, local);
262284
if c.notices.is_empty() {
263285
let inherited_notices: Vec<String> =
264286
base_doc.notices.iter().map(|s| sanitize(s)).collect();
@@ -357,6 +379,7 @@ fn render_contract<'ast, 'gcx>(
357379
ctx,
358380
name_to_page,
359381
page_path,
382+
local,
360383
inherited.as_ref(),
361384
);
362385
}
@@ -368,7 +391,7 @@ fn render_contract<'ast, 'gcx>(
368391
for (span, e, docs) in &events {
369392
writeln!(out, "### {}", e.name.as_str()).unwrap();
370393
writeln!(out).unwrap();
371-
let c = collect_comments(docs, name_to_page, page_path);
394+
let c = collect_comments(docs, name_to_page, page_path, local);
372395
write_comment_block(&mut out, &c);
373396
write_code_block(&mut out, &ctx.dedented_snippet(*span));
374397
write_param_table(&mut out, "Parameters", &e.parameters, &c, ctx);
@@ -381,7 +404,7 @@ fn render_contract<'ast, 'gcx>(
381404
for (span, e, docs) in &errors {
382405
writeln!(out, "### {}", e.name.as_str()).unwrap();
383406
writeln!(out).unwrap();
384-
let c = collect_comments(docs, name_to_page, page_path);
407+
let c = collect_comments(docs, name_to_page, page_path, local);
385408
write_comment_block(&mut out, &c);
386409
write_code_block(&mut out, &ctx.dedented_snippet(*span));
387410
write_param_table(&mut out, "Parameters", &e.parameters, &c, ctx);
@@ -394,7 +417,7 @@ fn render_contract<'ast, 'gcx>(
394417
for (span, s, docs) in &structs {
395418
writeln!(out, "### {}", s.name.as_str()).unwrap();
396419
writeln!(out).unwrap();
397-
let c = collect_comments(docs, name_to_page, page_path);
420+
let c = collect_comments(docs, name_to_page, page_path, local);
398421
write_comment_block(&mut out, &c);
399422
write_code_block(&mut out, &ctx.dedented_snippet(*span));
400423
write_struct_properties_table(&mut out, s.fields, &c, ctx);
@@ -407,7 +430,7 @@ fn render_contract<'ast, 'gcx>(
407430
for (span, e, docs) in &enums {
408431
writeln!(out, "### {}", e.name.as_str()).unwrap();
409432
writeln!(out).unwrap();
410-
let c = collect_comments(docs, name_to_page, page_path);
433+
let c = collect_comments(docs, name_to_page, page_path, local);
411434
write_comment_block(&mut out, &c);
412435
write_code_block(&mut out, &ctx.dedented_snippet(*span));
413436
write_enum_variants_table(&mut out, e.variants, &c);
@@ -420,7 +443,7 @@ fn render_contract<'ast, 'gcx>(
420443
for (span, u, docs) in &udvts {
421444
writeln!(out, "### {}", u.name.as_str()).unwrap();
422445
writeln!(out).unwrap();
423-
let c = collect_comments(docs, name_to_page, page_path);
446+
let c = collect_comments(docs, name_to_page, page_path, local);
424447
write_comment_block(&mut out, &c);
425448
write_code_block(&mut out, &format!("{};", ctx.dedented_snippet(*span)));
426449
}
@@ -440,14 +463,14 @@ fn render_free_functions(
440463
git_url: Option<&str>,
441464
) -> String {
442465
let title = if name.is_empty() { "function" } else { name };
443-
let first_comments = collect_comments(overloads[0].2, name_to_page, page_path);
466+
let first_comments = collect_comments(overloads[0].2, name_to_page, page_path, None);
444467
let mut out = String::new();
445468
write_frontmatter(&mut out, title, first_notice(&first_comments).as_deref());
446469
writeln!(out, "# {title}").unwrap();
447470
writeln!(out).unwrap();
448471
write_git_source(&mut out, git_url);
449472
for (span, f, docs) in overloads {
450-
render_function_section(&mut out, *span, f, docs, ctx, name_to_page, page_path, None);
473+
render_function_section(&mut out, *span, f, docs, ctx, name_to_page, page_path, None, None);
451474
}
452475
out
453476
}
@@ -472,7 +495,7 @@ fn render_constants(
472495
let name = v.name.map(|n| n.as_str().to_string()).unwrap_or_else(|| "_".to_string());
473496
writeln!(out, "## {name}").unwrap();
474497
writeln!(out).unwrap();
475-
let c = collect_comments(docs, name_to_page, page_path);
498+
let c = collect_comments(docs, name_to_page, page_path, None);
476499
write_comment_block(&mut out, &c);
477500
write_code_block(&mut out, &ctx.dedented_snippet(*span));
478501
}
@@ -491,7 +514,7 @@ fn render_struct<'ast>(
491514
git_url: Option<&str>,
492515
) -> String {
493516
let name = s.name.as_str();
494-
let c = collect_comments(docs, name_to_page, page_path);
517+
let c = collect_comments(docs, name_to_page, page_path, None);
495518
let mut out = String::new();
496519
write_frontmatter(&mut out, name, first_notice(&c).as_deref());
497520
writeln!(out, "# {name}").unwrap();
@@ -513,7 +536,7 @@ fn render_enum<'ast>(
513536
git_url: Option<&str>,
514537
) -> String {
515538
let name = e.name.as_str();
516-
let c = collect_comments(docs, name_to_page, page_path);
539+
let c = collect_comments(docs, name_to_page, page_path, None);
517540
let mut out = String::new();
518541
write_frontmatter(&mut out, name, first_notice(&c).as_deref());
519542
writeln!(out, "# {name}").unwrap();
@@ -535,7 +558,7 @@ fn render_udvt<'ast>(
535558
git_url: Option<&str>,
536559
) -> String {
537560
let name = u.name.as_str();
538-
let c = collect_comments(docs, name_to_page, page_path);
561+
let c = collect_comments(docs, name_to_page, page_path, None);
539562
let mut out = String::new();
540563
write_frontmatter(&mut out, name, first_notice(&c).as_deref());
541564
writeln!(out, "# {name}").unwrap();
@@ -556,7 +579,7 @@ fn render_error<'ast>(
556579
git_url: Option<&str>,
557580
) -> String {
558581
let name = e.name.as_str();
559-
let c = collect_comments(docs, name_to_page, page_path);
582+
let c = collect_comments(docs, name_to_page, page_path, None);
560583
let mut out = String::new();
561584
write_frontmatter(&mut out, name, first_notice(&c).as_deref());
562585
writeln!(out, "# {name}").unwrap();
@@ -578,7 +601,7 @@ fn render_event<'ast>(
578601
git_url: Option<&str>,
579602
) -> String {
580603
let name = e.name.as_str();
581-
let c = collect_comments(docs, name_to_page, page_path);
604+
let c = collect_comments(docs, name_to_page, page_path, None);
582605
let mut out = String::new();
583606
write_frontmatter(&mut out, name, first_notice(&c).as_deref());
584607
writeln!(out, "# {name}").unwrap();
@@ -600,6 +623,7 @@ fn render_function_section(
600623
ctx: &Ctx<'_>,
601624
name_to_page: &NameToPage,
602625
page_path: &Path,
626+
local: Option<&hir_ext::LocalMembers>,
603627
inherited: Option<&hir_ext::InheritedDoc>,
604628
) {
605629
let heading = function_heading(f);
@@ -609,10 +633,10 @@ fn render_function_section(
609633
}
610634
writeln!(out, "### {heading}").unwrap();
611635
writeln!(out).unwrap();
612-
let mut c = collect_comments(docs, name_to_page, page_path);
636+
let mut c = collect_comments(docs, name_to_page, page_path, local);
613637
// Merge inherited natspec for missing tags.
614638
if let Some(inherited) = inherited {
615-
let sanitize = |s: &str| hir_ext::replace_inline_links(s, name_to_page, page_path);
639+
let sanitize = |s: &str| hir_ext::replace_inline_links(s, name_to_page, page_path, local);
616640
let inherited_notices: Vec<String> =
617641
inherited.notices.iter().map(|s| sanitize(s)).collect();
618642
let inherited_devs: Vec<String> = inherited.devs.iter().map(|s| sanitize(s)).collect();
@@ -724,6 +748,7 @@ fn collect_comments(
724748
docs: &DocComments<'_>,
725749
name_to_page: &NameToPage,
726750
page_path: &Path,
751+
local: Option<&hir_ext::LocalMembers>,
727752
) -> CommentData {
728753
let mut data = CommentData {
729754
titles: Vec::new(),
@@ -779,7 +804,7 @@ fn collect_comments(
779804
}
780805

781806
// Apply inline {Ident} -> markdown link replacement.
782-
let content = hir_ext::replace_inline_links(trimmed, name_to_page, page_path);
807+
let content = hir_ext::replace_inline_links(trimmed, name_to_page, page_path, local);
783808

784809
if is_continuation && !prev_doc_was_blank {
785810
let appended = match last_section {

0 commit comments

Comments
 (0)