Skip to content

Fix broken links to external base types and misrouted own-assembly crefs - #60

Merged
Malcolmnixon merged 6 commits into
mainfrom
fix/external-link-misrouting
Oct 15, 2026
Merged

Malcolmnixon merged 6 commits into
mainfrom
fix/external-link-misrouting

Conversation

@Malcolmnixon

Copy link
Copy Markdown
Member

Fixes two link regressions reported against 0.5.9 (55 broken links across the Plexus packages).

1. Inherited-from headings linked external base types (53 links). Base types resolved from another assembly (e.g. Avalonia's StyledElement, whose base chain stays inside Avalonia's module) are scoped to their own ModuleDefinition, so they were treated as intra-assembly and linked into our own api/ tree. TypeLinkResolver now receives the documented module and treats every other module as external: plain text unless the assembly is mapped with ApiMarkExternalLinks.

2. Own-assembly cref linked into the mapped package (2 links). A cref to a type/member of the documented assembly that was not emitted fell through to the external mapping, which matches by namespace prefix. Such crefs now never link externally (CrefTargetResolver.DeclaresCrefType).

Tests. New fixtures and tests for external base chains (mapped and unmapped), own-assembly cref with an overlapping mapping, unit tests for the new resolver behavior, and a regression test that every relative link on every page resolves to an emitted page (fails without the fix). Design, requirements and verification docs updated.

Reviews: code-review clean; formal reviews of the affected review-sets approved. Lint clean.

Not included: a generation-time link self-check (suggested by the reporter) - a separate feature.

Inherited-member headings linked base types resolved from other assemblies
as if they were pages of the documented assembly, because those types are
scoped to their own ModuleDefinition. TypeLinkResolver now knows the
documented module and treats every other module as external.

Crefs into the documented assembly that were not emitted fell through to
the namespace-prefix ExternalLinks mapping. They now never link externally.

Add regression tests, including a check that every relative link on every
page resolves to an emitted page, plus design, requirements and verification
updates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Moderate resolver issues involving exact type matching and constructed generic crefs remain unresolved.

2 open findings
What changed in this PR

Fixes .NET documentation link routing for external inheritance chains and own-assembly crefs.

Changes:

  • Distinguishes documented and foreign modules.
  • Prevents own-assembly crefs from using external mappings.
  • Adds fixtures, regression tests, and documentation updates.
File Summary
test/​ApiMark.DotNet.Tests/​TypeLinkResolverTests.cs Tests module-aware type linking.
test/​ApiMark.DotNet.Tests/​DotNetEmitterInheritedMembersTests.cs Tests external inheritance links.
test/​ApiMark.DotNet.Tests/​DotNetEmitterInheritanceDocsTests.cs Tests inheritance and cref regressions.
test/​ApiMark.DotNet.Tests/​CrefTargetResolverTests.cs Tests own-assembly cref detection.
test/​ApiMark.DotNet.Fixtures/​InheritedMembersShapes.cs Adds inheritance fixture usage.
test/​ApiMark.DotNet.Fixtures.External/​ExternalChainClasses.cs Adds external inheritance fixtures.
src/​ApiMark.DotNet/​XmlDocReader.cs Prevents misrouted own-assembly crefs.
src/​ApiMark.DotNet/​TypeLinkResolver.cs Handles foreign module scopes.
src/​ApiMark.DotNet/​DotNetGenerator.cs Supplies the documented module.
src/​ApiMark.DotNet/​CrefTargetResolver.cs Detects own-assembly cref targets.
docs/​verification/​api-mark-dot-net/​xml-doc-reader.md Documents XML documentation verification.
docs/​verification/​api-mark-dot-net/​type-link-resolver.md Documents type-link verification.
docs/​verification/​api-mark-dot-net/​cref-target-resolver.md Documents cref verification.
docs/​reqstream/​api-mark-dot-net/​xml-doc-reader.yaml Adds XML-doc requirements traceability.
docs/​reqstream/​api-mark-dot-net/​type-link-resolver.yaml Adds type-link requirements traceability.
docs/​reqstream/​api-mark-dot-net/​cref-target-resolver.yaml Adds cref requirements traceability.
docs/​design/​api-mark-dot-net/​xml-doc-reader.md Documents XML-doc behavior.
docs/​design/​api-mark-dot-net/​type-link-resolver.md Documents module-aware linking.
docs/​design/​api-mark-dot-net/​dot-net-generator.md Documents generator wiring.
docs/​design/​api-mark-dot-net/​cref-target-resolver.md Documents cref ownership behavior.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/design/api-mark-dot-net/type-link-resolver.md Outdated
Comment thread src/ApiMark.DotNet/TypeLinkResolver.cs
CppFixtureFileNamingTests builds the shared clang fixture but was not in the
ClangEnvironment collection, so it could race tests that set
APIMARK_CLANG_TIMEOUT_MS to an invalid value (failed on macOS CI).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 21:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Two moderate CrefTargetResolver issues remain unresolved.

0 open findings

2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid prefix scans for type crefs

src/​ApiMark.DotNet/​CrefTargetResolver.cs:139

This prefix scan also runs for T: crefs, so an external nested type can be misclassified as an own-assembly cref when the documented assembly happens to contain the same name as its outer type. For example, with local Ns.Outer and mapped external Ns.Outer.Inner, T:Ns.Outer.Inner matches the local T:Ns.Outer prefix and the new guard suppresses the valid external link. Only member kinds should search shorter declaring-type prefixes; a T: cref should require an exact indexed type (and malformed/unsupported kinds should return false).

🧠 Review effort: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 22:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The explicit-interface member fallback can still misroute own-assembly crefs through external mappings.

1 open finding

🧠 Review effort: Lite

Comment thread src/ApiMark.DotNet/CrefTargetResolver.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved blocking issues remain.

0 open findings

1 resolved since last review

🧠 Review effort: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 22:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The link-integrity regression test must also inspect signature text.

0 open findings

🧠 Review effort: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings October 15, 2026 23:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review comments were identified, and the changes include regression coverage.

0 open findings

🧠 Review effort: Lite

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.

2 participants