fix(codegen): use ASTRewrite instead of string manipulation for member insertion #94

Merged

hauschel.fred merged 1 commit from fix/52-codegen-astrewrite into main

2026-09-07 20:55:43 +00:00

hauschel.fred commented

2026-09-07 19:45:49 +00:00

Collaborator

Summary

CodeGenerationTools inserted generated methods, constructors and getters/setters
by searching source.lastIndexOf('}') and splicing text at that offset. This
breaks for nested/inner classes and any content after the target type's real
closing brace: the naive search finds the LAST } in the whole file, which
belongs to the enclosing type rather than the (possibly nested) target type,
silently inserting the new member into the wrong class.

The triage for this issue recounted 8 occurrences of this pattern (not the
4 methods named in the issue text) — this PR migrates all 8:
addMethod, generateMissingMethods (interface stubs), generateGettersSetters,
generateConstructor (two branches), generateEqualsHashCode, generateToString,
generateDelegateMethods.

Fix

Replace the string-splice with structural insertion via ASTRewrite, following
the pattern already established in this file's ConvertToLambdaRefactoring/
ImportOrganizer:

Parse the compilation unit into a DOM AST (ASTParser)
Locate the target type's AbstractTypeDeclaration by the IType's name-range
offset (works for nested/inner types, since the DOM tree mirrors nesting by
source position — no binding resolution needed)
Insert the generated source as a string placeholder into the correct
ListRewrite(BODY_DECLARATIONS_PROPERTY) position — last, or after the last
field declaration for constructors (preserving prior "insert after fields"
behavior)
Apply via the working-copy pattern (getWorkingCopy / applyTextEdit /
commitWorkingCopy)

Out of scope

addImplementsClause() (used by jdt_implement_interface) still does its own
manual string scanning for the implements clause. It wasn't part of the 8
lastIndexOf('}') sites this issue's triage tracked (it's a different kind of
string manipulation — extends/implements clause editing, not body-declaration
insertion), so it's intentionally untouched here. README.md's "Bekannte
Einschränkungen" table already has a row for this (currently mislabeled as
jdt_extract_interface, which doesn't apply — jdt_extract_interface uses
JDT's ExtractInterfaceProcessor and never did string manipulation; the row
actually describes addImplementsClause/jdt_implement_interface). Left as-is
since deciding whether to keep, correct, or re-file that row is a judgement
call above this package's scope.

Test plan

Bug reproduced first, against the pre-fix binary: jdt_add_method on a
nested class (Outer.Nested) landed the new method in the enclosing
Outer class instead of Nested
Same fixture, fixed binary: method lands correctly inside Nested
Ran jdt_generate_getters_setters, jdt_generate_constructor,
jdt_generate_equals_hashcode, jdt_generate_tostring in sequence
against a fixture class — all landed at the correct position
(constructor after fields, others appended), resulting file javac-compiles
mvn -pl org.naturzukunft.jdt.mcp -am compile green
tests/smoke-test.sh green (9/9)
mvn clean package (once) to build the product used for the above tests

Closes #52

🤖 Generated with Claude Code

https://claude.ai/code/session_018q6miiQHwQYFgZBy71aUhL

