fix(refactoring): single createChange() call and consistent error filtering #96

Merged

hauschel.fred merged 1 commit from fix/61-51-refactoring-status into main

2026-09-07 20:56:38 +00:00

hauschel.fred commented

2026-09-07 20:15:32 +00:00

Collaborator

Summary

createChange() was called twice per invocation (once in the preview branch,
once in the execute branch) in extractMethod, moveType, both inline
variants, changeMethodSignature, encapsulateField and introduceParameter
— same statement duplicated on both sides of the preview/execute split.
Computed once now, branch on previewOnly afterwards.
encapsulateField and introduceParameter gated on raw status.hasError()
instead of RefactoringSupport.getRealErrors(), unlike every other
refactoring method here — headless-mode participant/potential-match noise
could get reported as a blocking error. Both now filter through
getRealErrors() like the rest of the file.

Closes #61

Closes #51

Basis

Stacked on PR #95 (fix/79-move-type-cross-module), not yet merged to main.
Diff should be reviewed against that branch, not main.

Test plan

mvn clean compile after branch switch (avoid stale-target/ phantom errors)
mvn -pl org.naturzukunft.jdt.mcp -am compile
mvn clean package (product build, once)
tests/refactoring-test.sh against the packaged binary — 2/2 passed (also
re-confirms #79/#77 from the stacked base)
tests/smoke-test.sh against the packaged binary — 9/9 passed
Manual jdt_encapsulate_field red/green check against a fixture-parent
copy: temporarily reverted only the getRealErrors() line, rebuilt,
confirmed the code path in question; restored, rebuilt, confirmed
identical diff to the committed fix (see "Hürden" note below for why a
true false-positive repro wasn't reachable)

Found while verifying, not part of this fix

jdt_encapsulate_field throws an unconditional NullPointerException in
headless mode on every call, unrelated to #51/#61:
ProjectTemplateStore.load() → fInstanceStore is null when JDT tries to
render the generated getter/setter body from the code-template store
(SelfEncapsulateFieldRefactoring.checkFinalConditions() →
addGetterSetterChanges() → createGetterMethod()). Happens before the
status.hasError()/getRealErrors() check this PR touches is ever reached,
so a true end-to-end false-positive repro for #51 wasn't reachable — verified
the logic is correct and consistent with the rest of the file instead. This
predates this PR and needs its own issue (headless-mode template-store
bootstrapping).

## Summary - `createChange()` was called twice per invocation (once in the preview branch, once in the execute branch) in `extractMethod`, `moveType`, both inline variants, `changeMethodSignature`, `encapsulateField` and `introduceParameter` — same statement duplicated on both sides of the preview/execute split. Computed once now, branch on `previewOnly` afterwards. - `encapsulateField` and `introduceParameter` gated on raw `status.hasError()` instead of `RefactoringSupport.getRealErrors()`, unlike every other refactoring method here — headless-mode participant/potential-match noise could get reported as a blocking error. Both now filter through `getRealErrors()` like the rest of the file. Closes #61 Closes #51 ## Basis Stacked on PR #95 (`fix/79-move-type-cross-module`), not yet merged to `main`. Diff should be reviewed against that branch, not `main`. ## Test plan - [x] `mvn clean compile` after branch switch (avoid stale-`target/` phantom errors) - [x] `mvn -pl org.naturzukunft.jdt.mcp -am compile` - [x] `mvn clean package` (product build, once) - [x] `tests/refactoring-test.sh` against the packaged binary — 2/2 passed (also re-confirms #79/#77 from the stacked base) - [x] `tests/smoke-test.sh` against the packaged binary — 9/9 passed - [x] Manual `jdt_encapsulate_field` red/green check against a fixture-parent copy: temporarily reverted only the `getRealErrors()` line, rebuilt, confirmed the code path in question; restored, rebuilt, confirmed identical diff to the committed fix (see "Hürden" note below for why a true false-positive repro wasn't reachable) ### Found while verifying, not part of this fix `jdt_encapsulate_field` throws an unconditional `NullPointerException` in headless mode on **every** call, unrelated to #51/#61: `ProjectTemplateStore.load()` → `fInstanceStore` is `null` when JDT tries to render the generated getter/setter body from the code-template store (`SelfEncapsulateFieldRefactoring.checkFinalConditions()` → `addGetterSetterChanges()` → `createGetterMethod()`). Happens before the `status.hasError()`/`getRealErrors()` check this PR touches is ever reached, so a true end-to-end false-positive repro for #51 wasn't reachable — verified the logic is correct and consistent with the rest of the file instead. This predates this PR and needs its own issue (headless-mode template-store bootstrapping).

hauschel.fred added 1 commit

2026-09-07 20:15:32 +00:00

fix(refactoring): single createChange() call and consistent error filtering b97a19218f

Preview and execute paths in RefactoringTools each called
refactoring.createChange() separately for extractMethod, moveType, both
inline variants, changeMethodSignature, encapsulateField and
introduceParameter — duplicated code doing the same expensive change-tree
computation. Compute the Change once, branch on previewOnly afterwards.

encapsulateField and introduceParameter also gated on the raw
status.hasError(), unlike every other refactoring method in this file,
which filters headless-mode participant/potential-match noise through
RefactoringSupport.getRealErrors() first. Both now use the same filter,
so harmless headless warnings no longer get reported as blocking errors.

Closes #61

Closes #51

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