jdaugherty commented on code in PR #16005:
URL: https://github.com/apache/grails-core/pull/16005#discussion_r3609175634
##########
gradle/docs-dependencies.gradle:
##########
@@ -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/";]
}
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/";]
+ }
+ 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/";]
Review Comment:
Question on these three: since the data modules merged into this repo in
7.0, `org.grails.datastore.*` / `grails.gorm.*` classes are generated in this
repo's own aggregate groovydoc. `gorm.grails.org/latest/api/` is the standalone
pre-merge GORM site — is that the intended target, and are we sure these
prefixes don't hijack references that should resolve locally within the
combined docs?
##########
gradle/docs-dependencies.gradle:
##########
@@ -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/";]
Review Comment:
Groovydoc resolves `links` first-match-wins in list order
(`SimpleGroovyClassDoc.getDocUrl` iterates the links and returns on the first
`type.startsWith(prefix)` hit). Broadening this entry to `org.springframework.`
while it sits *before* the `org.springframework.boot.` entry means every Spring
Boot class reference now matches here first and links into
`docs.spring.io/spring-framework/...`, where the Boot packages don't exist —
the Boot entry below becomes dead code and all Boot links 404.
Most-specific-first fixes it: move the `org.springframework.boot.` entry
above this one.
##########
build-logic/docs-core/src/main/groovy/org/grails/doc/gradle/AuditGroovydocLinksTask.groovy:
##########
@@ -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()) {
Review Comment:
This guard is unreachable: with `@InputDirectory`, Gradle fails input
validation before the `@TaskAction` runs when the directory is missing.
##########
gradle/docs-dependencies.gradle:
##########
@@ -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/";]
}
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/";]
+ }
+ 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/";]
+ 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/";]
Review Comment:
`docs.groovy-lang.org/latest` is Groovy 5 while 7.0.x ships Groovy 4, so
these links will point at the wrong major version's API docs. The rest of this
block pins resolved versions — `GroovySystem.version` is available here to
build the versioned URL
(`https://docs.groovy-lang.org/docs/groovy-${GroovySystem.version}/html/gapi/`).
##########
grails-doc/build.gradle:
##########
@@ -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
Review Comment:
`combinedGroovydoc.get()` realizes the task eagerly and requires the
explicit `dependsOn` above. Provider wiring keeps it lazy and carries the
dependency implicitly:
```groovy
apiDocsDir = project.layout.dir(combinedGroovydoc.map { it.destinationDir })
```
(and then the `dependsOn` line can go.)
##########
build-logic/docs-core/src/main/groovy/org/grails/doc/gradle/AuditGroovydocLinksTask.groovy:
##########
@@ -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.")
Review Comment:
Import `org.gradle.api.GradleException` instead of the inline FQCN — it's
already on the compile classpath here, and the project convention is explicit
imports.
##########
build-logic/docs-core/src/main/groovy/org/grails/doc/gradle/GroovydocLinkAuditor.groovy:
##########
@@ -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 = [
Review Comment:
These nav-link patterns can never match real Groovydoc output. The templates
in `groovy-groovydoc` (checked 4.0.24, what 7.0.x uses) emit nav links
**double-quoted with a legitimate relative prefix**:
`href="${classDoc.relativeRootPath}deprecated-list.html"`. So the single-quoted
patterns are dead code — and if the quoting were "fixed", they'd flag every
nested class page's perfectly valid nav links, since the prefix is correct
there. That also means a "0 violations" run partly reflects this vacuity rather
than clean docs.
The inner-class check below is the genuinely valid part: class
cross-reference links *are* single-quoted in the templates, which is where the
`Outer/Inner.html` vs `Outer.Inner.html` phantom-link bug lives, and the
only-flag-if-the-dotted-file-exists guard is a smart false-positive suppressor.
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 `replacement` strings) are
never read anywhere, and `findViolationsInFile` only records the first match
per pattern per file, so the "Found N malformed links" total in the task's
exception would undercount.
##########
gradle/docs-dependencies.gradle:
##########
@@ -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/";]
}
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/";]
Review Comment:
Minor: these three Jakarta links gate on the resolved artifact version but
hardcode the spec version in the URL (`3.0`, `3.1`, `platform/10`), so they'll
silently go stale on the next spec bump. Deriving the URL segment from the
resolved version (as done for Hibernate above) would keep them consistent.
##########
gradle/docs-dependencies.gradle:
##########
@@ -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/";]
}
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/";]
+ }
+ 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/";]
Review Comment:
Actually, we host these directly now on our website, so we shoudlnt' refer
to the older docs link
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