## Summary `CodeGenerationTools` inserted generated methods, constructors and getters/setters by searching `source.lastIndexOf('}')` and splicing text at that offset. This breaks for nested/inner classes and any content after the target type's real closing brace: the naive search finds the LAST `}` in the whole file, which belongs to the enclosing type rather than the (possibly nested) target type, silently inserting the new member into the wrong class. The triage for this issue recounted **8** occurrences of this pattern (not the 4 methods named in the issue text) — this PR migrates all 8: `addMethod`, `generateMissingMethods` (interface stubs), `generateGettersSetters`, `generateConstructor` (two branches), `generateEqualsHashCode`, `generateToString`, `generateDelegateMethods`. ## Fix Replace the string-splice with structural insertion via `ASTRewrite`, following the pattern already established in this file's `ConvertToLambdaRefactoring`/ `ImportOrganizer`: - Parse the compilation unit into a DOM AST (`ASTParser`) - Locate the target type's `AbstractTypeDeclaration` by the `IType`'s name-range offset (works for nested/inner types, since the DOM tree mirrors nesting by source position — no binding resolution needed) - Insert the generated source as a string placeholder into the correct `ListRewrite(BODY_DECLARATIONS_PROPERTY)` position — last, or after the last field declaration for constructors (preserving prior "insert after fields" behavior) - Apply via the working-copy pattern (`getWorkingCopy` / `applyTextEdit` / `commitWorkingCopy`) ## Out of scope `addImplementsClause()` (used by `jdt_implement_interface`) still does its own manual string scanning for the `implements` clause. It wasn't part of the 8 `lastIndexOf('}')` sites this issue's triage tracked (it's a different kind of string manipulation — extends/implements clause editing, not body-declaration insertion), so it's intentionally untouched here. `README.md`'s "Bekannte Einschränkungen" table already has a row for this (currently mislabeled as `jdt_extract_interface`, which doesn't apply — `jdt_extract_interface` uses JDT's `ExtractInterfaceProcessor` and never did string manipulation; the row actually describes `addImplementsClause`/`jdt_implement_interface`). Left as-is since deciding whether to keep, correct, or re-file that row is a judgement call above this package's scope. ## Test plan - [x] Bug reproduced first, against the pre-fix binary: `jdt_add_method` on a nested class (`Outer.Nested`) landed the new method in the enclosing `Outer` class instead of `Nested` - [x] Same fixture, fixed binary: method lands correctly inside `Nested` - [x] Ran `jdt_generate_getters_setters`, `jdt_generate_constructor`, `jdt_generate_equals_hashcode`, `jdt_generate_tostring` in sequence against a fixture class — all landed at the correct position (constructor after fields, others appended), resulting file `javac`-compiles - [x] `mvn -pl org.naturzukunft.jdt.mcp -am compile` green - [x] `tests/smoke-test.sh` green (9/9) - [x] `mvn clean package` (once) to build the product used for the above tests Closes #52 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018q6miiQHwQYFgZBy71aUhL

hauschel.fred added 1 commit

2026-09-07 19:45:49 +00:00

fix(codegen): use ASTRewrite instead of string manipulation for member insertion 5e4f4e0e74

CodeGenerationTools inserted generated methods, constructors and getters/setters
by searching for source.lastIndexOf('}') and splicing text at that offset. This
breaks for nested/inner classes and any content after the target type's real
closing brace: the naive search finds the LAST '}' in the whole file, which
belongs to the enclosing type rather than the (possibly nested) target type,
silently inserting the new member into the wrong class.

Replace all 8 occurrences of this pattern with structural insertion via
ASTRewrite: parse the compilation unit into a DOM AST, locate the target
type's AbstractTypeDeclaration by its IType name-range offset, and insert
the generated source as a string placeholder into the correct
ListRewrite(BODY_DECLARATIONS_PROPERTY) position (last, or after the last
field declaration for constructors). Apply via the working-copy pattern
already used by ConvertToLambdaRefactoring/ImportOrganizer in this file.

Reproduced the bug first: jdt_add_method on a nested class landed the new
method in the enclosing class instead, confirmed against the pre-fix binary.
Same fixture verified correct placement after the fix, and that generated
code still compiles.

Note: addImplementsClause() (used by jdt_implement_interface) still does its
own manual string scanning for the implements-clause and is intentionally
out of scope here - it wasn't part of the 8 lastIndexOf('}') sites this issue
tracked, and the parallel triage flagged it as a follow-up.

Closes #52

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018q6miiQHwQYFgZBy71aUhL

hauschel.fred merged commit f2ca7e84c0 into main

2026-09-07 20:55:43 +00:00

hauschel.fred referenced this pull request from a commit

2026-09-07 20:55:45 +00:00

Merge pull request 'fix(codegen): use ASTRewrite instead of string manipulation for member insertion' (#94) from fix/52-codegen-astrewrite into main

hauschel.fred referenced this pull request

2026-09-07 21:45:01 +00:00

jdt_implement_interface: implements-Klausel wird per String eingefügt statt ASTRewrite #101

Commenting is not possible because the repository is archived.

No reviewers