This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 17e66a2aab GROOVY-11988: Add support for {@inheritDoc} in external JDK
classes (address AI review comments)
17e66a2aab is described below
commit 17e66a2aab7d839e7e81075a52a81d73a5034d97
Author: Paul King <[email protected]>
AuthorDate: Fri May 8 21:12:22 2026 +1000
GROOVY-11988: Add support for {@inheritDoc} in external JDK classes
(address AI review comments)
---
.../tools/groovydoc/ExternalJavadocSupport.java | 67 +++++++++-
.../groovy/tools/groovydoc/TagRenderer.java | 6 +
.../groovy/tools/groovydoc/GroovyDocToolTest.java | 147 ++++++++++++++++++---
3 files changed, 198 insertions(+), 22 deletions(-)
diff --git
a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/ExternalJavadocSupport.java
b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/ExternalJavadocSupport.java
index bd6f13716e..47a5838818 100644
---
a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/ExternalJavadocSupport.java
+++
b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/ExternalJavadocSupport.java
@@ -45,6 +45,7 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
@@ -55,6 +56,7 @@ import java.util.zip.ZipFile;
* source method overrides a method declared outside the documented source set.
*/
final class ExternalJavadocSupport {
+ private static final System.Logger LOGGER =
System.getLogger(ExternalJavadocSupport.class.getName());
private static final JavaParser JAVA_PARSER = new JavaParser(
new
ParserConfiguration().setLanguageLevel(ParserConfiguration.LanguageLevel.BLEEDING_EDGE)
);
@@ -63,7 +65,10 @@ final class ExternalJavadocSupport {
private static final Map<Class<?>, List<ExternalMethodData>> METHOD_CACHE
= new ConcurrentHashMap<>();
private static final Map<Class<?>, GroovyMethodDoc[]> METHOD_DOC_CACHE =
new ConcurrentHashMap<>();
private static final AtomicInteger ACTIVE_CACHE_SESSIONS = new
AtomicInteger();
+ private static final AtomicBoolean SRC_ZIP_WARNING_LOGGED = new
AtomicBoolean();
+ private static final Object ZIP_LOCK = new Object();
private static final GroovyMethodDoc[] EMPTY_GROOVYMETHODDOC_ARRAY = new
GroovyMethodDoc[0];
+ private static ZipFile sharedZipFile;
private ExternalJavadocSupport() {
}
@@ -115,14 +120,16 @@ final class ExternalJavadocSupport {
}
/**
- * Clears all external Javadoc caches. This method is automatically called
when
- * the last active {@link CacheSession} is closed. It can also be called
manually
- * to force a reset of cached data.
+ * Clears all external Javadoc caches and releases the shared JDK source
archive
+ * handle. This method is automatically called when the last active
+ * {@link CacheSession} is closed. It can also be called manually to force
a reset
+ * of cached data.
*/
static void clearCaches() {
RAW_COMMENT_CACHE.clear();
METHOD_CACHE.clear();
METHOD_DOC_CACHE.clear();
+ closeSharedZipFile();
}
private static GroovyMethodDoc[] cachedMethodDocsFor(Class<?>
externalClass) {
@@ -277,11 +284,26 @@ final class ExternalJavadocSupport {
}
private static Optional<CompilationUnit> loadCompilationUnit(Class<?>
externalClass) {
- if (JDK_SRC_ZIP == null) return Optional.empty();
+ if (JDK_SRC_ZIP == null) {
+ warnMissingSrcZipOnce();
+ return Optional.empty();
+ }
String entryName = sourceEntryName(externalClass);
if (entryName == null) return Optional.empty();
+ ZipFile shared = sharedZipFileForActiveSession();
+ if (shared != null) {
+ return parseCompilationUnit(shared, entryName);
+ }
try (ZipFile zip = new ZipFile(JDK_SRC_ZIP.toFile())) {
+ return parseCompilationUnit(zip, entryName);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+
+ private static Optional<CompilationUnit> parseCompilationUnit(ZipFile zip,
String entryName) {
+ try {
ZipEntry entry = zip.getEntry(entryName);
if (entry == null) {
entry = findFallbackEntry(zip, entryName);
@@ -297,7 +319,40 @@ final class ExternalJavadocSupport {
}
}
- private static ZipEntry findFallbackEntry(ZipFile zip, String entryName) {
+ private static ZipFile sharedZipFileForActiveSession() {
+ if (JDK_SRC_ZIP == null || ACTIVE_CACHE_SESSIONS.get() == 0) return
null;
+ synchronized (ZIP_LOCK) {
+ if (sharedZipFile != null) return sharedZipFile;
+ if (ACTIVE_CACHE_SESSIONS.get() == 0) return null;
+ try {
+ sharedZipFile = new ZipFile(JDK_SRC_ZIP.toFile());
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ return sharedZipFile;
+ }
+ }
+
+ private static void closeSharedZipFile() {
+ synchronized (ZIP_LOCK) {
+ if (sharedZipFile == null) return;
+ try {
+ sharedZipFile.close();
+ } catch (IOException ignored) {
+ }
+ sharedZipFile = null;
+ }
+ }
+
+ private static void warnMissingSrcZipOnce() {
+ if (SRC_ZIP_WARNING_LOGGED.compareAndSet(false, true)) {
+ LOGGER.log(System.Logger.Level.WARNING,
+ "JDK src.zip not found under java.home=" +
System.getProperty("java.home")
+ + "; external {@inheritDoc} resolution from JDK
classes is unavailable.");
+ }
+ }
+
+ static ZipEntry findFallbackEntry(ZipFile zip, String entryName) {
int slash = entryName.indexOf('/');
String suffix = slash >= 0 ? "/" + entryName.substring(slash + 1) :
"/" + entryName;
return zip.stream()
@@ -395,7 +450,7 @@ final class ExternalJavadocSupport {
return false;
}
- private static String eraseTypeName(String declaredType) {
+ static String eraseTypeName(String declaredType) {
if (declaredType == null || declaredType.isEmpty()) return "";
StringBuilder erased = new StringBuilder(declaredType.length());
int genericDepth = 0;
diff --git
a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java
b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java
index 7fc8960192..8eaa1c59db 100644
---
a/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java
+++
b/subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java
@@ -1156,6 +1156,12 @@ final class TagRenderer {
}
private static boolean isTypeVariableName(String typeName) {
+ // Heuristic for recognizing a Java type-variable name when matching
an override
+ // against its parent declaration during {@inheritDoc} resolution.
Matches the
+ // common single-letter (optionally numbered) convention (T, E, R, K,
V, T1, ...).
+ // KEY and VALUE are accepted because Groovy's own
org.apache.groovy.json.internal.Cache
+ // declares its type parameters as <KEY, VALUE>, which would otherwise
look like
+ // real class names to this matcher.
return typeName.matches("[A-Z][0-9]?") || "KEY".equals(typeName) ||
"VALUE".equals(typeName);
}
diff --git
a/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java
b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java
index 8dd876411e..95f8d12ff2 100644
---
a/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java
+++
b/subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java
@@ -47,6 +47,9 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
public class GroovyDocToolTest extends GroovyTestCase {
private static final String MOCK_DIR = "mock/doc";
@@ -824,6 +827,7 @@ public class GroovyDocToolTest extends GroovyTestCase {
}
public void testInheritDocResolvesFromExternalJdkAbstractClassInHtml()
throws Exception {
+ if (skipIfNoJdkSrcZip()) return;
String base = "org/codehaus/groovy/tools/groovydoc/testfiles";
htmlTool.add(List.of(base + "/JavaExtendsWriterInheritDoc.java"));
@@ -836,15 +840,16 @@ public class GroovyDocToolTest extends GroovyTestCase {
assertNotNull("Expected JavaExtendsWriterInheritDoc.html in output",
doc);
assertNotNull("Expected close() section in:\n" + doc, closeSection);
assertNotNull("Expected flush() section in:\n" + doc, flushSection);
- assertTrue("Expected inherited close() text from java.io.Writer in:\n"
+ doc,
- normalizeWhitespace(closeSection).contains("Closes the
stream"));
- assertTrue("Expected inherited flush() text from java.io.Writer in:\n"
+ doc,
- normalizeWhitespace(flushSection).contains("Flushes the
stream"));
+ assertMatches("Expected inherited close() text from java.io.Writer
in:\n" + doc,
+ "(?i)close[sd]?\\b[^\\n]{0,40}\\bstream",
normalizeWhitespace(closeSection));
+ assertMatches("Expected inherited flush() text from java.io.Writer
in:\n" + doc,
+ "(?i)flush(es|ed|ing)?\\b[^\\n]{0,40}\\bstream",
normalizeWhitespace(flushSection));
assertFalse("External JDK inheritDoc should not remain literal in:\n"
+ doc,
doc.contains("{@inheritDoc}"));
}
public void testInheritDocResolvesFromExternalObjectMethodInHtml() throws
Exception {
+ if (skipIfNoJdkSrcZip()) return;
String base = "org/codehaus/groovy/tools/groovydoc/testfiles";
htmlTool.add(List.of(base + "/JavaObjectCloneInheritDocChild.java"));
@@ -855,8 +860,8 @@ public class GroovyDocToolTest extends GroovyTestCase {
String cloneSection = findMethodSection(doc, "clone", "");
assertNotNull("Expected JavaObjectCloneInheritDocChild.html in
output", doc);
assertNotNull("Expected clone() section in:\n" + doc, cloneSection);
- assertTrue("Expected inherited clone() text from java.lang.Object
in:\n" + doc,
- normalizeWhitespace(cloneSection).contains("Creates and
returns a copy of this object"));
+ assertMatches("Expected inherited clone() text from java.lang.Object
in:\n" + doc,
+ "(?i)copy\\s+of\\s+this\\s+object",
normalizeWhitespace(cloneSection));
assertFalse("External Object inheritDoc should not remain literal
in:\n" + doc,
doc.contains("{@inheritDoc}"));
}
@@ -903,6 +908,7 @@ public class GroovyDocToolTest extends GroovyTestCase {
}
public void testInheritDocResolvesFromExternalMapAndObjectMethodsInHtml()
throws Exception {
+ if (skipIfNoJdkSrcZip()) return;
String base = "org/codehaus/groovy/tools/groovydoc/testfiles";
htmlTool.add(List.of(base + "/JavaImplementsMapInheritDoc.java"));
@@ -919,16 +925,16 @@ public class GroovyDocToolTest extends GroovyTestCase {
assertNotNull("Expected containsValue(Object) section in:\n" + doc,
containsValueSection);
assertNotNull("Expected equals(Object) section in:\n" + doc,
equalsSection);
assertNotNull("Expected hashCode() section in:\n" + doc,
hashCodeSection);
- assertTrue("Expected inherited clear() text from java.util.Map in:\n"
+ doc,
- normalizeWhitespace(clearSection).contains("Removes all of the
mappings from this map"));
- assertTrue("Expected inherited containsValue(Object) text from
java.util.Map in:\n" + doc,
- containsValueSection.contains("Returns <CODE>true</CODE> if
this map maps one or more keys to the"));
- assertTrue("Expected inherited equals(Object) text from
java.lang.Object in:\n" + doc,
- equalsSection.contains("Indicates whether some other object is
\"equal to\" this one"));
- assertTrue("Expected normalized inherited hashCode() text from
java.lang.Object in:\n" + doc,
- normalizeWhitespace(hashCodeSection).contains("a hash code
value for this object"));
- assertFalse("Inherited external docs should not retain raw Javadoc
comment markers in:\n" + doc,
- normalizeWhitespace(doc).contains("* Removes all of the
mappings"));
+ assertMatches("Expected inherited clear() text from java.util.Map
in:\n" + doc,
+ "(?i)remov(es|ing)?\\b[^\\n]{0,30}\\bmapping",
normalizeWhitespace(clearSection));
+ assertMatches("Expected inherited containsValue(Object) text from
java.util.Map in:\n" + doc,
+ "(?i)map\\s+maps?\\s+one\\s+or\\s+more\\s+key",
normalizeWhitespace(containsValueSection));
+ assertMatches("Expected inherited equals(Object) text from
java.lang.Object in:\n" + doc,
+ "(?i)\\bother\\s+object\\b[^\\n]{0,40}\\bequal",
normalizeWhitespace(equalsSection));
+ assertMatches("Expected normalized inherited hashCode() text from
java.lang.Object in:\n" + doc,
+ "(?i)hash\\s*code\\b[^\\n]{0,30}\\bobject",
normalizeWhitespace(hashCodeSection));
+ assertFalse("Inherited external docs should not retain raw Javadoc
comment markers (a leading '* ') in:\n" + doc,
+
Pattern.compile("\\*\\s+(?i:Removes|Returns|Indicates|Creates|Closes|Flushes)\\b").matcher(doc).find());
assertFalse("Inherited external docs should not leave raw link/index
inline tags in:\n" + doc,
doc.contains("{@linkplain") || doc.contains("{@index"));
assertFalse("External Map/Object inheritDoc should not remain literal
in:\n" + doc,
@@ -1051,6 +1057,31 @@ public class GroovyDocToolTest extends GroovyTestCase {
return text == null ? null : text.replaceAll("\\s+", " ").trim();
}
+ private static void assertMatches(String message, String regex, String
content) {
+ if (content == null ||
!Pattern.compile(regex).matcher(content).find()) {
+ fail(message + "\n pattern: " + regex + "\n content: " +
content);
+ }
+ }
+
+ private boolean skipIfNoJdkSrcZip() {
+ if (jdkSrcZipPath() != null) return false;
+ System.out.println("[skip] " + getName() + ": JDK src.zip not found
under java.home="
+ + System.getProperty("java.home"));
+ return true;
+ }
+
+ private static Path jdkSrcZipPath() {
+ String javaHome = System.getProperty("java.home");
+ if (javaHome == null || javaHome.isEmpty()) return null;
+ Path home = Paths.get(javaHome);
+ Path direct = home.resolve("lib/src.zip");
+ if (Files.isRegularFile(direct)) return direct;
+ Path parent = home.getParent();
+ if (parent == null) return null;
+ Path sibling = parent.resolve("lib/src.zip");
+ return Files.isRegularFile(sibling) ? sibling : null;
+ }
+
private static GroovyMethodDoc findMethod(GroovyClassDoc classDoc, String
name, int parameterCount) {
for (GroovyMethodDoc method : classDoc.methods()) {
if (name.equals(method.name()) && method.parameters().length ==
parameterCount) {
@@ -3448,4 +3479,88 @@ public class GroovyDocToolTest extends GroovyTestCase {
return null;
}
+
+ public void testEraseTypeNameStripsGenericsAndWildcards() {
+ assertEquals("List",
ExternalJavadocSupport.eraseTypeName("List<String>"));
+ assertEquals("Map", ExternalJavadocSupport.eraseTypeName("Map<String,
Integer>"));
+ assertEquals("Map", ExternalJavadocSupport.eraseTypeName("Map<String,
Map<String, Integer>>"));
+ assertEquals("Number", ExternalJavadocSupport.eraseTypeName("? extends
Number"));
+ assertEquals("Integer", ExternalJavadocSupport.eraseTypeName("? super
Integer"));
+ assertEquals("Object", ExternalJavadocSupport.eraseTypeName("?"));
+ assertEquals("List[]", ExternalJavadocSupport.eraseTypeName("List<?
extends T>[]"));
+ assertEquals("", ExternalJavadocSupport.eraseTypeName(""));
+ assertEquals("", ExternalJavadocSupport.eraseTypeName(null));
+ }
+
+ public void testFindFallbackEntryFindsByTrailingPathSuffix() throws
Exception {
+ Path zipPath = Files.createTempFile("ext-javadoc-fallback", ".zip");
+ try {
+ try (ZipOutputStream out = new
ZipOutputStream(Files.newOutputStream(zipPath))) {
+ out.putNextEntry(new ZipEntry("noise/elsewhere/Other.java"));
+ out.write("// other".getBytes());
+ out.closeEntry();
+ out.putNextEntry(new
ZipEntry("some.module/com/example/Foo.java"));
+ out.write("// foo".getBytes());
+ out.closeEntry();
+ }
+ try (ZipFile zip = new ZipFile(zipPath.toFile())) {
+ ZipEntry hit = ExternalJavadocSupport.findFallbackEntry(zip,
"java.base/com/example/Foo.java");
+ assertNotNull("Expected fallback to find Foo.java by trailing
path", hit);
+ assertEquals("some.module/com/example/Foo.java",
hit.getName());
+
+ ZipEntry miss = ExternalJavadocSupport.findFallbackEntry(zip,
"java.base/com/example/Missing.java");
+ assertNull("No entry should match Missing.java", miss);
+ }
+ } finally {
+ Files.deleteIfExists(zipPath);
+ }
+ }
+
+ public void testFindFallbackEntryHandlesEntryNameWithoutSlash() throws
Exception {
+ Path zipPath = Files.createTempFile("ext-javadoc-fallback-noslash",
".zip");
+ try {
+ try (ZipOutputStream out = new
ZipOutputStream(Files.newOutputStream(zipPath))) {
+ out.putNextEntry(new ZipEntry("any/where/Bare.java"));
+ out.write("// bare".getBytes());
+ out.closeEntry();
+ }
+ try (ZipFile zip = new ZipFile(zipPath.toFile())) {
+ ZipEntry hit = ExternalJavadocSupport.findFallbackEntry(zip,
"Bare.java");
+ assertNotNull("Expected fallback to find entry by '/Bare.java'
suffix", hit);
+ assertEquals("any/where/Bare.java", hit.getName());
+ }
+ } finally {
+ Files.deleteIfExists(zipPath);
+ }
+ }
+
+ // Exercises the matchesTypeName same-package / java.lang / array-aware
branches: the JDK
+ // source for java.lang.Throwable declares getMessage() returning String,
setStackTrace
+ // taking StackTraceElement[] (bare same-package types). Override-matching
has to fall
+ // through past canonical/typeName checks before resolving these to the
reflected
+ // java.lang.* qualified types and producing a non-empty raw comment.
+ public void testMatchesTypeNameResolvesUnqualifiedJavaLangTypes() {
+ if (skipIfNoJdkSrcZip()) return;
+ try (AutoCloseable ignored =
ExternalJavadocSupport.openCacheSession()) {
+ GroovyMethodDoc[] docs = ExternalJavadocSupport.methodsFor(new
ExternalGroovyClassDoc(Throwable.class));
+ assertTrue("Expected reflective methods for java.lang.Throwable",
docs.length > 0);
+
+ boolean foundGetMessage = false;
+ boolean foundSetStackTrace = false;
+ for (GroovyMethodDoc doc : docs) {
+ String raw = ((SimpleGroovyMethodDoc) doc).getRawCommentText();
+ if (raw == null || raw.isEmpty()) continue;
+ if ("getMessage".equals(doc.name()) && doc.parameters().length
== 0) {
+ foundGetMessage = true;
+ } else if ("setStackTrace".equals(doc.name()) &&
doc.parameters().length == 1) {
+ foundSetStackTrace = true;
+ }
+ }
+ assertTrue("Expected resolved Javadoc for Throwable#getMessage()",
foundGetMessage);
+ assertTrue("Expected resolved Javadoc for
Throwable#setStackTrace(StackTraceElement[])"
+ + " (bare same-package array parameter)",
foundSetStackTrace);
+ } catch (Exception e) {
+ fail("session close threw: " + e);
+ }
+ }
}
