Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .agents/skills/xtend-to-java/rules/00-decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ LOGGER.info("Processing file: %s".formatted(path));

Add at class level **only when the module's effective JDT settings have `nonExternalizedStringLiteral=warning`**.

To check: look at `<module>/.settings/org.eclipse.jdt.core.prefs`. If absent (most modules), check the project's shared settings at `ddk-configuration/.settings/org.eclipse.jdt.core.prefs`.

**Current DDK state:** all modules inherit `warning` from `ddk-configuration`, so all migrated classes currently need `@SuppressWarnings("nls")` at class level. If a test-specific settings profile is later added with `ignore` (as in ASMD), test classes would not need it.
To check: look at `<module>/.settings/org.eclipse.jdt.core.prefs`. If absent, check the shared settings project's `.settings/org.eclipse.jdt.core.prefs`.

```java
@SuppressWarnings("nls")
Expand Down
4 changes: 2 additions & 2 deletions .agents/skills/xtend-to-java/rules/01-imports-and-package.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import java.util.List;
import org.eclipse.xtext.testing.InjectWith;
import org.junit.jupiter.api.Test;

// Group 3: com.* (com.avaloq.* before com.google.*)
import com.avaloq.tools.ddk.check.core.test.util.CheckTestUtil;
// Group 3: com.* (com.example.* before com.google.*)
import com.example.mydsl.test.util.MyTestUtil;
import com.google.inject.Inject;
```

