Skip to content

fix(format): guard against null members in FormatJvmModelInferrer.inferConstants - #1474

Merged
joaodinissf merged 2 commits into
dsldevkit:masterfrom
joaodinissf:fix/format-inferconstants-null-leak
Aug 25, 2026
Merged

fix(format): guard against null members in FormatJvmModelInferrer.inferConstants#1474
joaodinissf merged 2 commits into
dsldevkit:masterfrom
joaodinissf:fix/format-inferconstants-null-leak

Conversation

@joaodinissf

Copy link
Copy Markdown
Collaborator

The bug

FormatJvmModelInferrer.inferConstants adds createConstant(format, c) to the inferred JVM model unconditionally:

for (final Constant c : FormatGeneratorUtil.getAllConstants(format)) {
  it.getMembers().add(createConstant(format, c));   // createConstant can return null
}

createConstant returns null for a value-less constant (its if (stringValue) … else if (intValue) … return null fall-through — reachable for a constant in an incomplete/error state during editing). Adding that null to the JvmMember EList throws IllegalArgumentException: The 'no null' constraint is violated.

Root cause — a migration regression

The original Xtend was members += allConstants.map[createConstant], which compiles to JvmTypesBuilder.operator_add, and operator_add silently skips null elements. The Xtend→Java migration translated += to a bare loop-add, which does not skip nulls — so the null-skip semantics were lost. The sibling inferRules in the same file kept the null-skip, making this an inconsistent one-off.

The fix

Guard the add (equivalent to operator_add's null-skip / an IterableExtensions.filterNull):

final JvmMember member = createConstant(format, c);
if (member != null) {
  it.getMembers().add(member);
}

Verification

  • New FormatJvmModelInferrerTest pins the behaviour: a value-less Constant must not yield a null member or throw.
  • A/B proven through the full test harness: the test fails on the pre-fix code (IllegalArgumentException: The 'no null' constraint is violated) and passes with the guard. Full local gate: BUILD SUCCESS, 359 tests / 0 failures, all static analysis clean.

Found by an archaeology audit of the merged Xtend→Java migrations. The skill rule that prevents this class of defect is added in a separate PR (#1473, rules/10 §10.5operator_add skips nulls).

🤖 Generated with Claude Code

@rubenporras

Copy link
Copy Markdown
Member

Can you rebase this commit?

@joaodinissf
joaodinissf force-pushed the fix/format-inferconstants-null-leak branch from 753cbd5 to 80c81e7 Compare August 24, 2026 14:17
@joaodinissf
joaodinissf marked this pull request as ready for review August 24, 2026 14:17
@rubenporras

Copy link
Copy Markdown
Member

The maven verify passed, so our baseline check is not working, I will need to investigate, or if you can, please, be my guest :)

joaodinissf added a commit to joaodinissf/dsl-devkit that referenced this pull request Aug 24, 2026
Temporary, dropped from the branch after the run (a git revert cannot
restore green: the revert commit itself would move the bundle's jgit
qualifier past the baseline again). With the baseline gate revived,
this commit's verify run must FAIL with 'Only qualifier changed for
(com.avaloq.tools.ddk/...)' - proving the gate that dsldevkit#1474 silently
bypassed is now active.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
joaodinissf added a commit that referenced this pull request Aug 24, 2026
The Actions dependency cache persists Tycho's HTTP cache
(~/.m2/repository/.cache/tycho) inside ~/.m2/repository. Tycho's
cache-first transport never revalidates cached 404/301 entries and
p2/releases/latest + p2/snapshots/latest are moving pointers, so
restored blobs served a stale baseline and the
compare-version-with-baselines gate passed vacuously (the mojo has no
'baseline not found' branch) - every compare execution completed in
1-22 ms with no network access and no comparisons, and the snapshot
baseline validator logged 'No baseline version' for all 64 modules in
every run since May. This is how #1474 changed bundle content without
a version bump and still passed maven-verify.

Delete only the DDK hosts' cached metadata after each cache restore,
in verify.yml and snapshot.yml: the baseline is fetched fresh (a few
KB) while versioned eclipse.org metadata and all downloaded artifacts
stay cached. release.yml runs no Maven build and needs no change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erConstants

inferConstants added createConstant(format, c) to the JVM model unconditionally.
createConstant returns null for a value-less constant (its switch fall-through), so
a null JvmMember reached it.getMembers().add(...) — which the EMF EList rejects with
IllegalArgumentException: The 'no null' constraint is violated (or leaks a null member
downstream). Regression class: the original Xtend `members += allConstants.map[createConstant]`
compiled to JvmTypesBuilder.operator_add, which silently skips nulls; the Xtend-to-Java
migration translated it to a bare loop-add that does not.

Guard the add (equivalent to operator_add's null-skip). Add FormatJvmModelInferrerTest
pinning the behaviour: a value-less constant must not produce a null member or throw.
A/B verified — the test fails on the pre-fix code (no null constraint violated) and passes
with the guard.

Found by an archaeology audit of the merged Xtend→Java migrations; the skill rule that
prevents this class is added separately (rules/10 §10.5).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joaodinissf
joaodinissf force-pushed the fix/format-inferconstants-null-leak branch from 80c81e7 to 9a67b51 Compare August 24, 2026 18:48
The compare-version-with-baselines gate correctly rejected this branch after
rebase: the null-guard change alters the bundle's content while its version
matched the v19.0.0 baseline (17.3.1). Micro bump per OSGi conventions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joaodinissf
joaodinissf merged commit 53b238e into dsldevkit:master Aug 25, 2026
4 checks passed
@joaodinissf
joaodinissf deleted the fix/format-inferconstants-null-leak branch August 25, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants