From 627637c0f277885b7560d3f7b052320bc765178e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Dinis=20Ferreira?= Date: Sun, 19 Jul 2026 13:24:59 +0200 Subject: [PATCH] docs: encode operator_add null-skip, exception lowering, and charset rules in xtend-to-java skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden the skill with three faithfulness rules and one meta-lesson learned from an independent blind re-migration of check.core and an archaeology audit of the merged migrations. - rules/10 §10.5: `+=` over a collection binds to JvmTypesBuilder.operator_add, which SKIPS nulls; the to builders return null on a null name/type. A bare addAll/loop-add leaks the null (IllegalArgumentException: element: null) past every static gate and test. Faithful Java needs filterNull or an explicit null guard. This shipped once — FormatJvmModelInferrer.inferConstants — cited as the worked example. - rules/05 §5.5: how Xtend lowers try/catch (catch(Specific) -> catch(Throwable)+ instanceof+sneakyThrow; uncaught checked body -> sneakyThrow(raw), no declared checked exceptions). Reproduce with Exceptions.sneakyThrow; never wrap in new RuntimeException/ IllegalStateException (changes the observed exception type). - rules/09 §9.11: charset is a sanctioned deviation — specify UTF_8 (PMD RelianceOnDefaultCharset forbids the default), do not reproduce the default-charset constructor. - known-pitfalls: behavioural equivalence is not literal-token equivalence — test-validate every divergence; the gate is the arbiter. Co-Authored-By: Claude Fable 5 --- .../xtend-to-java/rules/05-control-flow.md | 26 ++++++++++ .../xtend-to-java/rules/09-misc-syntax.md | 12 +++++ .../rules/10-jvm-model-inferrer.md | 48 ++++++++++++++++++- .../xtend-to-java/workflow/known-pitfalls.md | 2 + 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/.agents/skills/xtend-to-java/rules/05-control-flow.md b/.agents/skills/xtend-to-java/rules/05-control-flow.md index 2789f207dc..92408f2a4e 100644 --- a/.agents/skills/xtend-to-java/rules/05-control-flow.md +++ b/.agents/skills/xtend-to-java/rules/05-control-flow.md @@ -54,3 +54,29 @@ final String label = x != null ? x.getName() : ""; ``` For multi-line bodies, factor to a helper method or write `if`/`else` with an assignment in each branch. + +## 5.5 Exception handling — how Xtend lowers `try`/`catch` + +Xtend hides checked exceptions. Reading `xtend-gen/` shows the real lowering, and the faithful Java must +reproduce the **observed exception behaviour** — not a convenient re-wrap. + +- **`catch (SpecificException e)`** compiles to: + ```java + catch (Throwable _t) { + if (_t instanceof SpecificException) { /* the catch body */ } + else { throw Exceptions.sneakyThrow(_t); } + } + ``` + So the original catches its declared type and **sneaky-rethrows everything else unchanged**. A migration + that writes a plain `catch (SpecificException e)` silently drops the sneaky-rethrow of other throwables — + usually acceptable, but note it; a migration that *narrows* `catch (Throwable)` to `catch (Exception)` + changes behaviour (drops `Error`). +- **A body that throws a checked exception with no `catch`** compiles to a whole-body + `try { ... } catch (Throwable _e) { throw Exceptions.sneakyThrow(_e); }` and the method declares **no** + checked exceptions — the **original throwable propagates UNWRAPPED**. +- **The faithful reproduction** is `org.eclipse.xtext.xbase.lib.Exceptions.sneakyThrow(e)` (the same utility + the compiler uses; it rethrows the original throwable), keeping the method's `throws` clause as Xtend left it. +- **DO NOT wrap in a new exception type.** `throw new RuntimeException(e)` / `new IllegalStateException(e)` + changes the exception type callers observe — a real behavioural regression, not a style choice. (Legitimate + `throw new IllegalStateException("message")` on a genuinely-bad state — with no caught cause — is unrelated + and fine.) diff --git a/.agents/skills/xtend-to-java/rules/09-misc-syntax.md b/.agents/skills/xtend-to-java/rules/09-misc-syntax.md index 5ec9596658..a188b7d828 100644 --- a/.agents/skills/xtend-to-java/rules/09-misc-syntax.md +++ b/.agents/skills/xtend-to-java/rules/09-misc-syntax.md @@ -170,3 +170,15 @@ Rules: - **Copy Javadoc from the Xtend source verbatim.** Never generate, guess, or infer Javadoc that was not in the original. Invented comments are misleading. - **`@throws` tags**: Only add when (1) the method already has Javadoc AND (2) the migrated signature declares a `throws` clause. Do not add Javadoc just to host a `@throws` tag. - Do **not** add `@SuppressWarnings("all")` — the Xtend compiler injects this into `xtend-gen/`; human-converted Java shouldn't have it. + +## 9.11 Charset — a sanctioned deviation from `xtend-gen` + +When the Xtend source constructs a reader/writer with **no charset** (`new InputStreamReader(stream)`, +`new String(bytes)`, `.getBytes()`), `xtend-gen` faithfully reproduces the **platform-default** charset. +**Do not reproduce that** — PMD `RelianceOnDefaultCharset` legitimately forbids it (platform-dependent), so +the byte-faithful version cannot pass the gate. Specify `java.nio.charset.StandardCharsets.UTF_8` (the real +encoding of the workspace's source files): `new InputStreamReader(stream, StandardCharsets.UTF_8)`. + +This is one of the few places a migration **should** diverge from `xtend-gen`. General principle: **where a +valid lint rule and a literal `xtend-gen` behaviour conflict on a latent-bug pattern (default charset, an +un-guarded resource, etc.), the migration fixes the bug — it does not suppress the rule.** diff --git a/.agents/skills/xtend-to-java/rules/10-jvm-model-inferrer.md b/.agents/skills/xtend-to-java/rules/10-jvm-model-inferrer.md index bed5baeb42..03391e651d 100644 --- a/.agents/skills/xtend-to-java/rules/10-jvm-model-inferrer.md +++ b/.agents/skills/xtend-to-java/rules/10-jvm-model-inferrer.md @@ -13,7 +13,7 @@ is the canonical worked example — read it in full before migrating another inf | `def dispatch infer(X x, IJvmDeclaredTypeAcceptor acceptor, boolean preIndexingPhase)` | `_infer(final X x, final IJvmDeclaredTypeAcceptor acceptor, final boolean isPreIndexingPhase)` + the dispatcher pattern ([`rules/09-misc-syntax.md`](./09-misc-syntax.md) §9.7) | | `x.toClass(name)` | `jvmTypesBuilder.toClass(x, name)` | | `acceptor.accept(cls, [ ... ])` | `acceptor.accept(cls, initializer)` where `initializer` is a `Procedure1` (see `FormatJvmModelInferrer.java:182-192`) | -| `members += x` / `superTypes += x` / `annotations += x` | `it.getMembers().add(x)` / `it.getSuperTypes().add(x)` / `it.getAnnotations().add(x)` | +| `members += x` / `superTypes += x` / `annotations += x` | `it.getMembers().add(x)` / `it.getSuperTypes().add(x)` / `it.getAnnotations().add(x)` — **for a SINGLE element only**; for `+= aCollection` see §10.5 (`operator_add` skips nulls, plain `add`/`addAll` does not) | | `x.toMethod(name, type) [ ... ]` | `jvmTypesBuilder.toMethod(x, name, type, initializer)` with a `Procedure1` (`:223-235`) | | `x.toField(name, type) [ ... ]` / `x.toParameter(name, type)` | `jvmTypesBuilder.toField(x, name, type, initializer)` / `jvmTypesBuilder.toParameter(x, name, type)` (`:245`) | | `typeRef(T)` / `typeRef(name)` | `_typeReferenceBuilder.typeRef(...)` — the protected field inherited from `AbstractModelInferrer` (`:202-204`); for lookups needing a context object use `typeReferences.getTypeForName(name, context)` (`:235`) | @@ -49,7 +49,51 @@ Xtend assigns bodies two ways; both become `jvmTypesBuilder.setBody(method, ...) (same file, `:113`). - `members += list.map(...).flatten.filterNull` chains: see [`references/xtend-library-replacements.md`](../references/xtend-library-replacements.md) - for `flatten`/`filterNull` stream equivalents; the result feeds `getMembers().addAll(...)`. + for `flatten`/`filterNull` stream equivalents; the result feeds the add — but read §10.5 first + for the null-skip requirement, which is the most dangerous inferrer-migration trap. + +## 10.5 ⚠ `operator_add` (`+=`) SKIPS nulls — plain `add`/`addAll` does NOT + +**This is the highest-risk inferrer defect: it passes every static gate (PMD/Checkstyle/SpotBugs) and +every test that does not happen to feed a null, then throws `IllegalArgumentException: element: null` +at validation/generation in production.** + +When the right-hand side of `+=` is a **collection** — `someJvmList += iterable.map[toField(...)]` — +the Xtend compiler binds it to `JvmTypesBuilder.operator_add(EList, Iterable)`, which **silently skips +null elements** while adding. And the JvmTypesBuilder factories **return null**: +`toField` / `toMethod` / `toClass` / `toConstructor` / `toGetter` / `toSetter` / `toEnumerationLiteral` +/ `toAnnotation` / `toParameter` all return `null` when their name (or type) argument is null, and any +helper with a `return null` fall-through (a `switch`/`if` that doesn't match) does too. + +So the faithful Java of `list += map[to(...)]` is **not** a bare loop or `addAll`: + +```java +// WRONG — leaks nulls (createConstant returns null for a value-less constant): +for (final Constant c : constants) { + it.getMembers().add(createConstant(format, c)); // adds null → element: null downstream +} + +// RIGHT — reproduce operator_add's null-skip (either form): +for (final Constant c : constants) { + final JvmMember member = createConstant(format, c); + if (member != null) { + it.getMembers().add(member); + } +} +// or, matching the Xtend chain shape: +Iterables.addAll(it.getMembers(), IterableExtensions.filterNull(ListExtensions.map(constants, c -> createConstant(format, c)))); +``` + +**Checklist for every `+=`/collection-add site in a migrated inferrer:** can the mapped producer return +null (nullable name/type, or a `return null` branch)? If yes, there MUST be a `filterNull` or an explicit +null guard. A bare `addAll`/loop-`add` over a null-capable map is a faithfulness regression. + +> Real shipped example: `FormatJvmModelInferrer.inferConstants` translated the Xtend +> `members += allConstants.map[createConstant]` (null-skipping `operator_add`) into a bare +> `for { it.getMembers().add(createConstant(format, c)); }`. `createConstant` returns null for a +> value-less constant, so a null `JvmMember` leaked into the model — undetected by all gates and tests +> until a value-less constant is declared. Note that a single-element `+=` (e.g. `members += toField(...)`) +> is safe to translate as a plain `.add`; the trap is only the **collection** overload. ## 10.4 Verification diff --git a/.agents/skills/xtend-to-java/workflow/known-pitfalls.md b/.agents/skills/xtend-to-java/workflow/known-pitfalls.md index 16fa0cde31..82a9c8974b 100644 --- a/.agents/skills/xtend-to-java/workflow/known-pitfalls.md +++ b/.agents/skills/xtend-to-java/workflow/known-pitfalls.md @@ -42,3 +42,5 @@ Consolidated table of common mistakes and their fixes. Review before and after e | **Text block ≠ inline-`'''` exactly** | Java text blocks strip trailing whitespace on each content line and add a trailing newline before the closing `"""`; an inline-`'''` Xtend template preserves trailing spaces and omits the trailing newline. For string OUTPUT, match `xtend-gen` exactly (`\s` / `\` escapes). When the delta is provably behaviour-inert (e.g. a parser "no syntax errors" assertion) a clean text block is fine — say so in the commit/PR. | | **`final`-on-locals consistency** | Not an enforced gate, but keep locals consistently `final` within a file; mixed `final`/non-`final` siblings is a readability nit only. | | **Don't carry `xbase.lib` types into migrated Java** | The `->` pair operator compiles to `org.eclipse.xtext.xbase.lib.Pair` — an Xtend runtime type. Don't keep it in the `.java`: replace with a small `private record` (named fields, accepts `null`) or `java.util.Map.entry` — but `Map.entry` **rejects null** keys/values, so use a record when nulls are possible. Bonus: a non-generic record vararg drops the `@SafeVarargs` a `Pair<…>` vararg required. Migrating off Xtend means migrating off `xbase.lib` too. | +| **`operator_add` (`+=` over a collection) skips nulls** | The single most dangerous inferrer trap — see [`rules/10-jvm-model-inferrer.md`](../rules/10-jvm-model-inferrer.md) §10.5. `list += map[toField/toMethod/...]` binds to `JvmTypesBuilder.operator_add`, which drops null elements; the factories return null on a null name/type. A bare `addAll`/loop-`add` leaks the null → `IllegalArgumentException: element: null` in production, past all gates and tests. Shipped once (`FormatJvmModelInferrer.inferConstants`). Always `filterNull` / null-guard a null-capable collection add. | +| **Behavioural equivalence ≠ literal-token equivalence** | When verifying a migration (or reconciling two migrations) against `xtend-gen`, do NOT decide "faithful" by whether a token (`filterNull`, a `catch`, a charset arg) textually appears. `xtend-gen` semantics can live in a call whose Java equivalent needs *extra* code (e.g. `operator_add`'s null-skip → an explicit `filterNull`; §10.5). **Test-validate every behavioural divergence — the test gate is the arbiter, not a token diff.** The `filterNull`-looks-spurious trap cost a real regression when trusted without a test run. |