Expand Down
21 changes: 19 additions & 2 deletions .agents/skills/xtend-to-java/rules/04-templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ for (final String k : properties.keySet()) {
return builder;
```

**Tier 4 does not apply to every control-flow template.** A template whose `xtend-gen/` chain contains
`newLineIfNotEmpty()` after a dynamic value, a two-arg `append(value, indent)` of a possibly multi-line
value, or `appendImmediate(...)` **stays on the reference `StringConcatenation` chain** — none of the
three has a `StringBuilder`/text-block equivalent. Convert to `StringBuilder` (or a text block) only
when every `newLineIfNotEmpty()` in the chain follows a static literal tail (§4.8, rule 35).

## 4.2 Template control flow patterns

- `«IF condition»...«ENDIF»` → `if (condition) { builder.append(...); }`
Expand Down Expand Up @@ -151,8 +157,8 @@ string must NOT end with `\n`, use a **line continuation** `\` on the last conte
String s = """

check configuration mdlc
for com.avaloq.tools.dsl.labeldef.LabelDef {
catalog com.avaloq.tools.dsl.labeldef.validation.LabelDefCoreChecks { }
for com.example.mydsl.MyDsl {
catalog com.example.mydsl.validation.MyDslCoreChecks { }
}\
""";
```
Expand Down Expand Up @@ -243,6 +249,17 @@ these source-verified semantics decide what is safe:
- **Leave `appendImmediate(sep, indent)` loops untouched** — the separator insertion inspects
trailing segments; keep its surrounding append sequence as-is.

**When the chain stays as-is:** see the Tier 4 caveat above — `newLineIfNotEmpty()` after a dynamic value, two-arg `append(value, indent)` of a possibly multi-line value, or `appendImmediate(...)` keep the reference `StringConcatenation` chain (rule 35).

**Null values:** `StringConcatenation.append(null)` appends nothing, while `"%s".formatted(null)`
yields `"null"`. Wrap a nullable `String` moved into `.formatted()` with `Strings.emptyIfNull(value)`
(`org.eclipse.xtext.util.Strings`, String-only overload).
**Keep any `StringConcatenationClient`-valued or otherwise specially-rendered interpolation on the
`StringConcatenation` chain (rule 35)**; never convert it to `.formatted()`.
For other nullable values, use `Objects.toString(value, "")` (`java.util.Objects`) **only when
`toString()` is exactly what `append` would render** (boxed numbers, plain `CharSequence`); this
preserves the "append nothing" semantics for null.

**Verification:** each coalesced run must be proven byte-identical with an executable old-vs-new
harness over an input battery (empty / single-line / multi-line / newline-terminated / `%`-bearing
values).
2 changes: 1 addition & 1 deletion .agents/skills/xtend-to-java/rules/06-extension-methods.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Convert the field, then rewrite every call site:

- Field: `@Inject extension MyHelper helper` → `@Inject private MyHelper helper;`
- Call site: `obj.extensionMethod(args)` → `helper.extensionMethod(obj, args)` (the implicit receiver `obj` moves to the first parameter).
- If the extension field had no name (`@Inject extension CheckGeneratorNaming`), invent one following the convention: camelCase class name starting lowercase (`checkGeneratorNaming`).
- If the extension field had no name (`@Inject extension MyGeneratorNaming`), invent one following the convention: camelCase class name starting lowercase (`myGeneratorNaming`).

Tip: The `xtend-gen/` output shows exactly how the Xtend compiler resolved every extension call — use it as the reference.

Expand Down
26 changes: 16 additions & 10 deletions .agents/skills/xtend-to-java/rules/09-misc-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ Generates a constructor taking all final fields as parameters. Write it manually

### `@Tag`

Applies only if the project uses DDK's test framework (`@Tag` / `TagExtension` from `com.avaloq.tools.ddk.xtext.test.core`).

Assigns sequential integers (starting at `TagCompilationParticipant.COUNTER_BASE = 10000`) to test
marker fields at Xtend compile time via `TagCompilationParticipant`.

Standard Java APT (`AbstractProcessor`) **cannot** replicate this — it can only generate new source
files, not modify initializers of existing fields. Use `TagExtension` instead: a JUnit 5
`BeforeEachCallback` in `com.avaloq.tools.ddk.xtext.test.core` that performs the same sequential
`BeforeEachCallback` that performs the same sequential
assignment at runtime via reflection.

**Migration steps:**
Expand All @@ -84,10 +86,7 @@ assignment at runtime via reflection.
```java
@ExtendWith({InjectionExtension.class, TagExtension.class})
```
3. Add the import:
```java
import com.avaloq.tools.ddk.xtext.test.TagExtension;
```
3. Import `TagExtension` from the test framework's package.

**Why not `final`?** `TagExtension` uses `Field.setInt()` to write the value at runtime.
`Field.setInt()` on a `final` field throws `IllegalAccessException` even after `setAccessible(true)`
Expand Down Expand Up @@ -138,7 +137,14 @@ public class MyFormatter extends AbstractFormatter {
Rules:
- **Keep the `_` prefix** — the Xtext runtime resolves dispatch by name.
- **Suppress at class level**: `@SuppressWarnings({"checkstyle:MethodName", "PMD.UnusedFormalParameter"})`
- Order `instanceof` checks from most specific to least specific.
- **Take the case ORDER from `xtend-gen/`, never from source order.** Xtend sorts dispatch cases by
type specificity, not by declaration order — e.g. `OperationCall` and `TypeSelectExpression` extend
both `Expression` and `FeatureCall`, so their relative position is not what the `.xtend` suggests.
Order `instanceof` checks most specific first, exactly as `xtend-gen/` did.
- **Keep the terminal `else { throw new IllegalArgumentException("Unhandled parameter types: …"); }`
exactly as `xtend-gen/` emits it**, even after a `!= null` / `== null` pair. Java's definite-return
analysis does not treat that pair as exhaustive; deleting the terminal throw leaves a non-void
dispatcher without a return on every path.
- If the original `dispatch` had `override`, add `@Override` to the **dispatcher**, not the `_` methods.
- The dispatcher parameter type should be the common supertype (often `Object` or `EObject`).
- If the parent class has dispatch methods with the same name, the dispatcher must call `super._methodName()` for types not handled locally.
Expand Down Expand Up @@ -181,14 +187,14 @@ the **data contract** first:

- if the API or format supplies an encoding, honour it — e.g. pass `file.getCharset()` when reading an Eclipse
`IFile` (the `InputStreamReader(InputStream, String)` overload accepts that value);
- for repository-owned text governed by `ddk-parent/pom.xml`'s UTF-8 project encoding, use
`java.nio.charset.StandardCharsets.UTF_8`:
- for repository-owned text governed by the parent POM's `project.build.sourceEncoding`, use
the corresponding charset; for UTF-8, use `java.nio.charset.StandardCharsets.UTF_8`:
`new InputStreamReader(stream, StandardCharsets.UTF_8)`;
- for opaque external data with no documented encoding, do **not** guess UTF-8 from the Java source-encoding
setting. Establish the contract. If platform-default encoding is genuinely part of that contract, use
`Charset.defaultCharset()` explicitly and record why preserving it is intentional.

This is a sanctioned divergence from `xtend-gen` only when the chosen charset follows a verified contract;
call it out and test it. Do not use a lint warning as blanket permission for an unrelated behaviour change.
Two legacy `// NOPMD` suppressions of this rule exist in hand-written code (`CheckPreferencesHelper`,
`XtextGMFResourceUtil`); they are grandfathered, not a precedent for migrations.
Any legacy `// NOPMD` suppressions of this rule in hand-written code are grandfathered,
not a precedent for migrations.
43 changes: 25 additions & 18 deletions .agents/skills/xtend-to-java/rules/10-jvm-model-inferrer.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
# JVM model inferrer (JvmTypesBuilder)

Xtend model inferrers (`class X extends AbstractModelInferrer` with `def dispatch infer(...)`)
build Java types through the `JvmTypesBuilder` extension DSL. The migrated
`FormatJvmModelInferrer.java` (`com.avaloq.tools.ddk.xtext.format/src/com/avaloq/tools/ddk/xtext/format/jvmmodel/`)
is the canonical worked example — read it in full before migrating another inferrer.
build Java types through the `JvmTypesBuilder` extension DSL. Read the inferrer's `.xtend` source
and `xtend-gen/` output in full before migrating it.

## 10.1 Core mapping

Expand All @@ -12,28 +11,26 @@ is the canonical worked example — read it in full before migrating another inf
| `@Inject extension JvmTypesBuilder` | `@Inject private JvmTypesBuilder jvmTypesBuilder;` — every extension call becomes explicit (`jvmTypesBuilder.toClass(...)`) |
| `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.<JvmGenericType>accept(cls, initializer)` where `initializer` is a `Procedure1<JvmGenericType>` (see `FormatJvmModelInferrer._infer`) |
| `acceptor.accept(cls, [ ... ])` | `acceptor.<JvmGenericType>accept(cls, initializer)` where `initializer` is a `Procedure1<JvmGenericType>` |
| `members += x` / `superTypes += x` / `annotations += x` | `it.getMembers().add(x)` / … — **only when `x` is provably non-null**; both `JvmTypesBuilder.operator_add` overloads skip nulls (single element and collection), so see §10.4 before translating any `+=` |
| `x.toMethod(name, type) [ ... ]` | `jvmTypesBuilder.toMethod(x, name, type, initializer)` with a `Procedure1<JvmOperation>` (see `FormatJvmModelInferrer.inferGetGrammarAccess`) |
| `x.toMethod(name, type) [ ... ]` | `jvmTypesBuilder.toMethod(x, name, type, initializer)` with a `Procedure1<JvmOperation>` |
| `x.toField(name, type) [ ... ]` / `x.toParameter(name, type)` | `jvmTypesBuilder.toField(x, name, type, initializer)` / `jvmTypesBuilder.toParameter(x, name, type)` |
| `typeRef(T)` / `typeRef(name)` | `_typeReferenceBuilder.typeRef(...)` — the protected field inherited from `AbstractModelInferrer`; for lookups needing a context object use `typeReferences.getTypeForName(name, context)` |
| `documentation = '''...'''` | `jvmTypesBuilder.setDocumentation(it, "...".formatted(...))` (see `FormatJvmModelInferrer.inferClass`) |
| `documentation = '''...'''` | `jvmTypesBuilder.setDocumentation(it, "...".formatted(...))` |
| `static = true` / `visibility = PROTECTED` / `abstract = true` | `it.setStatic(true)` / `method.setVisibility(JvmVisibility.PROTECTED)` / `it.setAbstract(true)` |
| `initializer = expr` (on a field) | set inside the field's initializer `Procedure1` via the corresponding setter/`jvmTypesBuilder` call — read `xtend-gen/` for the exact form |

## 10.2 Method bodies

Xtend assigns bodies two ways; both become `jvmTypesBuilder.setBody(method, ...)`:

- `body = [append('''...''')]` (procedure form) → `Procedure1<ITreeAppendable>` that appends —
the form the migrated file uses throughout:
- `body = [append('''...''')]` (procedure form) → `Procedure1<ITreeAppendable>` that appends:
```java
final Procedure1<ITreeAppendable> body = (final ITreeAppendable appendable) -> {
appendable.append("return (%sGrammarAccess) super.getGrammarAccess();".formatted(...));
appendable.append("return %s;".formatted(expression));
};
jvmTypesBuilder.setBody(method, body);
```
(see `FormatJvmModelInferrer.inferGetGrammarAccess`)
- `body = '''template'''` (template form) → the Xtend compiler emits the
`StringConcatenationClient` overload of `setBody`. Either keep that overload (check
`xtend-gen/`) or convert to the `Procedure1<ITreeAppendable>` form with the template text
Expand All @@ -43,10 +40,8 @@ Xtend assigns bodies two ways; both become `jvmTypesBuilder.setBody(method, ...)
## 10.3 Gate notes specific to inferrers

- The inference closures are long by design; bracket the class with
`// CHECKSTYLE:CHECK-OFF LambdaBodyLength the model-inference closures mirror the Xtext JvmTypesBuilder API and are kept whole`
(see the class-level suppression in `FormatJvmModelInferrer`).
- Emitted Java source fragments are repeated literals — `// CHECKSTYLE:CONSTANTS-OFF` applies
(see the class-level suppression in `FormatJvmModelInferrer`).
`// CHECKSTYLE:CHECK-OFF LambdaBodyLength the model-inference closures mirror the Xtext JvmTypesBuilder API and are kept whole`.
- Emitted Java source fragments are repeated literals — `// CHECKSTYLE:CONSTANTS-OFF` applies.
- `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 the add — but read §10.4 first
Expand Down Expand Up @@ -75,6 +70,11 @@ argument does not by itself trigger a null return in the field/method/parameter/
exact overload used rather than treating this list as a substitute for source inspection. Any local helper
with a `return null` fall-through (a `switch`/`if` that doesn't match) is a trigger too.

| Producer | Nullability | Add form |
|---|---|---|
| `JvmTypeReferenceBuilder.typeRef(Class, ...)` / `typeRef(String, ...)` | **Provably non-null** — a lookup miss returns `createUnknownTypeReference(name)` (bytecode: `findDeclaredType → ifnonnull → createUnknownTypeReference`, both paths `areturn`) | `superTypes += typeRef(X)` → plain `it.getSuperTypes().add(_typeReferenceBuilder.typeRef(X))` |
| `toField` / `toMethod` / `toParameter` with a nullable **name** (e.g. a model element's `getName()`) | Nullable — the builders guard source element **and** name | Guarded add / `Objects::nonNull` filter |

So the faithful Java of any `+=` whose right-hand side can be null is a guarded add:

```java
Expand Down Expand Up @@ -104,10 +104,17 @@ return null (nullable source element or name, or a `return null` branch)? If yes
guard / `Objects::nonNull` filter. A bare `add`/`addAll` over a null-capable producer is a faithfulness
regression.

> Real shipped example: `FormatJvmModelInferrer.inferConstants` used a bare add for
> `members += allConstants.map[createConstant]`, although `createConstant` returns null for a value-less
> constant. The guard now in that method and its regression test are the canonical fix; the defect escaped
> the gates because no earlier test supplied the null-producing input.
> Example: a bare `members += list.map[create]` skips nulls even without an explicit `filterNull`.
> If `create` returns null for some inputs, translating it to an unguarded `add`/`addAll` throws.
> Preserve the null-skip with a guarded add:
> ```java
> for (final Input input : list) {
> final JvmMember member = create(input);
> if (member != null) { it.getMembers().add(member); }
> }
> ```
> Test the null-producing inputs explicitly: static gates and tests that never supply them cannot
> detect this regression.

## 10.5 Verification

Expand Down
Loading