Conversation
- refined parsing exception error reporting;
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.29.11 to 3.30.1. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@3c3833e...f1f6e5f) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 3.30.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [com.google.code.gson:gson](https://github.com/google/gson) from 2.13.1 to 2.13.2. - [Release notes](https://github.com/google/gson/releases) - [Changelog](https://github.com/google/gson/blob/main/CHANGELOG.md) - [Commits](google/gson@gson-parent-2.13.1...gson-parent-2.13.2) --- updated-dependencies: - dependency-name: com.google.code.gson:gson dependency-version: 2.13.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…constructs to check; - update test error check;
There was a problem hiding this comment.
Pull request overview
This PR is an experiment to move Commons JEXL’s parser/tooling toward a newer JavaCC (v8) toolchain, adjusting the grammar and parser node types accordingly, and updating/relocating tests around switch parsing/execution.
Changes:
- Switch-related parsing/execution changes: add strict-mode runtime errors for unmatched switch, and add/move dedicated switch tests.
- JavaCC/JJTree migration work: update
Parser.jjt, visitor/node base classes, and Maven build configuration to use newer JavaCC artifacts. - Dependency/CI metadata updates: bump
gson(test scope) and refresh pinned CodeQL action SHAs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/test/java/org/apache/commons/jexl3/SwitchTest.java |
New focused tests for switch statement/expression parsing and strict-mode behavior. |
src/test/java/org/apache/commons/jexl3/LambdaTest.java |
Adjusts a lambda parse-failure expectation to match updated lookahead/grammar behavior. |
src/test/java/org/apache/commons/jexl3/JexlTest.java |
Test cleanups and adds a property assignment assertion; introduces new imports. |
src/test/java/org/apache/commons/jexl3/Issues400Test.java |
Removes switch-related tests that were moved into SwitchTest. |
src/main/java/org/apache/commons/jexl3/parser/SimpleNode.java |
Updates node inheritance model to align with new JavaCC/JJTree output. |
src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java |
Updates visitor fallback method signature to the new base node type. |
src/main/java/org/apache/commons/jexl3/parser/Parser.jjt |
Significant grammar/token/lookahead updates for the newer JavaCC toolchain. |
src/main/java/org/apache/commons/jexl3/internal/Interpreter.java |
Throws in strict mode when switch has no matching case; minor Javadoc tweaks. |
src/changes/changes.xml |
Records the gson version bump in the changelog. |
pom.xml |
Adds snapshot repos + JavaCC 8 snapshot properties and rewires the JavaCC Maven plugin execution. |
.github/workflows/scorecards-analysis.yml |
Updates pinned upload-sarif action SHA. |
.github/workflows/codeql-analysis.yml |
Updates pinned CodeQL action SHAs (init, autobuild, analyze). |
Comments suppressed due to low confidence (3)
src/test/java/org/apache/commons/jexl3/SwitchTest.java:101
- After the expected parsing failure,
scriptstill refers to the previously created (valid) script, soassertNotNull(script)doesn’t validate anything about the failingcreateScriptcall. Consider removing this assertion or asserting on the exception only / explicitly keeping the previous script in a separate variable if you want to ensure it wasn’t modified.
script = jexl.createScript(src, "x");
fail("should not be able to create script with break in switch");
} catch (final JexlException.Parsing xparse) {
assertTrue(xparse.getMessage().contains("break"));
}
assertNotNull(script);
}
@Test
pom.xml:137
- The POM now hard-codes a snapshot repository and pluginRepository pointing at Sonatype snapshots (with both releases and snapshots enabled). This affects all consumers/builds of the project and can make dependency resolution non-reproducible. Prefer gating snapshot repos behind a Maven profile (off by default) or using developer
~/.m2/settings.xml, and disable<releases>for a snapshots-only repository.
<enabled>true</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<name>Plugin Portal Snapshots</name>
<id>plugin-portal-snapshots</id>
<url>https://central.sonatype.com/repository/maven-snapshots/</url>
<releases>
<enabled>true</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</pluginRepository>
</pluginRepositories>
<dependencies>
<dependency>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
<version>1.3.5</version>
</dependency>
pom.xml:205
org.javacc:coreandorg.javacc.generator:javaare declared as separate<plugin>entries under<pluginManagement>, but they are not Maven plugins (and are already present as dependencies ofjavacc-maven-plugin). Keeping them as plugins is confusing and may trigger unnecessary plugin resolution; consider removing these extra<plugin>blocks and leaving them only as<dependency>entries forjavacc-maven-plugin.
<plugin>
<groupId>org.javacc.generator</groupId>
<artifactId>java</artifactId>
<version>${javacc.java.version}</version>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <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> |
There was a problem hiding this 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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.