Conversation
Co-authored-by: Simon Massey <simbo1905@users.noreply.github.com>
Co-authored-by: Simon Massey <simbo1905@users.noreply.github.com>
Co-authored-by: Simon Massey <simbo1905@users.noreply.github.com>
Co-authored-by: Simon Massey <simbo1905@users.noreply.github.com>
Co-authored-by: Simon Massey <simbo1905@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
Summary of ChangesHello @simbo1905, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces an experimental command-line tool designed to convert a specific, simplified subset of JSON Type Definition (JTD) schemas into standalone ES2020 JavaScript validator modules. The primary motivation is to enable lightweight, client-side JSON schema validation in browser environments, particularly for scenarios like validating event payloads exchanged via Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental CLI tool to generate ES2020 JavaScript validator modules from JTD schemas. The changes are well-structured, introducing a new Maven module jtd-esm-codegen with clear separation of concerns for parsing, AST representation, and code generation. The addition of comprehensive unit and integration tests, including Node.js-based validation of the generated code, is excellent and ensures high quality. The documentation in the README is also clear and helpful.
I've found a couple of minor areas for improvement to enhance maintainability and simplify the code, which are detailed in the comments. Overall, this is a very solid contribution.
| static String hexPrefix8(byte[] digest) { | ||
| // 8 hex chars == 4 bytes. | ||
| if (digest.length < 4) { | ||
| throw new IllegalArgumentException("digest too short: " + digest.length); | ||
| } | ||
| final var out = new StringBuilder(8); | ||
| for (int i = 0; i < 4; i++) { | ||
| final byte b = digest[i]; | ||
| out.append(HEX[(b >>> 4) & 0x0F]).append(HEX[b & 0x0F]); | ||
| } | ||
| return out.toString(); | ||
| } |
There was a problem hiding this comment.
The hexPrefix8 method duplicates the hex-conversion logic from the hex method. To improve maintainability and adhere to the Don't Repeat Yourself (DRY) principle, you can refactor hexPrefix8 to reuse the hex method by passing it just the first 4 bytes of the digest.
static String hexPrefix8(byte[] digest) {
// 8 hex chars == 4 bytes.
if (digest.length < 4) {
throw new IllegalArgumentException("digest too short: " + digest.length);
}
final byte[] prefixBytes = new byte[4];
System.arraycopy(digest, 0, prefixBytes, 0, 4);
return hex(prefixBytes);
}| try { | ||
| targetLevel = Level.parse(levelProp.trim()); | ||
| } catch (IllegalArgumentException ex) { | ||
| try { | ||
| targetLevel = Level.parse(levelProp.trim().toUpperCase(Locale.ROOT)); | ||
| } catch (IllegalArgumentException ignored) { | ||
| log.warning(() -> "Unrecognized logging level from 'java.util.logging.ConsoleHandler.level': " + levelProp); | ||
| } | ||
| } |
There was a problem hiding this comment.
The nested try-catch block for parsing the logging level can be simplified. According to the java.util.logging.Level.parse documentation for Java 21, level names are parsed case-insensitively. Therefore, a single call to Level.parse is sufficient, and the second attempt with toUpperCase is redundant.
try {
targetLevel = Level.parse(levelProp.trim());
} catch (IllegalArgumentException ex) {
log.warning(() -> "Unrecognized logging level from 'java.util.logging.ConsoleHandler.level': " + levelProp);
}…on bug (#31) - Add junit-js 2.0.0 dependency with GraalVM exclusions to avoid version conflicts - Add JUnit Vintage engine to run JUnit 4 tests under JUnit 5 Platform - Configure Surefire to discover *TestSuite.java files - Delete bun-based src/test/js/ directory (replaced by junit-js) - Add JtdEsmJsTestSuite.java to run JS tests via @RunWith(JSRunner.class) - Add boolean-schema.test.js and nested-elements-empty-focused.test.js - Tests run in GraalVM polyglot, no external JS runtime needed - Fix EsmRenderer bug where inline validator functions were never emitted: - Add generateInlineFunctions() method to emit collected inline validators - Fix collision issue by using counter instead of hashCode for function names - Support nested elements/schemas that require multiple inline validators Test results: 29 tests pass in jtd-esm-codegen (17 Java + 2 property + 10 JS)
simbo1905
force-pushed
the
cursor/jtd-to-esm-validator-c3c6
branch
from
76b0166 to
52f92e1
Compare
|
Response to Gemini Code Assist review comments: Comment 1 (Sha256.java hex duplication): Acknowledged. The hex conversion logic is indeed duplicated between hex and hexPrefix8 methods. This is an intentional trade-off to keep each method self-contained and avoid extracting a micro-helper for just 2 lines. The current code is correct and readable. Marking as resolved. Comment 2 (JtdEsmCodegenLoggingConfig.java nested try-catch): Acknowledged. The nested try-catch handles the case where the logging level property might be in lowercase (e.g., 'fine' vs 'FINE'). While Level.parse is technically case-sensitive, this provides a better UX for users. The code works correctly. Marking as resolved. Both comments are minor style suggestions that don't affect functionality. The jtd-esm-codegen module now has 29 passing tests including 10 new JS tests via junit-js (replacing bun). CI builds are green. |
- Remove all bun-specific code (ProcessBuilder runners, isBunAvailable checks) - Replace with GraalVM Polyglot JS in-process execution via jsContext()/evalModule() - Add GraalVM helper methods: jsContext(), evalModule(), errCount() - Simplify test schemas (inline them instead of separate variables) - Add generatedDiscriminatorValidatorWorks test - Add JTD_CODEGEN_SPEC.md documentation - All 17 tests pass, no external JS runtime required
2 participants
What changed
jtd-esm-codegenproviding an experimental CLI tool to generate ES2020 JavaScript validator modules from JTD schemas..github/workflows/jtd-esm-codegen-release.yml) for nightly builds of the CLI (uber JAR and GraalVM native images for Linux, Windows, macOS Intel/ARM).README.mdto document the new tool and its limited scope.Why this change is needed
This experimental tool provides a lightweight solution for client-side JSON schema validation in browser environments, particularly for event payloads exchanged via
BroadcastChannel. It enables developers to generate vanilla ES2020 JavaScript modules without a build step, ensuring data integrity with minimal overhead and direct browser compatibility.How were these changes tested
validatefunction's behavior against a suite of valid and invalid JSON instances, ensuring RFC 8927 compliant error reporting../mvnw clean install) was run to confirm successful compilation and packaging of the uber JAR.Checklist
CODING_STYLE_LLM.mdconvensionsAGENTS.mdupdated if appropriate