Fix #2058: Add links to configuration page

[Jamison929611]

Summary

• generate local Javadoc links for resolved custom configuration types, including nested types
• add stable anchors for configuration keys and link configuration-field references to those anchors
• cover intermediate metadata and rendered Markdown with focused doclet tests

Fixes #2058

Verification

• Focused ConfigurationCollectorDocletTest: 4 tests, 0 failures or errors
• Reactor package through maven-resolver-tools with the focused test and Checkstyle, Spotless, and RAT
• spotless:check and checkstyle:check for maven-resolver-tools
• git diff --check
• Full mvn verify: the broader Windows run is blocked in the unchanged maven-resolver-transport-file tests because the current account cannot create symbolic links
• Integration tests with the run-its profile were not run

Contribution checklist

• This pull request addresses one issue without unrelated changes.
• The commit has a meaningful subject and body.
• Unit tests cover the behavioral changes.
• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004.

[gnodet]

Clean addition of Javadoc type links and cross-reference anchor links for the configuration page, correctly addressing #2058. The implementation is well-structured:

• getJavadocUrl correctly traverses PackageElement to handle nested types
• resolveReferencedField + getConfigurationKey properly resolve {@link #FIELD} references to configuration key names
• HTML anchors use dot-separated configuration key identifiers, which are valid id values
• Velocity templates correctly treat empty strings as falsy for conditional rendering
• Tests cover both metadata extraction and full-pipeline rendered Markdown output

One minor observation: the !typeElement.getQualifiedName().toString().startsWith("java.") filter excludes JDK types from Javadoc links, but javax.*/jakarta.*/third-party types would still get a relative apidocs/ link. In practice this is a non-issue since configuration types here are internal enums/classes, but worth noting for future-proofing.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[Jamison929611]

Thanks for the review and approval. The PR appears ready from my side. Could you please merge it when you have a chance? If any additional checks or changes are still required, please let me know. Thanks!

[kwin]

Looks good as first draft, however this requires some more work to distinguish between internal and external links (and potentially skip unavailable javadoc links) as well as supporting links to elements and package/modules next to the one for types.

[Jamison929611]

@kwin The requested changes are implemented in 0d7e9e9, and each inline review thread has been updated with details. Could you please re-review when convenient? The PR is now mergeable; the Verify workflow is awaiting maintainer approval.

[gnodet]

Re-review (2nd pass): Substantial expansion addressing all three points of @kwin's feedback:

1. ✅ Uses JavadocLinkGenerator from maven-plugin-tools-api for version-aware link formatting (Javadoc 8 vs 10+)
2. ✅ Internal/external link distinction with configurable base URLs (--internal-javadoc-url, --external-javadoc-url)
3. ✅ Supports types, fields, methods, constructors, packages, and modules with visibility filtering

The priority ordering in visitLink is correct — configuration-key cross-references take precedence over Javadoc links, which fall back to plain text. Test coverage is comprehensive (6 test methods).

One minor observation below about dependency documentation.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Minor: maven-settings isn't directly imported by any source file — it appears to be needed at runtime because maven-core (its usual provider) is excluded from maven-plugin-tools-api above. A brief comment explaining this transitive dependency rationale would help future maintainers understand why it's explicitly declared.

[Jamison929611]

Documented in 79a8ef4: JavadocLinkGenerator's constructor signature references Settings, so maven-settings is declared directly because maven-core is excluded.

[gnodet]

Re-review (3rd pass): The maven-settings dependency comment addresses the finding from the previous review. The explanation is clear — JavadocLinkGenerator's constructor references Settings, and maven-core (its usual transitive provider) is excluded above.

All previously raised items are resolved. The implementation comprehensively addresses #2058 with proper internal/external link distinction, configurable URLs, and support for all element kinds.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[kwin]

This requires additional arguments now to actually consider the javadoc. Also there is a ordering problem as the aggregate javadoc is created after populating the configuration.

[Jamison929611]

Addressed in 21bcb9a. The render-configuration-page execution now passes the internal Javadoc URL, the Javadoc version, and the Java 21 external API URL. The shared POM properties keep those values aligned with maven-javadoc-plugin. For the ordering issue, configuration collection does not require the aggregate output to exist: internal references are validated against the discovered src/main/java roots while emitting apidocs/ links, so the aggregate Javadocs can still be generated later by the site lifecycle. The machine-readable properties/YAML execution remains unchanged.

[Jamison929611]

@kwin, could you please take another look when convenient?

[gnodet]

Re-review (4th pass): Both of @kwin's concerns are properly addressed:

1. ✅ Additional arguments: the render-configuration-page execution now passes --internal-javadoc-url, --internal-javadoc-version, and --external-javadoc-url with values from shared POM properties (javadocVersion, javadocJavaApiUrl) that stay in sync with the maven-javadoc-plugin configuration.

2. ✅ Ordering problem: the doclet discovers src/main/java source roots from the processed files (via findMainJavaSourceRoot) and validates internal types against those source trees. This means the configuration page can be generated before the aggregate Javadoc exists — the relative apidocs/ links resolve correctly once the site lifecycle generates the aggregate output.

The shared POM properties eliminate the previously hardcoded "21" and Java API URL, keeping both the Javadoc plugin and the configuration doclet aligned.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[kwin]

I still see no javadoc links in maven-resolver/target/generated-site/markdown/configuration.md. Most probably this is related to the fact that the referenced classes are from a foreign module (i.e. not part of src/main/java within maven-resolver-tools). The logic to check if a type is part of the aggregated javadoc or not need to be reworked.

[Jamison929611]

Fixed in 4f21cd4.

The missing link was caused by internalJavadocSourceRoots being derived only from source files that declare configuration keys. That excluded sibling reactor modules whose types are part of the aggregate Javadoc.

The collector now:

• keeps the doclet compilation units limited to marker-bearing configuration sources;
• derives aggregate Javadoc membership from all reactor src/main/java source roots;
• detects those roots using Path operations so the collection also works on Windows.

I added an end-to-end regression test where a configuration source in one module references ConfigurationProperties.HttpVersion from a sibling module. The targeted reactor package run passes with 9 tests, and the real generated target/generated-site/markdown/configuration.md now contains both the ConfigurationProperties.HttpVersion Javadoc link and the #aether.transport.http.version configuration anchor.

@kwin, could you please take another look?

[kwin]

@Jamison929611 That logic does not work for configuration pages being created for Maven itself: https://github.com/apache/maven/blob/7dc59176c0b0cb09198bef11425b97ece582c606/pom.xml#L1067

[gnodet]

Re-review (5th pass): The cross-module source-root detection fix correctly addresses @kwin's feedback.

The core change: javaSourceFiles now collects ALL .java files under src/main/java across the reactor (not just configuration-marker-bearing sources), and internalJavadocSourceRoots are derived from this broader set. This means types from sibling modules (e.g., ConfigurationProperties.HttpVersion from maven-resolver-api) are correctly identified as internal and receive proper apidocs/ links.

The isInternalJavadocElement() fallback chain is sound: tries docTrees.getPath() first (for types in doclet compilation units), then falls back to isTypeInInternalSourceTree() (filesystem check across all discovered source roots).

The new test linksTypesFromSiblingAggregateJavadocSourceRoots directly validates the cross-module scenario with a configuration source in one module referencing a type from a sibling module.

Good defensive improvements: Path.endsWith() replacing string-based /src/main/java/ check for Windows compatibility, and replace('\\', '/') for path normalization.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[Jamison929611]

Addressed in 56e2323.

I verified the Maven execution you linked: it intentionally keeps api/maven-api-core as the positional configuration source root, while its aggregate Javadocs cover the wider reactor.

CollectConfiguration now exposes a repeatable --internal-javadoc-source-tree option. Configuration declarations are still collected only below the positional root; source roots used to validate aggregate Javadoc links are discovered independently from the configured tree(s). If the option is omitted, it defaults to the positional root, preserving the existing Resolver invocation.

For the Maven execution, the existing positional argument can remain unchanged and the page execution can add:

<argument>--internal-javadoc-source-tree=${project.basedir}</argument>

The regression test now invokes the public CLI with a narrow configuration-module positional root and a separate reactor-wide source tree, rather than placing both modules below the positional root. The full ConfigurationCollectorDocletTest suite passes (9/9), the 26-module package build succeeds, and Resolver's real generated configuration page still contains the HttpVersion Javadoc link and the configuration anchor using the default behavior.

@kwin, could you please take another look?

[gnodet]

Re-review (6th pass): The repeatable --internal-javadoc-source-tree option is the right approach for the Maven cross-reactor case @kwin flagged — it separates the configuration declaration root (positional argument) from the broader aggregate Javadoc source trees, which is exactly what Maven's own configuration page build needs.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[kwin]

This deserves some more explanation. How exactly are the directories traversed to find source roots?

[Jamison929611]

Clarified in 9620dee. Each supplied tree is traversed recursively with Files.walk (without following symbolic links by default). Java files are retained only when their nearest ancestor path ends in src/main/java, and those distinct ancestors become the aggregate Javadoc source roots. The option may be repeated; when omitted, discovery uses the positional source processing tree. The CLI help now documents the recursive traversal, ancestor matching, deduplication, repeatability, and default.