Port jtd-esm-codegen onto the incubator main - #169
Conversation
Port the jtd-esm-codegen module from the jdt-to-esm branch onto current main (parent 2026.08.31, jdk.incubator.java.util.json API). The module generates standalone ES2020 ESM validator modules from a flat JTD (RFC 8927) subset for browser consumers; its test suite retains 359 tests (316 official validation.json conformance via GraalJS, 31 unit, 2 jqwik property, 10 junit-js) with 316/316 conformance preserved. Mechanical adaptations applied: - Module pom parent version 0.1.9 -> 2026.08.31. - Package move jdk.sandbox.java.util.json -> jdk.incubator.java.util.json (JtdParser, JtdEsmConformanceTest, JtdEsmPropertyTest); the three unused JSON imports in JtdToEsmCodegenTest were deleted. - API renames: members() -> asMap(), bool() -> asBoolean(), string() -> asString(), elements() -> asList(), element(i) -> get(i), Json.toDisplayString(x, 0) -> (x, ""). - Root pom: jtd-esm-codegen registered as an unconditional reactor module (release target 21; GraalJS is test-scoped only). - .gitignore: removed the jtd-esm-codegen/ ignore entry. - ci.yml exp_tests: 1679 -> 2038, measured empirically from surefire/failsafe XML totals after a full-reactor verify (matches Agent 1's estimate of 1679 + 359). - README: module-table row + "JTD to ESM Validator Codegen (Experimental)" section; AGENTS: module registry entry. The nightly release workflow (jtd-esm-codegen-release.yml) is deliberately deferred to a follow-up. Verify with: mvn -pl jtd-esm-codegen -am clean test -Djava.util.logging.ConsoleHandler.level=INFO (359 tests, 0 failures/errors) mvn clean verify -Djava.util.logging.ConsoleHandler.level=INFO (BUILD SUCCESS; reactor XML total 2038 tests, 0 failures/errors)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13285c078c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sb.append("// ").append(schema.id()).append("-").append(shaPrefix8).append(".js\n"); | ||
| sb.append("// Generated from JTD schema: ").append(schema.id()).append("\n"); |
There was a problem hiding this comment.
When metadata.id contains a newline, its remaining text is emitted outside the // comment and becomes executable module source. For example, an ID containing "safe\nglobalThis.compromised=true;//" causes that assignment to run when the generated validator is imported, so generating code from an untrusted schema permits JavaScript injection; encode newlines/control characters or omit the ID from comments.
Useful? React with 👍 / 👎.
|
|
||
| LOG.fine(() -> "Generated validator: " + outJs); |
There was a problem hiding this comment.
When metadata.id begins with / or contains ../, outDir.resolve(fileName) can resolve outside the requested output directory, and the subsequent Files.writeString truncates an existing file at that location. A schema such as one with ID ../settings therefore lets an untrusted input overwrite files writable by the CLI process; restrict the ID to a safe filename component and verify the normalized result remains under outDir.
Useful? React with 👍 / 👎.
| private static void generateDefinitionFunction(StringBuilder sb, String defName, JtdNode node, RenderContext ctx) { | ||
| final String safeName = toSafeName(defName); | ||
|
|
||
| sb.append("function validate_").append(safeName).append("(v, errors, p, sp) {\n"); |
There was a problem hiding this comment.
When two valid definition names sanitize to the same value, such as a-b and a_b, both are emitted as validate_a_b. This produces duplicate top-level declarations in the generated ES module and prevents refs from retaining distinct validators, so assign unique generated identifiers rather than relying only on character replacement.
Useful? React with 👍 / 👎.
| // 1. Ref | ||
| if (schema.asMap().containsKey("ref")) { | ||
| final var ref = stringValue(schema.asMap().get("ref"), propName, "ref"); | ||
| coreNode = new RefNode(ref); | ||
| } | ||
| // 2. Type | ||
| else if (schema.asMap().containsKey("type")) { |
There was a problem hiding this comment.
When a schema contains multiple form keywords, such as {"type":"string","enum":["x"]}, this if/else if chain silently chooses the first form and generates a validator instead of rejecting the invalid JTD schema. The added code-generation specification requires the forms to be mutually exclusive, so count the present form selectors before dispatching.
Useful? React with 👍 / 👎.
| sb.append(indent).append(" for (const k in ").append(valueExpr).append(") {\n"); | ||
| final String valValue = valueExpr + "[k]"; | ||
| final String valPath = pathExpr + " + \"/\" + k"; |
There was a problem hiding this comment.
When a values object has a key containing / or ~ and its value fails validation, the generated error path appends the key verbatim, so key a/b is reported as /a/b rather than /a~1b. This makes the returned instancePath point to a different structure; emit a runtime JSON Pointer token escape here and in the analogous additional-properties path.
Useful? React with 👍 / 👎.
| - `enum` | ||
| - `metadata.id` (used for the output filename prefix) | ||
|
|
There was a problem hiding this comment.
The README says elements, values, discriminators, refs, and nested properties are rejected, but the added parser, renderer, specification, and conformance tests explicitly implement all of them and no stated Unsupported JTD feature exception exists. Users therefore cannot rely on the documented interface or error behavior; update this section to describe the full implementation rather than the obsolete flat-schema subset.
AGENTS.md reference: AGENTS.md:L22-L29
Useful? React with 👍 / 👎.
1 participant
Closes #168
What was done
Ported the
jtd-esm-codegenmodule from thejdt-to-esmbranch onto current main as a single atomic commit. The module is an experimental CLI that reads a JTD (RFC 8927) schema and generates a standalone vanilla ES2020 ESM module exporting avalidate(instance)function for browser payload validation. Module inventory (≈3540 LOC): 5 main sources (JtdAst,JtdParser,EsmRenderer,JtdToEsmCli,Sha256), 7 test classes (conformance, unit, jqwik property, junit-js suite, GraalJS runner, spec extractor, JUL logging config), 5 test resources,JTD_CODEGEN_SPEC.md, and its pom.Adaptations applied (per Agent 1's analysis checklist)
0.1.9→2026.08.31.jdk.sandbox.java.util.json→jdk.incubator.java.util.jsoninJtdParser,JtdEsmConformanceTest,JtdEsmPropertyTest; the 3 unused JSON imports inJtdToEsmCodegenTestwere deleted.members()→asMap(),bool()→asBoolean(),string()→asString(),elements()→asList(),element(i)→get(i),Json.toDisplayString(x, 0)→(x, "").pom.xml:<module>jtd-esm-codegen</module>registered as an unconditional reactor module (its compilereleasetarget is 21 and GraalJS is test-scoped only, so it needs no JDK 24 profile)..gitignore: removed thejtd-esm-codegen/ignore entry.ci.yml:exp_tests1679 → 2038 — measured empirically from the CI's own counting method (sum oftestsattributes across all surefire/failsafe XML files in a full-reactorclean verify), matching Agent 1's 1679 + 359 estimate exactly.jtd2jarper current main; the branch'sjdt2jartypo was not carried over).Test results
Full test suite preserved: 359 tests, 0 failures/errors/skipped — 316 official
validation.jsonconformance cases (executed via GraalJS, 316/316), 31 unit tests, 2 jqwik property tests, 10 junit-js tests. No test deleted, disabled, or weakened.Full-reactor
clean verifyper-module XML test counts: json-compatibility-suite 1, json-java21 348, json-java21-api-tracker 14, json-java21-jsonpath 100, json-java21-jtd 816, json-java21-jtd-codegen 398, jtd-esm-codegen 359, jtd2jar 2 — total 2038, 0 failures/errors. Pre-existing modules sum to 1679, identical to main's baseline, confirming no other module's counts changed.Verify locally:
Deliberately deferred
The nightly release workflow (
.github/workflows/jtd-esm-codegen-release.yml: uber JAR + native-image builds + prerelease publishing) is out of scope for #168 and was not ported; it can follow as a separate issue once this lands.