Skip to content

fix: scope field extraction to target class to prevent cross-class injection#1953

Open
mashraf-222 wants to merge 1 commit intomainfrom
cf-1087-field-injection-class-filter
Open

fix: scope field extraction to target class to prevent cross-class injection#1953
mashraf-222 wants to merge 1 commit intomainfrom
cf-1087-field-injection-class-filter

Conversation

@mashraf-222
Copy link
Copy Markdown
Contributor

Problem

When the LLM generates optimization code containing inner/anonymous classes with fields, those fields are incorrectly injected into the target method's enclosing class. This produces uncompilable code — e.g., type parameters like Iterator<? extends F> injected where F is not in scope.

Observed in Guava PR #12 (Iterables.mergeSorted) where inner class fields were injected into the outer Iterables class.

Root Cause

_parse_optimization_source() in replacement.py (line 72) called analyzer.find_fields(new_source) without a class_name filter, extracting ALL fields from ALL classes in the generated code — including inner classes, static nested classes, and anonymous classes.

Fix

Pass class_name=target_method.class_name to find_fields() so only fields belonging to the target method's class are extracted. The find_fields() API already supported this filter via its _walk_tree_for_fields() implementation — it just wasn't being used.

The field extraction was also moved after the target method lookup (previously it ran before we knew which class the target was in).

Validation

  • Bug reproduced: test creates an inner class with badField → it was injected into outer class
  • Fix confirmed: badField no longer injected, only OFFSET (same class as target) is added
  • All 33 replacement tests pass (32 existing + 1 new)

Test Coverage

  • tests/test_languages/test_java/test_replacement.pyTestFieldInjectionClassFiltering::test_inner_class_fields_not_injected_into_outer

Closes CF-1087

…jection

find_fields() was called without a class_name filter, causing fields from
inner/anonymous classes to be injected into the outer target class. Now
scoped to target_method.class_name using the existing filter parameter.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 1, 2026

Claude finished @mashraf-222's task in 2m 2s —— View job


PR Review Summary

  • Triage PR size — SMALL (5 lines production code changed)
  • Lint and type check — prek: passed, mypy: no issues
  • Resolve stale threads — none found
  • Code review
  • Duplicate detection

Prek Checks

All checks passed. No issues.

Code Review

The fix is correct and well-targeted.

What changed: _parse_optimization_source() now calls analyzer.find_fields(new_source, class_name=target_class_name) instead of analyzer.find_fields(new_source). The class_name parameter already existed in parser.py:585 — it just wasn't being used here.

One minor edge case to note: When target_method is None (line 133), target_class_name falls back to None, so find_fields still extracts from all classes. This is harmless in practice — if target_method is None, target_method_source is already "" (line 92) and the candidate will be skipped. But it's worth noting if this code path is ever changed.

Slight redundancy: target_method.class_name is already computed on line 122 as target_class_name_for_ctors (inside if target_method:). The new variable on line 133 is computed outside that block to handle the None case, which is correct. Not a bug, just slightly awkward naming given target_class_name_for_ctors exists immediately above.

Related unfiltered call at line 472: analyzer.find_fields(source) (original source, not generated code) is unfiltered — it collects fields from all classes for deduplication purposes. This is pre-existing and out of scope for this PR, but could theoretically cause false deduplication if inner/outer class field names collide.

Duplicate Detection

No duplicates detected. The fix is Java-specific and uses the existing class_name filter parameter in JavaAnalyzer.find_fields().


Overall this is a clean, minimal fix that addresses the root cause. The test accurately reproduces the Guava Iterables.mergeSorted bug pattern. LGTM.

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.

1 participant