Skip to content

feat: enrich diagnostics for AI-agent autofix (Closes #74) - #88

Merged
aviadshiber merged 5 commits into
mainfrom
claude/java-functional-support-XBVAM
May 12, 2026
Merged

feat: enrich diagnostics for AI-agent autofix (Closes #74)#88
aviadshiber merged 5 commits into
mainfrom
claude/java-functional-support-XBVAM

Conversation

@aviadshiber

Copy link
Copy Markdown
Owner

Summary

Addresses all 5 suggestions in #74 to make java-functional-lsp diagnostics self-describing enough for an AI coding agent to autofix on the first try.

  • DiagnosticData gets two new optional fields: recommended_api (library-agnostic API hint, e.g. "forEach (NOT ifPresent — Vavr Option)") and suggested_snippet (concrete fix using real AST variable names). Serialised to the LSP wire as recommendedApi / suggestedSnippet. Existing payloads stay backward-compatible.
  • Per-rule snippet builders read variable names, types, and expressions from the AST so the snippet is paste-able. Example: if (myOpt.isDefined()) return myOpt.get(); else return "fallback"; now produces a snippet that references myOpt and "fallback" literally.
  • impure-method refined: distinguishes IO side-effects (Try) from throw side-effects (Either) with different messages, different target_library, and different recommended_api. Diagnostic range now points at the offending statement instead of the method declaration (issue AI agent UX: richer diagnostic data for automated fix generation #74 fix: release-drafter permissions and bump to v7 #5).
  • Quick-fix coverage extended (issue AI agent UX: richer diagnostic data for automated fix generation #74 deps: bump hatchling from 1.28.0 to 1.29.0 #3): imperative-option-unwrap, mutable-dto, field-injection now have registered fix generators. Each bails safely when the AST shape isn't trivially rewritable.

Files touched

Area Files
Data model + wire serialization analyzers/base.py, server.py
Per-rule data builders analyzers/{mutation,functional,exception,spring,null}_checker.py
New quick-fixes fixes.py
Tests tests/test_{fixes,mutation_checker,functional_checker,exception_checker,spring_checker,server}.py

Test plan

  • uv run pytest tests/ — 659 passed (17 new), 7 skipped (jdtls-only).
  • uv run ruff check src/ tests/ — clean.
  • uv run mypy src/ — clean.
  • Manual smoke test in a Java client: confirm recommendedApi / suggestedSnippet appear in Diagnostic.data JSON for at least imperative-option-unwrap and impure-method.
  • Manual quick-fix test: open if (opt.isDefined()) return opt.get(); else return "x"; — confirm the Convert-to-Option-map code action applies cleanly.
  • Verify impure-method on a throw-only method points at the throw line and produces the "exceptions" message variant.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq


Generated by Claude Code

claude added 2 commits May 11, 2026 08:25
Adds two optional fields to DiagnosticData — recommended_api and suggested_snippet —
and propagates them on the LSP wire as recommendedApi / suggestedSnippet. Per-rule
builders synthesise concrete fix snippets using real variable names from the AST,
so agents can paste fixes without re-deriving them.

- impure-method now distinguishes IO from throw side-effects (different target_library,
  message, and recommended_api) and points the diagnostic at the offending statement
  instead of the method declaration.
- Quick-fix coverage extended: imperative-option-unwrap, mutable-dto, field-injection.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq
CI runs `ruff format --check` which fails on local changes not formatted
to match. Apply the formatter on the touched files.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq
@aviadshiber
aviadshiber marked this pull request as ready for review May 11, 2026 08:34
@aviadshiber

Copy link
Copy Markdown
Owner Author

@claude review with ensemble (with majority vote)

Copy link
Copy Markdown
Owner Author

Ensemble review of #88 (majority vote, 3 independent reviewers)

Blockers (single reviewer, but type/correctness-fatal)

  1. fix_imperative_option_unwrap allows else-less ifsrc/java_functional_lsp/fixes.py:1056-1067. When the if has no else, the rewrite emits return opt.map(it -> it); which returns Option<T> instead of T, breaking the method's return type. Require alternative is not None or bail.
  2. fix_field_injection produces uncompilable codesrc/java_functional_lsp/fixes.py:1149-1185. The WorkspaceEdit removes @Autowired and adds final but never synthesises/updates a constructor, leaving an uninitialised final field. Per the docstring this is acknowledged, but the _FIX_TITLES entry promises "+ constructor injection". Either synthesise the constructor or remove the entry from _FIX_REGISTRY and rely solely on the suggested_snippet. (R2 blocker, R3 major: "rename or remove from registry".)

Major (≥2 reviewers agreed)

  1. Hardcoded class name "Foo" in _build_field_injection_data snippetsrc/java_functional_lsp/analyzers/spring_checker.py:50-62. Snippet says public Foo(final ... ) for every class. Walk to find_ancestor(field_decl, "class_declaration") and use its identifier. (R1, R2, R3.)
  2. Non-paste-able placeholders in suggested_snippet stringsmutation_checker.py:48 (@Value\npublic class Foo { ... }), spring_checker.py:31-36 (public Foo foo() { return new Foo(); }), mutation_checker.py imperative-option-unwrap ({ /* use value */ }). The { ... } / (...) placeholders won't compile if pasted verbatim. Either rebuild from AST or rename the field to make example-only semantics clear. (R2 major, R3 minor.)
  3. Degenerate null-check-to-monadic snippetsrc/java_functional_lsp/analyzers/functional_checker.py:71-79. Option.of(x).map(v -> v).getOrElse(null) is an identity map with an always-null default, re-introducing the very null we flagged. Either omit suggested_snippet for this rule or build a real lambda body from the if-then return expression text. (R1 major, R2 minor.)
  4. fix_mutable_dto is unsafe in two scenariossrc/java_functional_lsp/fixes.py:1078-1119. (a) Diagnostic message explicitly says "Use @ConstructorBinding instead of @Data/@Setter for @ConfigurationProperties classes" (mutation_checker.py:139-141), but the fix rewrites to @Value anyway — should bail when sibling @ConfigurationProperties is present. (b) Blanket rewrite of @Setter is unsafe — @Setter users typically have explicit setters/non-final fields; @Value makes the class final and strips setters. Restrict to @Data or also bail when explicit setter methods exist. (R1 + R2.)

Minor (≥2 reviewers agreed)

  1. autoImportVavr flag gates a Lombok importsrc/java_functional_lsp/fixes.py:1098. Users who disable Vavr auto-import (but still want Lombok auto-import) lose import lombok.Value;. Use a separate flag or always import for Lombok. (R1 + R2.)
  2. data: dict[str, str] | None annotation too narrowsrc/java_functional_lsp/server.py:305. Future non-string fields (numbers, bools, lists) will silently mis-type. Use dict[str, Any] | None to mirror the LSP Diagnostic.data shape. (R1 + R3.)

Minor (single-reviewer, worth flagging)

  1. Static snippets in null_checker.py ignore the AST (R2) — inconsistent with the PR's "real variable names from the AST" goal (R3 mentions the same inconsistency from the architecture side).
  2. Multi-declarator field handling_build_field_injection_data reads only the first variable_declarator, so private Bar a, b; snippets only a (R1).
  3. fix_mutable_dto doesn't verify grandparent.type == "class_declaration" before rewriting — if invoked with a wider diagnostic range, may rewrite an unrelated annotation (R1).
  4. _classify_side_effect returns str rather than Literal["throw", "io"] (R3) — typos in the discriminator would silently fall through to the IO branch.
  5. fix_field_injection annotation-removal trims only a single space — tab-separated @Autowired\tprivate ... leaves a stray tab (R1, R2).
  6. No e2e test for recommendedApi/suggestedSnippet on the wire — only unit-level _to_lsp_diagnostic checks exist (R2). CONTRIBUTING.md:48 also lists only the original three DiagnosticData fields (R2).

What did NOT come up as a concern

  • The serialization gate (if diag.data.recommended_api is not None) is correctly implemented — verified by test_to_lsp_diagnostic_omits_optional_fields_when_none.
  • The removed _is_side_effect_invocation/_is_side_effect_statement class methods have no external callers; fixes.py uses the module-level is_side_effect_invocation.
  • The _FIX_TITLES_FIX_REGISTRY import-time assertion still holds with 7 entries.

Suggested triage

I'd treat #1, #2, #4, #5 as ship-blockers (one produces broken Java, one is misleading enough to mislead the agents this PR aims to help) and address them in follow-up commits on this branch. #3, #6 are easy AST lookups. The rest is fair game for follow-up issues.

Let me know which subset to fix here vs split into follow-ups.


Generated by Claude Code

claude added 3 commits May 11, 2026 12:47
Blockers:
- fix_imperative_option_unwrap now bails when there's no else branch — the
  previous behavior produced `return opt.map(it -> it);` returning Option<T>
  instead of the method's declared T (type mismatch).
- field-injection is no longer in _FIX_REGISTRY: the previous WorkspaceEdit
  removed @Autowired and added `final` but never synthesised a constructor,
  leaving an uninitialised final field that wouldn't compile. The diagnostic
  + suggested_snippet remain available for AI agents to apply themselves.

Majors:
- Snippets for field-injection, mutable-dto, and component-annotation now use
  the real enclosing class name from the AST rather than the placeholder `Foo`.
- null-check-to-monadic snippet builds the lambda body from the real return
  expression (rewriting the checked variable as `it`) and pulls the else-fallback
  from either the nested else-branch or the fallthrough return — replacing the
  degenerate `Option.of(x).map(v -> v).getOrElse(null)`.
- fix_mutable_dto restricted to @DaTa only (@Setter rewrites silently broke
  callers by making the class final + stripping setters); also bails on
  @ConfigurationProperties (use @ConstructorBinding instead) and verifies the
  annotation is on a class_declaration.
- Placeholder strings (`{ ... }`, `(...)`) removed from static snippets.

Minors:
- autoImportLombok flag added (was incorrectly gated on autoImportVavr).
- _to_lsp_diagnostic data typed as dict[str, Any] | None for future non-string
  fields.
- field-injection snippet now bails for multi-declarator fields.
- _classify_side_effect kind typed as Literal["throw", "io"].
- null_checker assignment/field-assignment snippets pull real variable name and
  declared type from the AST.

Docs + tests:
- CONTRIBUTING.md "Adding a New Rule" updated with recommended_api and
  suggested_snippet guidance.
- New e2e test asserts recommendedApi + suggestedSnippet appear on the LSP wire.
- 13 regression tests covering each fix above.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq
…d variants, snippet quality)

Material items from the three individual reviewer reports:

1. frozen-mutation snippet now bails on chained LHS (e.g. `foo.getList()`,
   `this.items`) and explicitly spells out the io.vavr.collection.List type
   migration — pasting `items = items.append(...)` alone wouldn't compile when
   `items` is still a java.util.List.

2. impure-method io vs throw variants surface via distinct fix_type values
   (`EXTRACT_PURE_LOGIC_IO` / `EXTRACT_PURE_LOGIC_THROW`), so agents can filter
   on the variant without parsing target_library or message text. The wire-
   visible rule code stays `impure-method` to preserve user severity-overrides.

3. mutable-variable recommended_api drops `final +` (it's a language modifier,
   not an API). The rationale field still mentions final.

4. throw-statement snippet picks a single alternative (`return Either.left(e);`)
   instead of mixing two with a `//` comment — agents pasting verbatim would
   otherwise get the comment too.

5. KNOWN_RULES set added in analyzers/__init__.py; server import-time assertion
   catches typos in _FIX_TITLES that reference rules no analyzer emits.

6. _build_throw_statement_data filters named_children for the expression
   (tree-sitter-java doesn't field-name throw's expression).

7. All _build_*_data builders refactored to use dataclasses.replace instead of
   hand-copying every DiagnosticData field — adding a new field to
   DiagnosticData no longer requires touching every builder.

8. _VAVR_MUTATION_METHODS lifted to module scope (was re-allocated per
   diagnostic).

9. New `_apply_edits` test helper applies TextEdits to source and lets tests
   assert on resulting source — catches off-by-one range bugs.

10. _classify_side_effect docstring acknowledges DFS-first semantics for the
    rare case of a single statement containing both throw and IO.

11 regression tests covering each of the above. 679 tests pass, ruff format +
check + mypy strict all clean.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq
Minor bump for the AI-agent diagnostic enrichment feature (PR #88, Closes #74):
DiagnosticData adds optional recommendedApi + suggestedSnippet fields, three new
quick-fix generators, refined impure-method discrimination.

https://claude.ai/code/session_01Sp1TuWErAKym7CJ8n9pdrq
@aviadshiber
aviadshiber merged commit d321341 into main May 12, 2026
13 checks passed

Copy link
Copy Markdown
Owner Author

Suggested release notes for v0.11.0

The release-drafter created a draft labeled v0.10.1 (default patch bump) because no enhancement/feature/rule label was set on this PR. To publish as v0.11.0, edit the draft at https://github.com/aviadshiber/java-functional-lsp/releases (top of the list) — change the tag to v0.11.0, replace the body with the markdown below, and click Publish release. That fires publish.yml on release: published → PyPI.


Tag: v0.11.0 (targets main, commit d321341)

Title: v0.11.0 — AI-agent diagnostic enrichment (Closes #74)

Body:

## What's Changed

`DiagnosticData` now carries two optional fields — `recommendedApi` and `suggestedSnippet` — that flow through to the LSP wire so AI coding agents (e.g. Claude Code) can produce correct fixes on the first try without re-deriving Vavr/Lombok APIs or variable names.

### Highlights
- **AST-aware snippets**: per-rule builders pull real variable names, types, and return expressions from the AST so snippets are paste-able. Example for `if (myOpt.isDefined()) return myOpt.get(); else return "fallback";` produces `return myOpt.map(it -> it).getOrElse("fallback");`.
- **`impure-method` discriminates IO vs throw**: distinct messages, distinct `target_library` (Try vs Either), distinct `fix_type` (`EXTRACT_PURE_LOGIC_IO` / `_THROW`); diagnostic now points at the offending statement, not the method declaration.
- **New quick-fixes**: `imperative-option-unwrap`, `mutable-dto`. `field-injection` ships richer diagnostic + snippet but no quick-fix (constructor synthesis would risk clobbering existing user code).
- **`KNOWN_RULES` symmetry check** at server import time catches typos in `_FIX_TITLES` that reference non-existent rule codes.
- **New `autoImportLombok` config flag** (defaults true) so users can disable Vavr auto-import without losing Lombok auto-import (or vice versa).

### Test plan
- 90 new tests including end-to-end wire assertion that `recommendedApi`/`suggestedSnippet` reach the client.
- `ruff format/check` + `mypy --strict` clean.

### Commits
- feat: enrich diagnostics for AI-agent autofix (Closes #74) (#88)

**Full Changelog**: https://github.com/aviadshiber/java-functional-lsp/compare/v0.10.0...v0.11.0

For future PRs: adding the enhancement or feature label before merging will cause release-drafter to resolve a minor bump automatically (per .github/release-drafter.yml: version-resolver.minor).


Generated by Claude Code

aviadshiber added a commit that referenced this pull request May 12, 2026
PR #88 bumped pyproject.toml to 0.11.0 but left
src/java_functional_lsp/__init__.py at 0.10.0, so the installed
0.11.0 package reported "java-functional-lsp 0.10.0" via --version.

Bump both files to 0.11.1 so the published artifact reports the
correct version. No functional change.

Co-authored-by: Aviad Shiber <aviad.s@taboola.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
aviadshiber added a commit that referenced this pull request May 15, 2026
PR #88 (v0.11.0) bumped pyproject.toml but left
src/java_functional_lsp/__init__.py at 0.10.0, shipping a package
whose --version disagreed with its installed metadata. v0.11.1
synced them.

This test reads both values and asserts they match, so the next
bump that touches only one side fails CI before it ships.

Co-authored-by: Aviad Shiber <aviad.s@taboola.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
aviadshiber added a commit that referenced this pull request Jun 14, 2026
… hook (#70), AI-UX closure (#74) (#94)

* feat: add option-map-nullable rule for Vavr Some(null) hazard (#69)

Detect Vavr Option chains where .map() can wrap a null mapper result as
Some(null) — unlike java.util.Optional — and a chained .filter()/.map()/
.forEach()/etc. then NPEs on the value.

Conservative three-gate detection to avoid false positives:
1. a value-consuming follower is chained after .map(...)
2. the receiver chain is rooted in a literal Option.of()/Option.ofOptional()
3. the lambda body matches a known possibly-null shape (x.get(key) —
   Map.get / JsonNode.get; zero-arg get() and integer-index get(0) excluded)

The data payload carries a paste-able .flatMap(x -> Option.of(...)) snippet
built from the real lambda parameter and body text.

https://claude.ai/code/session_018SmBpTQU3hqtw8EbWMcZ5M

* feat: lint edited .java files via PostToolUse hook (#70)

Add hooks/post_tool_lint.py: runs java-functional-lsp on the edited file
after Edit/MultiEdit/Write and surfaces violations to Claude as
hookSpecificOutput.additionalContext, so Vavr-specific rules are checked
in the editing flow instead of only via manual /lint-java runs.

- only fires on .java files; lints just the changed file (in-process
  import with CLI fallback when the package isn't importable under the
  hook's python3)
- silent on clean files; diagnostics capped at 25 to bound context
- failure-safe: every path exits 0, SIGALRM + subprocess timeout cap
  runtime, so a linter problem can never break the editing session

The existing reminder hook is narrowed to Read — the new hook embeds its
own fix instruction for edits, so keeping the reminder on Edit|Write
would duplicate context.

https://claude.ai/code/session_018SmBpTQU3hqtw8EbWMcZ5M

* fix: derive real lambda body in unwrap snippet; sync docs (#74)

Complete issue #74 (PR #88 shipped the core — variant-aware impure-method,
recommendedApi/suggestedSnippet payloads, and two new quick fixes):

- imperative-option-unwrap's suggested_snippet now derives the lambda body
  from the real then-branch expression (return opt.get().toUpperCase()
  becomes .map(value -> value.toUpperCase())) instead of the placeholder
  value -> value, mirroring the quick-fix rewrite in fixes.py
- README/SKILL.md: flip Quick Fix column for mutable-dto and
  imperative-option-unwrap, document the two code actions, the
  recommendedApi/suggestedSnippet data fields, and autoImportLombok
- README/SKILL.md/editor READMEs: rule count 16 -> 17 for the new
  option-map-nullable rule (vscode README still said 12)
- uv.lock: refresh stale package version (0.10.0 -> 0.11.1)

https://claude.ai/code/session_018SmBpTQU3hqtw8EbWMcZ5M

* fix: exclude octal/binary index literals from option-map-nullable; reuse IGNORED_CHILDREN

Ensemble review findings on PR #94:

- xs.get(010) / xs.get(0b101) falsely fired option-map-nullable because
  tree-sitter emits octal_integer_literal / binary_integer_literal node
  types that were missing from _INTEGER_LITERAL_TYPES; index access throws
  rather than returning null regardless of radix. Verified empirically.
- mutation_checker's new helpers re-declared the comment-filter tuple
  locally; use the shared IGNORED_CHILDREN constant from base instead.

https://claude.ai/code/session_018SmBpTQU3hqtw8EbWMcZ5M

* refactor: consolidate return-extraction and var.get() rewrite into base.py

Majority-agreement finding from the ensemble review (Reuse + Altitude
agents independently flagged it): three copies of single-return-statement
extraction lived in functional_checker, mutation_checker, and fixes.py,
and the var.get()->param regex rewrite was duplicated between
mutation_checker and fixes.py. All now live in base.py (the leaf module,
so no import cycle): single_return_stmt, return_expr_node,
return_expr_text, single_return_expr_text, rewrite_var_get_call,
rewrite_var_references.

The shared rewrite also fixes two confirmed defects in the old copies:
- regex \b never matches before $-prefixed Java identifiers (and falsely
  matches inside them), silently dropping the rewrite
- no tolerance for whitespace around '.'/parens (opt .get () is valid Java)

Gate order in option-map-nullable kept as-is: benchmarked the suggested
lambda-shape-first reorder and the chain walk is ~3x cheaper per .map()
on fluent-heavy code (4.4ms vs 12.2ms per 4000-call pass), so the claim
was refuted by measurement; documented in the gate comment.

https://claude.ai/code/session_018SmBpTQU3hqtw8EbWMcZ5M

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants