[
https://issues.apache.org/jira/browse/GROOVY-11988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18078218#comment-18078218
]
ASF GitHub Bot commented on GROOVY-11988:
-----------------------------------------
Copilot commented on code in PR #2512:
URL: https://github.com/apache/groovy/pull/2512#discussion_r3182238291
##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java:
##########
@@ -855,8 +857,18 @@ private GroovyClassDoc
resolveInternalClassDocFromImport(GroovyRootDoc rootDoc,
if (isPrimitiveType(baseName)) return null;
for (String importName : importedClassesAndPackages) {
String targetClassName = null;
+ String nestedBaseName = baseName.replace('.', '$');
if (aliases.containsKey(baseName)) {
targetClassName = aliases.get(baseName);
+ } else if (baseName.contains(".")) {
+ int dot = baseName.indexOf('.');
+ String outerName = baseName.substring(0, dot);
+ String nestedSuffix = baseName.substring(dot).replace('.',
'$');
+ if (importName.endsWith("/" + outerName)) {
+ targetClassName = importName + nestedSuffix;
+ } else if (importName.endsWith("/*")) {
+ targetClassName = importName.substring(0,
importName.length() - 1) + nestedBaseName;
+ }
Review Comment:
The nested-type normalization here uses baseName.replace('.', '$') /
nestedSuffix.replace('.', '$'), but this class’s internal nested naming uses
'.' in fullPathName (e.g., fullPathName + "." + innerClassName). Using '$' here
is likely to make classNamedExact(...) miss nested classes. Consider
normalizing nested names to the same '.'-based convention used elsewhere in
this file, or converting importName/baseName into that representation before
lookup.
##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java:
##########
@@ -882,11 +894,23 @@ private GroovyClassDoc
resolveInternalClassDocFromImport(GroovyRootDoc rootDoc,
private GroovyClassDoc
resolveInternalClassDocFromSamePackage(GroovyRootDoc rootDoc, String baseName) {
if (isPrimitiveType(baseName)) return null;
- if (baseName.contains(".")) return null;
int lastSlash = fullPathName.lastIndexOf('/');
if (lastSlash < 0) return null;
String pkg = fullPathName.substring(0, lastSlash + 1);
- return ((SimpleGroovyRootDoc)rootDoc).classNamedExact(pkg + baseName);
+ String candidate = baseName.contains(".") ? baseName.replace('.', '$')
: baseName;
+ return ((SimpleGroovyRootDoc)rootDoc).classNamedExact(pkg + candidate);
+ }
+
+ private GroovyClassDoc
resolveNestedClassDocFromEnclosingTypes(GroovyRootDoc rootDoc, String baseName)
{
+ if (rootDoc == null || fullPathName == null ||
!fullPathName.contains("$")) return null;
+ String nestedSuffix = baseName.replace('.', '$');
+ String current = fullPathName;
+ while (current.contains("$")) {
+ current = current.substring(0, current.lastIndexOf('$'));
+ GroovyClassDoc doc = ((SimpleGroovyRootDoc)
rootDoc).classNamedExact(current + "$" + nestedSuffix);
Review Comment:
resolveNestedClassDocFromEnclosingTypes checks for '$' in fullPathName and
walks enclosing types by lastIndexOf('$'), but this class builds nested class
fullPathName values using '.' separators (e.g., outer + "." + inner). As-is,
this method will never run for normal nested classes and the lookup key
(current + "$" + nestedSuffix) won’t match stored keys. Consider switching this
logic to the '.' nested separator used by fullPathName/classNamedExact.
##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java:
##########
@@ -882,11 +894,23 @@ private GroovyClassDoc
resolveInternalClassDocFromImport(GroovyRootDoc rootDoc,
private GroovyClassDoc
resolveInternalClassDocFromSamePackage(GroovyRootDoc rootDoc, String baseName) {
if (isPrimitiveType(baseName)) return null;
- if (baseName.contains(".")) return null;
int lastSlash = fullPathName.lastIndexOf('/');
if (lastSlash < 0) return null;
String pkg = fullPathName.substring(0, lastSlash + 1);
- return ((SimpleGroovyRootDoc)rootDoc).classNamedExact(pkg + baseName);
+ String candidate = baseName.contains(".") ? baseName.replace('.', '$')
: baseName;
+ return ((SimpleGroovyRootDoc)rootDoc).classNamedExact(pkg + candidate);
Review Comment:
resolveInternalClassDocFromSamePackage now maps dotted names to '$'
(baseName.replace('.', '$')), but nested classes appear to be keyed with '.'
separators in fullPathName elsewhere in this class. This lookup likely won’t
find same-package nested classes unless the same convention is used
consistently.
##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java:
##########
@@ -961,20 +967,22 @@ private static String resolveInheritDoc(GroovyMemberDoc
memberDoc,
Set<GroovyMethodDoc> visited,
InheritDocContext
inheritDocContext) {
if (!(memberDoc instanceof GroovyMethodDoc thisMethod)) return null;
- if (classDoc == null) return null;
+ if (classDoc == null) return "";
if (visited == null) visited = new HashSet<>();
if (!visited.add(thisMethod)) return ""; // cycle: suppress literal
{@inheritDoc}
GroovyMethodDoc parent = findInheritedMethod(thisMethod, classDoc, new
HashSet<>());
- if (parent == null) return null;
- if (parent instanceof SimpleGroovyMemberDoc parentMember
- && parentMember.belongsToClass instanceof SimpleGroovyClassDoc
parentClassDoc) {
+ if (parent == null) return "";
+ if (parent instanceof SimpleGroovyMemberDoc parentMember) {
Review Comment:
resolveInheritDoc’s Javadoc says it returns null when no parent method is
found or doc is unavailable, but the implementation now returns "" for several
of those cases (e.g., classDoc==null, parent==null, missing inheritedTag).
Since the caller treats non-null as “resolved”, this changes behavior and makes
the method contract misleading. Either restore null for “unresolved” or update
the contract/caller logic to match the new semantics.
> Add support for {@inheritDoc} in external JDK classes
> -----------------------------------------------------
>
> Key: GROOVY-11988
> URL: https://issues.apache.org/jira/browse/GROOVY-11988
> Project: Groovy
> Issue Type: Improvement
> Components: groovy-groovydoc
> Reporter: Daniel Sun
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)