Conversation
fabiomadge
previously approved these changes
Mar 31, 2026
Contributor
There was a problem hiding this comment.
While reviewing Aaron's doc, I found several bugs in the code generator that would make the doc describe incorrect behavior. I also reconsidered some design choices. Rather than documenting known bugs, I fixed them:
fromCategoryName?/fromIonName?silently returned wrong values (Lean namespace resolution issue)Init.Strfields serialized asidentinstead ofstrlit- Reflection-based serializer replaced with generated
toIonmethods on each record - Records are now nested inside their category interface (eliminates most
_suffix collisions)
The doc is rewritten to match the new structure. All existing tests updated and passing.
Happy to split the implementation changes into a separate PR if you'd prefer to land the doc independently.
fabiomadge
force-pushed
the
ddm-java-doc
branch
3 times, most recently
from
190e98c to
78389da
Compare
… bug fixes Restructure the generated Java code to use nested records inside category interfaces, replacing the previous flat file-per-operator layout. Each record now has a generated toIon method with field-specific serialization, replacing the reflection-based IonSerializer. Code generator changes: - Operators are nested records within their category's sealed interface - Single-op categories with name collision use "Of" as record name - Generated toIon methods on each record (no reflection, no separator map) - IonSerializer is now a slim helper class with typed public methods - Node interface gains toIon(IonSerializer) method signature - Factory class name is now escaped and PascalCased like other names Bug fixes: - Fix SepFormat.fromCategoryName? and fromIonName? returning some .none for all inputs due to Lean namespace resolution of bare 'none' inside namespace SepFormat (now uses .none which resolves via expected type) - Fix Init.Str fields serializing as (ident ...) instead of (strlit ...) - Remove dead "non-sealed" entry from javaReservedWords Tests and proofs: - Add fromIonName_none_of_invalid theorem (invalid inputs return none) - Add #guard for fromCategoryName? regression - Strengthen roundtrip test to verify strlit for Init.Str fields - Regenerate test Ion data with correct strlit serialization - Update all 15 tests for new nested records structure Documentation: - Rewrite DDMJavaCodeGen.md to reflect new structure - Document multi-file Ion format, ion-java dependency, program header - Document naming rules (Of, _ suffix), factory method naming - Fix serialization format (ident vs strlit distinction) - Document syncat/metadata handling, SourceRange ambiguity, precision limits
The restructured Node.java now imports com.amazon.ion.IonSexp for the toIon method, requiring ion-java on the classpath. Download the jar from Maven Central before the Lean build step so the test runs in CI. Also make the test fail loudly if the jar is missing instead of silently skipping.
fabiomadge
force-pushed
the
ddm-java-doc
branch
from
78389da to
eeecb69
Compare
fabiomadge
previously approved these changes
Mar 31, 2026
Contributor
Author
|
Thanks, @fabiomadge! I'm quite happy to have the code fixes be in this PR, too. |
MikaelMayer
reviewed
Apr 1, 2026
…ify docs - Rename _used to used in Gen.lean since it's actually referenced - Extract newTagged private helper in IonSerializer.java template, converting 5 primitive serializers to one-liners - Note case-insensitive comparison in collision docs
MikaelMayer
previously approved these changes
Apr 2, 2026
joscoh
reviewed
Apr 2, 2026
tautschnig
approved these changes
Apr 3, 2026
MikaelMayer
approved these changes
Apr 3, 2026
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
6 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.
Adds
docs/DDMJavaCodeGen.md, a reference for the DDM Java code generator. Covers CLI usage (strata javaGen), Lean API, generated file structure, type mapping, Ion serialization format, name disambiguation, and limitations. Documentation only — no code changes.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.