[
https://issues.apache.org/jira/browse/GROOVY-11668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17952670#comment-17952670
]
ASF GitHub Bot commented on GROOVY-11668:
-----------------------------------------
Copilot commented on code in PR #2228:
URL: https://github.com/apache/groovy/pull/2228#discussion_r2096073231
##########
subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java:
##########
@@ -18,13 +18,18 @@
*/
package org.codehaus.groovy.tools.groovydoc;
+import com.github.javaparser.ParserConfiguration;
+import com.github.javaparser.StaticJavaParser;
import groovy.test.GroovyTestCase;
+import org.apache.tools.ant.taskdefs.condition.JavaVersion;
Review Comment:
This import isn't used anywhere in the test class; please remove it to keep
imports clean.
```suggestion
```
##########
subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovydoc.java:
##########
@@ -144,6 +145,15 @@ public void setNoVersionStamp(boolean noVersionStamp) {
this.noVersionStamp = noVersionStamp;
}
+ /**
+ * Defaults to a popular Java version. Otherwise, override here for a
specific expected source file version.
+ *
+ * @param javaVersion the expected source level of any Java files that may
be parsed
+ */
+ public void setJavaVersion(String javaVersion) {
Review Comment:
The Javadoc mentions a 'popular Java version' but the code doesn't set a
default. Clarify what default is used when this setter is not called and which
version strings are valid.
##########
subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java:
##########
@@ -1026,6 +1040,63 @@ public void testClassDeclarationHeader() throws
Exception {
assertTrue("The Groovy derived class declaration header should match
in:\n" + groovyDerivedClass, derivedClass.matcher(groovyDerivedClass).find());
}
+ public void testLanguageLevelNotSupported() throws Exception {
+ htmlTool = makeHtmltool(new ArrayList<LinkArgument>(),
ParserConfiguration.LanguageLevel.JAVA_1_4.name(), new Properties());
+
+ final String base =
"org/codehaus/groovy/tools/groovydoc/testfiles/generics";
+
+ PrintStream originalSystemErr = System.err;
+ ByteArrayOutputStream systemErr = new ByteArrayOutputStream();
+ try {
+ System.setErr(new PrintStream(systemErr));
+ htmlTool.add(Arrays.asList(
+ base + "/Java.java"
+ ));
+ }
+ finally {
+ System.setErr(originalSystemErr);
+ }
+ final String errorMessage = systemErr.toString();
+ System.err.println(errorMessage);
+
+ final MockOutputTool output = new MockOutputTool();
+ htmlTool.renderToOutput(output, MOCK_DIR);
+
+ final String javadoc = output.getText(MOCK_DIR + "/" + base +
"/Java.html");
+ assertNull("Javadoc should be null since language level is not
supported", javadoc);
+
+ assertTrue("Expected to find Java file in error message",
errorMessage.contains("org/codehaus/groovy/tools/groovydoc/testfiles/generics/Java.java"));
+ assertTrue("Expected to find language level not supported",
errorMessage.contains("Pay attention that this feature is supported starting
from"));
Review Comment:
The test expects a specific guidance message, but no such message is emitted
by the code under test. Either adjust the test or output the expected message
when an unsupported level is encountered.
##########
subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/GroovyDocTool.java:
##########
@@ -71,12 +77,41 @@ public GroovyDocTool(ResourceManager resourceManager,
String[] sourcepaths, Stri
}
}
+ private ParserConfiguration.LanguageLevel calculateLanguageLevel(String
javaVersion) {
+ String version = Optional.ofNullable(javaVersion)
+ .map(String::trim)
+ .map(s -> s.toUpperCase())
+ .filter(s -> !s.isEmpty())
+ .orElse(null);
+
+ if (version == null) {
+ return null;
+ }
+
+ try {
+ return ParserConfiguration.LanguageLevel.valueOf(version);
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException("Unsupported Java Version: " +
javaVersion);
+ }
+ }
+
public void add(List<String> filenames) throws IOException {
if (templateEngine != null) {
// only print out if we are being used for template generation
log.debug("Loading source files for " + filenames);
}
- rootDocBuilder.buildTree(filenames);
+
+ try {
+ if(javaLanguageLevel != null) {
+
StaticJavaParser.getParserConfiguration().setLanguageLevel(javaLanguageLevel);
+ }
+ rootDocBuilder.buildTree(filenames);
+ }
+ finally {
+ if(javaLanguageLevel != null) {
+
StaticJavaParser.getParserConfiguration().setLanguageLevel(null);
+ }
Review Comment:
You reset the global parser level to null after parsing, but you don't
restore the previous setting. Consider saving the old level before setting and
restoring it in the finally block to avoid side effects in multi-threaded
contexts.
```suggestion
ParserConfiguration.LanguageLevel previousLanguageLevel =
StaticJavaParser.getParserConfiguration().getLanguageLevel();
if (javaLanguageLevel != null) {
StaticJavaParser.getParserConfiguration().setLanguageLevel(javaLanguageLevel);
}
rootDocBuilder.buildTree(filenames);
}
finally {
StaticJavaParser.getParserConfiguration().setLanguageLevel(previousLanguageLevel);
```
> Groovydoc crashes with higher Java language levels
> --------------------------------------------------
>
> Key: GROOVY-11668
> URL: https://issues.apache.org/jira/browse/GROOVY-11668
> Project: Groovy
> Issue Type: Improvement
> Components: GroovyDoc
> Reporter: James Daugherty
> Priority: Critical
>
> Groovydoc supports both groovy & java files. It supports parsing via the
> JavaParser library & the library supports specifying a language level.
> However, groovydoc does not have a way of setting it for the library at this
> time. By default the library defaults to the "popular" language level of
> that release - for the current JavaParser version that is Java 11.
> As projects update to newer language features, it is desirable to continue
> using groovydoc for later Java versions. This change is to add an option to
> groovydoc to specify the language level based on the possible values in the
> enum ParserConfiguration.LanguageLevel from the JavaParser library.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
