Skip to content

Commit 5b71cc1

Browse files
joaodinissfclaude
andcommitted
docs: skill review trims and staleness fixes
Applies the adversarially-verified findings of the skill review (13-agent Opus panel over the stacked #1453+#1461 state; 25 confirmed findings): Cuts: duplicate string-decision list in 00-decisions; duplicated commit-message fence in multi-file-batch; four known-pitfalls rows that were verbatim checklist echoes; copyright restatements reduced to pointers (formatting-and-commit stays canonical). Staleness/contradictions: one-file-conversion step 8 and the checklist final invariant now teach the two-step rename+translate scheme; stale hard-coded rule counts dropped; overview slice branch now based on upstream/master; import order stated once (java.* -> org.* -> com.*, junit inside org, static block separate) and aligned in rule 11 and the pitfalls row; == equality rule gains the primitive-operand carve-out (enums stay object-form); rollback recipe corrected for two-step slices (reset --hard HEAD~N / revert both shas, never -m 1) and stated once; SKILL.md lambda cheat-sheet aligned with 07 (expression form); Pairs guidance defers to the no-xbase.lib pitfall; dead "ledger's Method 1" pointer removed. Small additions: trailing-\s trap noted at point of use in 4.3; stable-rule-ID legend in the checklist header; overview Step 0 leads with the bash listing; examples no longer claim their whitespace WAS verified (they illustrate; real migrations must verify) and use the PMD-safe explicit StringBuilder capacity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ANGM7L3BaEGfGUmPVKRt4
1 parent 3c65b3c commit 5b71cc1

14 files changed

Lines changed: 57 additions & 55 deletions

.agents/skills/xtend-to-java/SKILL.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,15 @@ Use this table for quick mechanical transforms. Full details in the rule files.
105105
| `obj?.method()` | `obj != null ? obj.method() : null` (or guard clause) |
106106
| `x ?: default` | `x != null ? x : default` |
107107
| `===` / `!==` (identity) | `==` / `!=` |
108-
| `==` / `!=` (equality) | `.equals()` / `!Objects.equals(a, b)` |
108+
| `==` / `!=` (equality, object operands) | `.equals()` / `!Objects.equals(a, b)` — primitives keep `==`/`!=` (see [`rules/08`](rules/08-operator-overloads.md) §8.1) |
109109
| `a..b` (range) | `IntStream.rangeClosed(a, b)` |
110110

111111
### Lambdas and collections
112112

113113
| Xtend | Java |
114114
|-------|------|
115-
| `[param \| body]` | `(param) -> { return body; }` |
116-
| `[body]` (implicit `it`) | `(it) -> { return body; }` — name the parameter explicitly |
115+
| `[param \| body]` | `(param) -> body` (braces + `return` only for multi-statement) |
116+
| `[body]` (implicit `it`) | `(it) -> body` — name the parameter explicitly |
117117
| `list.filter[condition]` | `list.stream().filter(x -> condition).toList()` |
118118
| `list.map[transform]` | `list.stream().map(x -> transform).toList()` |
119119
| `list.forEach[action]` | `list.forEach(x -> action)` (or `for` loop) |

.agents/skills/xtend-to-java/examples/00-basic-generator.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,4 @@ public class MyGenerator {
101101
- **Template → StringBuilder** (tier 4) because the template has `«IF»` and `«FOR»` control flow.
102102
- **`it` parameter renamed** to `model` (descriptive name).
103103
- **`Iterables.filter(iter, Type.class)`** kept as Guava — genuinely more concise for type-safe filtering.
104-
- **Whitespace verified against `xtend-gen/`** — the template output was confirmed by reading the generated code.
104+
- **Whitespace must be verified against `xtend-gen/`**for a real migration, confirm the template output by reading the generated code (this example illustrates the shape).

.agents/skills/xtend-to-java/examples/01-template-with-for-loop.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ When the body is a simple expression, use `Collectors.joining()`:
1616

1717
```java
1818
public CharSequence renderArgs(final List<Argument> args) {
19-
final StringBuilder builder = new StringBuilder();
19+
final StringBuilder builder = new StringBuilder(512);
2020
builder.append("(");
2121
builder.append(args.stream()
2222
.map(a -> a.getType() + " " + a.getName())
@@ -32,7 +32,7 @@ When the body has multi-statement logic, use a flag:
3232

3333
```java
3434
public CharSequence renderArgs(final List<Argument> args) {
35-
final StringBuilder builder = new StringBuilder();
35+
final StringBuilder builder = new StringBuilder(512);
3636
builder.append("(");
3737
boolean first = true;
3838
for (final Argument a : args) {
@@ -52,4 +52,4 @@ public CharSequence renderArgs(final List<Argument> args) {
5252
- The `«FOR … SEPARATOR ", "»` block became either a stream with `Collectors.joining(", ")` or a boolean flag.
5353
- The `«a.type»` and `«a.name»` interpolations became `a.getType()` and `a.getName()`.
5454
- The trailing newline (implicit `\n` at end of template `'''`) is preserved by explicit `"\n"`.
55-
- **Whitespace was verified against `xtend-gen/`** — the template starts with `(` directly (no leading newline because the first `'''` line has content after the opening mark).
55+
- **Whitespace must be verified against `xtend-gen/`** note the template starts with `(` directly (no leading newline because the first `'''` line has content after the opening mark).

.agents/skills/xtend-to-java/rules/00-decisions.md

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Use Java standard library for collections and streams. Keep Guava only where it
4444

4545
## String building — 4-tier decision tree
4646

47-
Choose the most readable Java idiom based on the template pattern:
47+
Choose the most readable Java idiom based on the template pattern (evaluate top-down, first match wins):
4848

4949
| # | Template pattern | Java idiom |
5050
|---|---|---|
@@ -53,12 +53,6 @@ Choose the most readable Java idiom based on the template pattern:
5353
| 3 | Interpolation, no control flow | `.formatted()` (text block if multi-line, literal if single-line) |
5454
| 4 | Control flow (`«IF»`, `«FOR»`) | `StringBuilder` with explicit `if`/`for` |
5555

56-
**Decision rules — in order:**
57-
1. **No interpolation, single line** → string literal
58-
2. **No interpolation, multi-line** → text block
59-
3. **Interpolation, no control flow**`.formatted()` (on text block if multi-line, on literal if single-line)
60-
4. **Control flow**`StringBuilder` — always
61-
6256
**`.formatted()` limitations — fall back to concatenation ONLY when:**
6357
- The interpolated expression is glued to adjacent text with no whitespace/delimiter boundary, making `%s` ambiguous (e.g., `"pre" + expr + "suf"` where `"pre%ssuf"` is confusing)
6458
- The template contains literal `%` characters (would need escaping as `%%`)

.agents/skills/xtend-to-java/rules/01-imports-and-package.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,23 @@ Keep identical. Add a trailing semicolon if missing.
1616

1717
## 1.3 Import order convention
1818

19-
Follow the project's Eclipse import order — **standard/framework first, project-specific second**:
19+
Canonical order (ground truth from migrated files): **`java.*``org.*``com.*`**, one blank
20+
line between groups, alphabetical by full path within each group. `javax.*` sorts with `java.*`;
21+
`junit.*` sorts inside `org.*` (it is `org.junit.*`). Static imports form their own block.
2022

2123
```java
22-
// Group 1: org.*, java.*, javax.*, junit.* (standard/framework)
23-
import org.eclipse.xtext.testing.InjectWith;
24-
import org.junit.jupiter.api.Test;
24+
// Group 1: java.* / javax.*
2525
import java.util.List;
2626

27-
// Blank line separator
27+
// Group 2: org.* (incl. org.junit.*)
28+
import org.eclipse.xtext.testing.InjectWith;
29+
import org.junit.jupiter.api.Test;
2830

29-
// Group 2: com.* (project-specific: com.avaloq.*, com.google.*, etc.)
31+
// Group 3: com.* (com.avaloq.* before com.google.*)
3032
import com.avaloq.tools.ddk.check.core.test.util.CheckTestUtil;
3133
import com.google.inject.Inject;
3234
```
3335

34-
Within each group, imports are sorted alphabetically. There is always exactly one blank line between the two groups.
35-
3636
## 1.4 Class declaration
3737

3838
- Xtend classes are `public` by default. Make this explicit in Java.

.agents/skills/xtend-to-java/rules/04-templates.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ The `\` suppresses the line terminator. Place the closing `"""` at column 0 to p
162162
**When to use this pattern:** check the `xtend-gen/` output. If the original string starts with `\n`
163163
and does NOT end with `\n`, the `\` escape is the correct approach.
164164

165+
Text blocks also strip trailing whitespace on every content line; if `xtend-gen/` shows
166+
significant trailing spaces, preserve them with the `\s` escape. See
167+
[`workflow/known-pitfalls.md`](../workflow/known-pitfalls.md) (text-block row).
168+
165169
## 4.4 Return type
166170

167171
Methods that return templates should return `CharSequence` (matches `StringBuilder`).
@@ -241,4 +245,4 @@ these source-verified semantics decide what is safe:
241245

242246
**Verification:** each coalesced run must be proven byte-identical with an executable old-vs-new
243247
harness over an input battery (empty / single-line / multi-line / newline-terminated / `%`-bearing
244-
values), per the ledger's Method 1.
248+
values).

.agents/skills/xtend-to-java/rules/08-operator-overloads.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- `===` (Xtend identity-equals) → `==` (Java)
66
- `!==` (Xtend identity-not-equals) → `!=` (Java)
77
- `==` in Xtend is `.equals()` — convert to `.equals()` or `Objects.equals()` (use `Objects.equals()` when either operand could be null).
8+
- **Applies to object/boxed operands.** `==`/`!=` between primitive-typed operands (int, long, short, byte, char, float, double, boolean) compile to Java `==`/`!=`, not `.equals()` — check operand types in `xtend-gen/` first. (Enums are NOT primitives: enum `==` follows the object form shown in `xtend-gen/`.)
89

910
## 8.2 Null-safe navigation `?.`
1011

.agents/skills/xtend-to-java/rules/09-misc-syntax.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ Rules:
155155
- **Multiple return paths**: Xtend implicitly returns the last expression. Add explicit `return` on **every** non-void path.
156156
- **`class` keyword as literal**: `SomeClass` used as a class literal → `SomeClass.class`.
157157
- **Static method reference `::`**: `ClassName::methodName``ClassName.methodName()` (or keep as a Java method reference where the receiving API accepts one).
158-
- **Pairs**: `key -> value` (in pair-construction context) → `Pair.of(key, value)` or `Map.entry(key, value)`.
158+
- **Pairs**: `key -> value` compiles to `org.eclipse.xtext.xbase.lib.Pair` — never keep it in migrated Java. Use a small `private record` (nulls OK) or `Map.entry` (rejects null); see [`workflow/known-pitfalls.md`](../workflow/known-pitfalls.md) (xbase.lib row).
159159
- **`^keyword`** (escaped reserved word in Xtend): Drop the `^` — most Xtend escapes aren't Java keywords.
160160

161161
## 9.9 Guice DI

.agents/skills/xtend-to-java/workflow/formatting-and-commit.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,10 @@ refactor: drop Xtend build infrastructure from <plugin name> # when applicable
106106
The rename commit body must state that it is a pure `git mv` and intentionally non-compiling.
107107

108108
PR title: `refactor: migrate Xtend to Java - <plugin name>`.
109+
110+
## Rollback
111+
112+
Undo the whole slice: `git reset --hard HEAD~N` where N = commits in the slice (2, or 3 with
113+
infra). Plain `HEAD~1` reverts only the translate and strands the pure-`git mv` rename commit —
114+
a `.java` holding Xtend that won't compile. If already pushed: `git revert <translate-sha>
115+
<rename-sha>` — not `-m 1` (the rebase-merged commits are not merge commits).

.agents/skills/xtend-to-java/workflow/known-pitfalls.md

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,23 @@ Consolidated table of common mistakes and their fixes. Review before and after e
1111
| **Implicit returns** | Xtend methods return the last expression. The compiler catches missing returns but not wrong ones. |
1212
| **Property access vs getter** | Xtend `obj.name` may call `getName()`. In Java, write `obj.getName()` explicitly. Check `xtend-gen/` if unsure. |
1313
| **`CoreException` handling** | Xtend silently wraps checked exceptions. Java doesn't. Add explicit `try/catch` — the `xtend-gen/` file shows what was generated. |
14-
| **`val` leaks** | Never use `var`. Use the explicit type. |
1514
| **Invented Javadoc** | Never add **class/member Javadoc** that wasn't in the original. This is a migration, not a rewrite. (The file copyright header is the one exception — see next row — it is always normalised, not preserved.) |
1615
| **Generated supertypes live in `src-gen/`, not `xtend-gen/`** | When the class extends/overrides a generated `Abstract*` base (Module/Setup/runtime/UI), read that base in `src-gen/` (committed, present without a build — unlike `xtend-gen/`) for inherited constructor signatures, the real `@Override` targets, and the return types Xtend inferred. Don't guess the supertype API. |
17-
| **Copyright header ≠ "preserve original"** | The file MUST start with the Avaloq banner header, **replacing** whatever the source had — including a `/* generated by Xtext x.y */` stub marker or a `/** … */` Javadoc-style copyright block (these are the easy ones to wrongly "preserve", especially on IDE module/setup stubs). Normalising to the banner is required, not inventing. Match a sibling `.java` in the module. |
18-
| **Missing `@throws` tags** | When Java migration adds `throws` and method already has Javadoc, Checkstyle requires `@throws`. Add it; don't create Javadoc just for the tag. |
19-
| **Duplicate string literals** | Checkstyle flags strings appearing 2+ times. In tests, extract to constants. In generators, use `CHECKSTYLE:CONSTANTS-OFF/ON`. |
16+
| **Copyright header ≠ "preserve original"** | Always normalise to the Avaloq banner, replacing whatever the source had — see [`formatting-and-commit.md`](./formatting-and-commit.md) §Copyright header. |
2017
| **`@Data` / `@Accessors`** | These generate code at compile time. The `xtend-gen/` output shows exactly what — copy equals/hashCode/toString/getters from there. |
2118
| **`@Tag` fields must not be `final`** | `TagExtension` assigns tag values via `Field.setInt()` at runtime. `Field.setInt()` on a `final` field fails on Java 9+ even after `setAccessible(true)`. IDE formatters and save-actions silently add `final` to `int` fields — always strip it from `@Tag` fields. See [`rules/09-misc-syntax.md`](../rules/09-misc-syntax.md) §9.6. |
2219
| **`BasicEList` in generic code** | Needs explicit type parameter — `new BasicEList<X>()`. |
2320
| **StringBuilder in `xtend-gen/`** | If `xtend-gen/` has `StringConcatenation` but Xtend has a template, that's the signal to use text block or `.formatted()` (tier 1–3) or `StringBuilder` (tier 4). |
2421
| **Non-parameterized logging** | Xtend files often have `"msg" + x` in log calls. Fix to `{}` placeholders. |
2522
| **PMD missing type-resolution** | Always `compile` before `pmd:check` or you'll miss `MissingOverride`, `LooseCoupling` etc. |
2623
| **`--fail-at-end` hides failures** | Check the final BUILD line, not intermediate output. |
27-
| **Dispatch method names** | Keep underscores. Suppress with `@SuppressWarnings`. Never rename. |
2824
| **IDE save actions** | "Organize Imports" in Eclipse may trigger save actions that auto-convert string concatenation to text blocks. Auto-conversion produces wrong results. Review `git diff` after any IDE action. |
29-
| **Import order** | Two groups (blank line between, no wildcards): framework (`java.*`/`javax.*`/`org.*`) first, then `com.*` alphabetically — so `com.avaloq.*` precedes `com.google.*`. Not enforced by checkstyle (no `ImportOrder` module); wrong order is diff churn only. |
25+
| **Import order** | See [`rules/01-imports-and-package.md`](../rules/01-imports-and-package.md) for the canonical order. Not enforced by checkstyle (no `ImportOrder` module); wrong order is diff churn only`com.avaloq.*` precedes `com.google.*`. |
3026
| **Eclipse CLI formatter** | Does NOT organize imports — only code formatting. Import order must be correct from the start. |
3127
| **IllegalCatch / IllegalThrows** | checkstyle `IllegalCatch` bans `catch (Exception/Throwable/RuntimeException)` — use the specific type, or a multi-catch (`catch (BadLocationException \| TemplateException e)`) re-thrown as `new IllegalStateException(e)`. `IllegalThrows` bans `throws Throwable/RuntimeException/Error` (plain `throws Exception` IS allowed — acceptable on a `@Test` when the JUnit-invoked API declares it; otherwise narrow to the actual checked type). Don't suppress `PMD.AvoidCatchingGenericException`. |
32-
| **Rollback** | If already pushed: `git revert -m 1 <sha>`. If local only: `git reset --hard HEAD~1`. After reverting, build and test. |
28+
| **Rollback** | A slice is 2-3 commits — plain `HEAD~1` strands the rename commit. Use the recipe in [`formatting-and-commit.md`](./formatting-and-commit.md) §Rollback. After reverting, build and test. |
3329
| **`ByteArrayInputStream.close()`** | It's a no-op. Safe to remove entirely. |
34-
| **`==` in Xtend** | Xtend `==` is `.equals()`, not identity. Convert to `.equals()` or `Objects.equals()`. Only `===`/`!==` are identity. |
30+
| **`==` in Xtend** | On object/boxed operands, Xtend `==` is `.equals()` — convert to `.equals()`/`Objects.equals()`; only `===`/`!==` are identity. Between primitive-typed operands it compiles to Java `==` — check operand types in `xtend-gen/` (see [`rules/08-operator-overloads.md`](../rules/08-operator-overloads.md) §8.1). |
3531

3632
## Learnings from the per-module migration campaign
3733

0 commit comments

Comments
 (0)