Repository navigation
Conversation
- Merge the duplicated rewriters (CodegenMojo and ImplCodeGen) into org.estore.compiler.QueryRewriter; both entry points now delegate to it. - Fail instead of silently falling back: Cypher syntax errors, non-literal query arguments, annotated methods without a query, and unparseable generated code raise QueryCompileException, which the Mojo reports as a MojoExecutionException naming the file. - CodeGenBuilder rejects constructs it used to drop or crash on (WHERE, OPTIONAL MATCH, DISTINCT, ORDER BY/SKIP/LIMIT, WITH, UNION, UNWIND, SET, multi-variable DELETE) and replaces System.exit calls with exceptions. Remove the never-implemented compiler NodePropScan. - Add a compile-queries profile in estore/pom.xml and use it in CI instead of invoking the plugin goal by hand. - Tests: QueryRewriterTest, CodegenMojoTest (first plugin tests), and CompiledQueryTest, which checks compiled results against the interpreter for each supported query shape. - Scope junit-jupiter-engine to test in estore so it no longer leaks into consumers; drop unused plugin dependencies.
The codegen Mojo now writes Transformed*Test.java into target/generated-test-sources/compiled-queries (same package layout) and registers that directory as a test source root, instead of writing next to the original sources in src/test. Stale output is cleared on each run, and mvn clean removes it. Drop the .gitignore rule for in-tree Transformed*Test.java so leftover copies from the old layout show up in git status rather than causing duplicate-class compile errors.
ImplCodeGen was the original standalone rewriter, reachable only through an exec:java@compile entry; the codegen plugin replaces it. Removing it lets QueryRewriter drop its lenient rewrite-everything mode, so it now has a single strict path with one place that adds error context. Also drop per-file timing from the Mojo, remove tests that duplicated coverage elsewhere, and share one helper across CompiledQueryTest's mutation cases.
The Mojo deletes outputDir on every run, and outputDir can be overridden with -DoutputDir, so a mistaken path such as src/test/java would wipe the test sources. Fail with a clear message instead.
gliga
reviewed
Oct 19, 2026
| <artifactId>estore</artifactId> | ||
| <version>1.0.0</version> | ||
| </dependency> | ||
| <dependency> |
Member
There was a problem hiding this comment.
this is a good catch, but can we have this in a separate PR
the entire dependency change
gliga
reviewed
Oct 19, 2026
|
|
||
| #### Run Tests with Compiled Queries | ||
|
|
||
| Test methods annotated with `@CompileQuery` can also run with their Cypher queries compiled |
Member
There was a problem hiding this comment.
If CompileQuery can only be used on tests, let's make that explicit (or be specific about locations)
Author
There was a problem hiding this comment.
Done, will potentially support non-test case in a future PR.
gliga
reviewed
Oct 19, 2026
| target/generated-test-sources/compiled-queries before the tests compile. The plugin | ||
| depends on this module, so it cannot be in the same reactor (even with -pl): install it | ||
| first, then build this module's POM directly, e.g. | ||
| mvn -pl estore -am install -DskipTests && mvn -pl estore-maven-plugin install |
Member
There was a problem hiding this comment.
I feel we are repeating this too many times; can we remove from here; we have in README and in one action; that should be good enough
gliga
reviewed
Oct 19, 2026
| import org.estore.compiler.QueryRewriter; | ||
|
|
||
| /** | ||
| * Rewrites the queries in every {@code @CompileQuery} method under {@code sourceDir} into Java |
gliga
reviewed
Oct 19, 2026
|
|
||
| @Test | ||
| void writesTransformedFileIntoOutputPackage() throws Exception { | ||
| writeSource("ATest", " @CompileQuery void m() { db.query(\"MATCH (n) RETURN n\"); }\n"); |
Member
There was a problem hiding this comment.
should we use diff names in case tests end up running in parallel? Maybe we based it on the name of the test itself
gliga
reviewed
Oct 19, 2026
| "FTest", | ||
| " @CompileQuery void m() { db.query(\"MATCH (n) RETURN n\"); }\n"); | ||
| runMojo(); | ||
| Files.delete(source); |
Member
There was a problem hiding this comment.
should we assert after this that it was deleted and before that it exists
gliga
reviewed
Oct 19, 2026
gliga
left a comment
gliga
left a comment
Member
There was a problem hiding this comment.
Can we have a separate CI action for rewrite tests
- Move the dependency scoping fix to its own PR. This PR keeps only the plugin's test-scoped JUnit 5 for its new tests and drops antlr and javaparser, which the rewritten Mojo no longer imports. - Run the rewritten tests in a separate Compiled Queries CI workflow; restore test-ci and coverage-ci to their main versions. - State in the README and the @CompileQuery javadoc that the annotation is for test methods only. - Remove the build instructions from the estore POM; the README and the workflow cover them. - CodegenMojoTest: derive class names from each test's method name, and check that generated output exists before the source is deleted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Before this PR,
@CompileQueryrewriting could fail without anyone noticing. A query the compiler couldn't handle fell back to the interpreter, or had parts dropped (WHERE,DISTINCT,WITH, …), and the tests still passed. Some code paths calledSystem.exit. The plugin had no tests.Changes
QueryRewriterreplaces the duplicated logic inCodegenMojoandImplCodeGen;ImplCodeGenis removed. Unsupported Cypher, syntax errors and non-literal query arguments now fail the build, naming the file and line. AllSystem.exitcalls are gone.compile-queriesprofile runs the codegen, and a newcompiled-queries-ci.ymlworkflow runs the rewritten tests. Generated tests go totarget/generated-test-sources/compiled-queriesinstead ofsrc/test.@CompileQueryis documented as test-only.QueryRewriterTestcovers rewriting and error cases.CodegenMojoTestadds the plugin's first tests.CompiledQueryTestchecks that compiled results match the interpreter for 12 query shapes.antlr/javaparser, which the rewritten Mojo no longer uses. The scoping fix is in Scope junit-jupiter-engine to test and drop unused plugin test deps #12.Testing
Ran each workflow's steps locally:
🤖 Generated with Claude Code using Claude Opus 5.5 (
claude-opus-5-5)