-
-
Notifications
You must be signed in to change notification settings - Fork 973
docs: fix broken external and internal links in Groovydocs #16005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.0.x
Are you sure you want to change the base?
Changes from all commits
2c6c661
b168d32
978a571
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.grails.doc.gradle | ||
|
|
||
| import org.gradle.api.DefaultTask | ||
| import org.gradle.api.file.DirectoryProperty | ||
| import org.gradle.api.tasks.InputDirectory | ||
| import org.gradle.api.tasks.TaskAction | ||
|
|
||
| /** | ||
| * A task that audits generated Groovydocs for malformed navigation links. | ||
| * Specifically targets 'phantom' links and relative path issues in navigation files | ||
| * like overview-summary.html, deprecated-list.html, and help-doc.html. | ||
| */ | ||
| abstract class AuditGroovydocLinksTask extends DefaultTask { | ||
|
|
||
| @InputDirectory | ||
| abstract DirectoryProperty getApiDocsDir() | ||
|
|
||
| @TaskAction | ||
| void auditLinks() { | ||
| File apiDir = apiDocsDir.get().asFile | ||
| if (!apiDir.exists()) { | ||
| logger.warn "API documentation directory does not exist: ${apiDir.absolutePath}" | ||
| return | ||
| } | ||
|
|
||
| List<String> violations = GroovydocLinkAuditor.findViolations(apiDir) | ||
|
|
||
| if (!violations.isEmpty()) { | ||
| violations.take(10).each { logger.error(it) } | ||
| if (violations.size() > 10) logger.error("... and ${violations.size() - 10} more") | ||
| throw new org.gradle.api.GradleException("Found ${violations.size()} malformed links in Groovydoc. Please fix the source issue rather than patching. See logs for details.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import |
||
| } else { | ||
| logger.lifecycle "No malformed Groovydoc links found in ${apiDir.absolutePath}" | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.grails.doc.gradle | ||
|
|
||
| import java.nio.file.Files | ||
| import java.nio.file.Path | ||
| import java.util.regex.Matcher | ||
| import java.util.regex.Pattern | ||
|
|
||
| /** | ||
| * Scans generated Groovydoc HTML for malformed navigation and inner-class | ||
| * links. Kept free of the Gradle API so it can be unit tested directly - | ||
| * {@code build-logic/docs-core} deliberately keeps Gradle classes off the | ||
| * test compile classpath. | ||
| */ | ||
| class GroovydocLinkAuditor { | ||
|
|
||
| // Patterns common to Groovydoc navigation failures | ||
| private static final Map<Pattern, String> NAV_LINK_PATTERNS = [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These nav-link patterns can never match real Groovydoc output. The templates in The inner-class check below is the genuinely valid part: class cross-reference links are single-quoted in the templates, which is where the Suggest dropping these patterns (or reworking them to detect links whose resolved target doesn't exist, like the inner-class check does). Two related smells that go away with them: the map values (the |
||
| (Pattern.compile(/href='([^']+?)\/deprecated-list\.html'/)): "href='deprecated-list.html'", | ||
| (Pattern.compile(/href='([^']+?)\/help-doc\.html'/)): "href='help-doc.html'", | ||
| (Pattern.compile(/href='([^']+?)\/index-all\.html'/)): "href='index-all.html'", | ||
| (Pattern.compile(/href='([^']+?)\/overview-summary\.html'/)): "href='overview-summary.html'" | ||
| ].asImmutable() | ||
|
|
||
| // Inner class path issues like Query/Order.Direction.html -> Query.Order.Direction.html | ||
| private static final Pattern INNER_CLASS_LINK_PATTERN = | ||
| Pattern.compile(/href='([^']+?)\/([A-Z][A-Za-z0-9_]*?)\/([A-Z][A-Za-z0-9_.]*?\.html)'/) | ||
|
|
||
| static List<String> findViolations(File apiDir) { | ||
| List<String> violations = [] | ||
|
|
||
| apiDir.eachFileRecurse { File file -> | ||
| if (file.name.endsWith('.html')) { | ||
| violations.addAll(findViolationsInFile(file)) | ||
| } | ||
| } | ||
|
|
||
| violations | ||
| } | ||
|
|
||
| private static List<String> findViolationsInFile(File file) { | ||
| List<String> violations = [] | ||
| String content = file.text | ||
|
|
||
| NAV_LINK_PATTERNS.each { Pattern pattern, String replacement -> | ||
| Matcher matcher = pattern.matcher(content) | ||
| if (matcher.find()) { | ||
| violations << "Malformed nav link in ${file.name}: ${matcher.group(0)}".toString() | ||
| } | ||
| } | ||
|
Comment on lines
+62
to
+67
|
||
|
|
||
| Matcher innerMatcher = INNER_CLASS_LINK_PATTERN.matcher(content) | ||
| while (innerMatcher.find()) { | ||
| String relPath = innerMatcher.group(1) | ||
| String outer = innerMatcher.group(2) | ||
| String inner = innerMatcher.group(3) | ||
|
|
||
| Path currentPath = file.toPath().parent | ||
| Path targetPath = currentPath.resolve(relPath).resolve("${outer}.${inner}").normalize() | ||
|
|
||
| if (Files.exists(targetPath)) { | ||
| violations << "Malformed inner class link in ${file.name}: ${innerMatcher.group(0)}".toString() | ||
| } | ||
| } | ||
|
|
||
| violations | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.grails.doc.gradle | ||
|
|
||
| import spock.lang.Specification | ||
| import spock.lang.TempDir | ||
|
|
||
| class GroovydocLinkAuditorSpec extends Specification { | ||
|
|
||
| @TempDir | ||
| File apiDir | ||
|
|
||
| void "reports no violations for clean navigation and inner class links"() { | ||
| given: | ||
| writeHtml('index.html', """ | ||
| <a href='deprecated-list.html'>Deprecated</a> | ||
| <a href='help-doc.html'>Help</a> | ||
| <a href='SomePackage.SomeClass.html'>SomeClass</a> | ||
| """) | ||
|
|
||
| expect: | ||
| GroovydocLinkAuditor.findViolations(apiDir).empty | ||
| } | ||
|
|
||
| void "flags a navigation link that is nested under a relative path prefix"() { | ||
| given: | ||
| writeHtml('index.html', "<a href='../deprecated-list.html'>Deprecated</a>") | ||
|
|
||
| when: | ||
| List<String> violations = GroovydocLinkAuditor.findViolations(apiDir) | ||
|
|
||
| then: | ||
| violations.size() == 1 | ||
| violations[0].contains('deprecated-list.html') | ||
| violations[0].contains('index.html') | ||
| } | ||
|
|
||
| void "flags an inner class link using a slash-separated path when the dotted file actually exists"() { | ||
| given: 'the real generated file uses the dotted Groovydoc naming convention' | ||
| writeHtml('Query.Order.Direction.html', '<p>Direction</p>') | ||
| and: 'another page links to it using a malformed slash-separated path' | ||
| writeHtml('index.html', "<a href='./Query/Order.Direction.html'>Direction</a>") | ||
|
|
||
| when: | ||
| List<String> violations = GroovydocLinkAuditor.findViolations(apiDir) | ||
|
|
||
| then: | ||
| violations.size() == 1 | ||
| violations[0].contains('Query/Order.Direction.html') | ||
| } | ||
|
|
||
| void "does not flag a slash-separated inner class link when no dotted file exists to resolve to"() { | ||
| given: 'the referenced target was never generated, so this cannot be the known malformed-path case' | ||
| writeHtml('index.html', "<a href='./Query/Order.Direction.html'>Direction</a>") | ||
|
|
||
| expect: | ||
| GroovydocLinkAuditor.findViolations(apiDir).empty | ||
| } | ||
|
|
||
| private File writeHtml(String relativePath, String content) { | ||
| File file = new File(apiDir, relativePath) | ||
| file.parentFile.mkdirs() | ||
| file.text = content | ||
| file | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,16 +31,10 @@ dependencies { | |
| } | ||
|
|
||
| String resolveProjectVersion(String artifact) { | ||
| String version = configurations.runtimeClasspath | ||
| .resolvedConfiguration | ||
| .resolvedArtifacts | ||
| .find { | ||
| it.moduleVersion.id.name == artifact | ||
| }?.moduleVersion?.id?.version | ||
| if (!version) { | ||
| return null | ||
| def component = configurations.runtimeClasspath.incoming.resolutionResult.allComponents.find { | ||
| it.moduleVersion?.name == artifact | ||
| } | ||
| version | ||
| return component?.moduleVersion?.version | ||
| } | ||
|
|
||
| def configureGroovyDoc = tasks.register('configureGroovyDoc') { | ||
|
|
@@ -56,12 +50,35 @@ def configureGroovyDoc = tasks.register('configureGroovyDoc') { | |
| } | ||
| def springVersion = resolveProjectVersion('spring-core') | ||
| if (springVersion) { | ||
| links << [packages: 'org.springframework.core.', href: "https://docs.spring.io/spring-framework/docs/${springVersion}/javadoc-api/"] | ||
| links << [packages: 'org.springframework.', href: "https://docs.spring.io/spring-framework/docs/${springVersion}/javadoc-api/"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Groovydoc resolves Most-specific-first fixes it: move the |
||
| } | ||
| def springBootVersion = resolveProjectVersion('spring-boot') | ||
| if (springBootVersion) { | ||
| links << [packages: 'org.springframework.boot.', href: "https://docs.spring.io/spring-boot/docs/${springBootVersion}/api/"] | ||
| } | ||
| def hibernateVersion = resolveProjectVersion('hibernate-core') | ||
| if (hibernateVersion) { | ||
| def shortVersion = hibernateVersion.split('\\.').take(2).join('.') | ||
| links << [packages: 'org.hibernate.', href: "https://docs.jboss.org/hibernate/orm/${shortVersion}/javadocs/"] | ||
| } | ||
| def jakartaValidationVersion = resolveProjectVersion('jakarta.validation-api') | ||
| if (jakartaValidationVersion) { | ||
| links << [packages: 'jakarta.validation.', href: "https://jakarta.ee/specifications/bean-validation/3.0/apidocs/"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: these three Jakarta links gate on the resolved artifact version but hardcode the spec version in the URL ( |
||
| } | ||
| def jakartaPersistenceVersion = resolveProjectVersion('jakarta.persistence-api') | ||
| if (jakartaPersistenceVersion) { | ||
| links << [packages: 'jakarta.persistence.', href: "https://jakarta.ee/specifications/persistence/3.1/apidocs/"] | ||
| } | ||
| def jakartaServletVersion = resolveProjectVersion('jakarta.servlet-api') | ||
| if (jakartaServletVersion) { | ||
| links << [packages: 'jakarta.servlet.', href: "https://jakarta.ee/specifications/platform/10/apidocs/"] | ||
| } | ||
| links << [packages: 'org.grails.datastore.', href: "https://gorm.grails.org/latest/api/"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question on these three: since the data modules merged into this repo in 7.0,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, we host these directly now on our website, so we shoudlnt' refer to the older docs link |
||
| links << [packages: 'grails.gorm.', href: "https://gorm.grails.org/latest/api/"] | ||
| links << [packages: 'org.grails.gorm.', href: "https://gorm.grails.org/latest/api/"] | ||
| links << [packages: 'groovy.', href: "https://docs.groovy-lang.org/latest/html/gapi/"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| links << [packages: 'org.apache.groovy.', href: "https://docs.groovy-lang.org/latest/html/gapi/"] | ||
| links << [packages: 'org.codehaus.groovy.', href: "https://docs.groovy-lang.org/latest/html/gapi/"] | ||
| if (it.ext.has('groovydocLinks')) { | ||
| links.addAll(it.ext.groovydocLinks as List<Map<String, String>>) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import grails.doc.git.FetchTagsTask | |
| import grails.doc.dropdown.CreateReleaseDropDownTask | ||
| import grails.doc.gradle.PublishGuideTask | ||
| import groovy.json.JsonSlurper | ||
| import org.grails.doc.gradle.AuditGroovydocLinksTask | ||
|
|
||
| import java.util.zip.ZipFile | ||
|
|
||
|
|
@@ -125,6 +126,11 @@ combinedGroovydoc.configure { Groovydoc gdoc -> | |
| gdoc.outputs.dir(gdoc.destinationDir) | ||
| } | ||
|
|
||
| def auditGroovydocLinks = tasks.register('auditGroovydocLinks', AuditGroovydocLinksTask) { | ||
| dependsOn combinedGroovydoc | ||
| apiDocsDir = combinedGroovydoc.get().destinationDir | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
apiDocsDir = project.layout.dir(combinedGroovydoc.map { it.destinationDir })(and then the |
||
| } | ||
|
Comment on lines
+129
to
+132
|
||
|
|
||
| String getVersion(String artifact) { | ||
| String version = configurations.runtimeClasspath | ||
| .resolvedConfiguration | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guard is unreachable: with
@InputDirectory, Gradle fails input validation before the@TaskActionruns when the directory is missing.