Copilot commented on code in PR #399:
URL: https://github.com/apache/commons-jexl/pull/399#discussion_r2813549198


##########
src/test/java/org/apache/commons/jexl3/JexlTest.java:
##########
@@ -36,6 +36,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import jdk.nashorn.internal.runtime.ParserException;

Review Comment:
   `ParserException` is imported from `jdk.nashorn.internal.runtime` but never 
used. This will fail Checkstyle (`UnusedImports`) and also relies on a 
JDK-internal package that may not exist on newer JDKs. Remove this import (and 
any related usage if added later).
   ```suggestion
   
   ```



##########
pom.xml:
##########
@@ -161,14 +225,89 @@
                     <finalName>commons-jexl-${project.version}</finalName>
                 </configuration>
             </plugin>
+
+            <!-- changed for JavaCC 8 -->
+            <plugin>
+              <groupId>org.javacc</groupId>
+              <artifactId>javacc-maven-plugin</artifactId>
+              <executions>
+                <execution>
+                  <id>jexl-jjtree</id>
+                  <phase>generate-sources</phase>
+                  <goals>
+                    <goal>jjtree-javacc</goal>
+                  </goals>
+                  <configuration>
+                    
<keepIntermediateDirectory>false</keepIntermediateDirectory> <!-- default -->
+                    
<sourceDirectory>${basedir}/src/main/java/org/apache/commons/jexl3/parser</sourceDirectory>
+                    <jjtreeCmdLineArgs>
+                      <arg>-CODE_GENERATOR="Java"</arg>
+                      <arg>-GRAMMAR_ENCODING="UTF-8"</arg>
+                      
<arg>-NODE_DIRECTORY="${project.basedir}/src/main/java"</arg>
+                      
<arg>-NODE_PACKAGE="org.apache.commons.jexl3.parser"</arg>
+                      
<arg>-JJTREE_OUTPUT_DIRECTORY="${project.build.directory}/generated-sources/jjtree"</arg>
+                    </jjtreeCmdLineArgs>
+                    <javaccCmdLineArgs>
+                      <arg>-CODE_GENERATOR="Java"</arg>
+                      <arg>-GRAMMAR_ENCODING="UTF-8"</arg>
+                      <!-- java and not the usual javacc dir... -->
+                      
<arg>-OUTPUT_DIRECTORY="${project.build.directory}/generated-sources/java"</arg>
+                    </javaccCmdLineArgs>

Review Comment:
   The JavaCC/JJTree config sets `-NODE_DIRECTORY` to 
`${project.basedir}/src/main/java`, which will generate/overwrite node sources 
directly in the checked-in source tree during `generate-sources` and can leave 
a clean checkout with a dirty working tree after a build. Prefer generating 
into `${project.build.directory}/generated-sources/...` (and adding that as a 
source root) to keep builds reproducible and avoid modifying tracked sources as 
a side-effect.



-- 
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]

Reply via email to