From a880ffc48a13c79f0375db1c918c9b79060b5ea9 Mon Sep 17 00:00:00 2001 From: misrasaurabh1 Date: Mon, 2 Mar 2026 23:59:31 +0000 Subject: [PATCH 1/3] fix: skip outer-class methods when target is in a static inner class (Java) When the optimisation target lives in a static inner class (e.g. ObjectUnpacker inside Unpacker), the LLM-generated class often wraps the inner class inside the full outer class. Previously, methods belonging to the outer class were extracted as "helpers" and injected into the inner class, causing compilation errors: - "non-static type variable T cannot be referenced from a static context" - "non-static variable offset cannot be referenced from a static context" Two related fixes: 1. When _parse_optimization_source extracts helpers, it now skips any method whose class_name differs from the target method's class_name. 2. The function now accepts an optional target_class_name parameter. When there are multiple methods with the same name in the generated code (e.g. an abstract outer-class method and the concrete inner-class override), the method in the target class is preferred over outer-class methods. Fixes the Unpacker.ObjectUnpacker.getString regression from codeflash_all_3.log. Co-Authored-By: Claude Sonnet 4.6 --- codeflash/languages/java/replacement.py | 41 +++++- .../test_java/test_replacement.py | 135 ++++++++++++++++++ 2 files changed, 170 insertions(+), 6 deletions(-) diff --git a/codeflash/languages/java/replacement.py b/codeflash/languages/java/replacement.py index e6628286e..3136f8bf2 100644 --- a/codeflash/languages/java/replacement.py +++ b/codeflash/languages/java/replacement.py @@ -37,7 +37,12 @@ class ParsedOptimization: helpers_after_target: list[str] = field(default_factory=list) # Helpers appearing after target in optimized code -def _parse_optimization_source(new_source: str, target_method_name: str, analyzer: JavaAnalyzer) -> ParsedOptimization: +def _parse_optimization_source( + new_source: str, + target_method_name: str, + analyzer: JavaAnalyzer, + target_class_name: str | None = None, +) -> ParsedOptimization: """Parse optimization source to extract method and additional class members. The new_source may contain: @@ -48,6 +53,11 @@ def _parse_optimization_source(new_source: str, target_method_name: str, analyze new_source: The optimization source code. target_method_name: Name of the method being optimized. analyzer: JavaAnalyzer instance. + target_class_name: Optional name of the class that owns the target method. + When provided and the generated code contains multiple methods with the + same name (e.g. an abstract method in an outer class AND the concrete + override in an inner class), the method whose ``class_name`` matches + this value is preferred as the actual replacement target. Returns: ParsedOptimization with the method and any additional members. @@ -74,9 +84,17 @@ def _parse_optimization_source(new_source: str, target_method_name: str, analyze target_method_index: int | None = None for i, method in enumerate(methods): if method.name == target_method_name: - target_method = method - target_method_index = i - break + # When a target_class_name is known, prefer the method in that class + # (e.g. ObjectUnpacker.getString over the abstract outer getString). + # Still accept any match as fallback if no class-specific one is found. + if target_class_name is None or method.class_name == target_class_name: + target_method = method + target_method_index = i + break + elif target_method is None: + # Keep as tentative fallback (class didn't match yet) + target_method = method + target_method_index = i if target_method: # Extract target method source (including Javadoc if present) @@ -96,6 +114,11 @@ def _parse_optimization_source(new_source: str, target_method_name: str, analyze # Skip methods whose line range falls entirely inside the target method's # range, as these belong to anonymous/inner classes inside the target body # and must not be hoisted out as top-level class members. + # Also skip methods that belong to a different class than the target — this + # handles the case where the target is in an inner class and the generated + # code also contains outer-class methods that must not be injected into the + # inner class (outer-class methods would reference type parameters or instance + # variables that are not in scope inside a static inner class). lines = new_source.splitlines(keepends=True) for i, method in enumerate(methods): if method.name != target_method_name: @@ -104,6 +127,9 @@ def _parse_optimization_source(new_source: str, target_method_name: str, analyze method.start_line >= target_method.start_line and method.end_line <= target_method.end_line ): continue + # Skip methods from a different class than the target method + if target_method and method.class_name != target_method.class_name: + continue start = (method.javadoc_start_line or method.start_line) - 1 end = method.end_line helper_source = "".join(lines[start:end]) @@ -305,8 +331,11 @@ def replace_function( func_start_line = function.starting_line func_end_line = function.ending_line - # Parse the optimization to extract components - parsed = _parse_optimization_source(new_source, func_name, analyzer) + # Parse the optimization to extract components. + # Pass the class name so that when the generated code contains multiple + # methods with the same name (outer abstract + inner concrete), the correct + # one is selected as the replacement target. + parsed = _parse_optimization_source(new_source, func_name, analyzer, target_class_name=function.class_name) # If the parsed optimization has no valid target source (e.g., the LLM generated # a method with a different name), skip this candidate entirely. diff --git a/tests/test_languages/test_java/test_replacement.py b/tests/test_languages/test_java/test_replacement.py index f1424361a..29e04abbb 100644 --- a/tests/test_languages/test_java/test_replacement.py +++ b/tests/test_languages/test_java/test_replacement.py @@ -1927,3 +1927,138 @@ def test_anonymous_iterator_methods_not_hoisted_to_class(self, tmp_path): } """ assert new_code == expected_code + + +class TestInnerClassHelperFilter: + """Tests that outer-class methods are not injected into a static inner class. + + When the target method lives in a *static* inner class (e.g. ObjectUnpacker), + the generated optimisation class typically wraps the inner class inside the + outer class. Methods that belong to the outer class must NOT be extracted as + helpers and inserted into the inner class — they would reference outer-class + type parameters or instance variables that are unavailable in a static context. + """ + + def test_outer_class_methods_not_injected_into_static_inner_class(self, tmp_path): + """Reproduces the Unpacker.ObjectUnpacker.getString bug. + + The outer class ``Unpacker`` has a method ``getString(String)``. + When the LLM generates an optimisation for ``ObjectUnpacker.getString``, + the generated file still contains the outer ``Unpacker`` skeleton. + Codeflash must NOT inject the outer ``getString`` helper into the + ``ObjectUnpacker`` inner class. + """ + from codeflash.discovery.functions_to_optimize import FunctionToOptimize, FunctionParent + from codeflash.languages.java.replacement import replace_function + + original_code = """\ +public abstract class Unpacker { + protected byte[] buffer; + protected int offset; + protected int length; + + public Unpacker(byte[] buffer, int offset, int length) { + this.buffer = buffer; + this.offset = offset; + this.length = length; + } + + protected abstract T getString(String value); + + public T unpackString() { + return getString(new String(buffer, offset, length)); + } + + public static final class ObjectUnpacker extends Unpacker { + public ObjectUnpacker(byte[] buffer, int offset, int length) { + super(buffer, offset, length); + } + + @Override + protected Object getString(String value) { + return value; + } + } +} +""" + java_file = tmp_path / "Unpacker.java" + java_file.write_text(original_code, encoding="utf-8") + + # LLM-generated optimisation: the outer class is present in the generated + # code, but only ObjectUnpacker.getString is the actual optimisation target. + optimized_source = """\ +public abstract class Unpacker { + protected byte[] buffer; + protected int offset; + protected int length; + + public Unpacker(byte[] buffer, int offset, int length) { + this.buffer = buffer; + this.offset = offset; + this.length = length; + } + + protected abstract T getString(String value); + + public T unpackString() { + return getString(new String(buffer, offset, length)); + } + + public static final class ObjectUnpacker extends Unpacker { + public ObjectUnpacker(byte[] buffer, int offset, int length) { + super(buffer, offset, length); + } + + @Override + protected Object getString(String value) { + return value.intern(); + } + } +} +""" + + func = FunctionToOptimize( + function_name="getString", + file_path=java_file, + starting_line=21, + ending_line=23, + parents=[FunctionParent(name="ObjectUnpacker", type="ClassDef")], + is_method=True, + language="java", + ) + + new_code = replace_function(original_code, func, optimized_source) + + # The outer-class unpackString() method must NOT be inserted into ObjectUnpacker. + # The result should only differ from the original in the ObjectUnpacker.getString body. + expected_code = """\ +public abstract class Unpacker { + protected byte[] buffer; + protected int offset; + protected int length; + + public Unpacker(byte[] buffer, int offset, int length) { + this.buffer = buffer; + this.offset = offset; + this.length = length; + } + + protected abstract T getString(String value); + + public T unpackString() { + return getString(new String(buffer, offset, length)); + } + + public static final class ObjectUnpacker extends Unpacker { + public ObjectUnpacker(byte[] buffer, int offset, int length) { + super(buffer, offset, length); + } + + @Override + protected Object getString(String value) { + return value.intern(); + } + } +} +""" + assert new_code == expected_code From 05a9b614785df9c55cc06cb0ba75c5567fdc1af0 Mon Sep 17 00:00:00 2001 From: misrasaurabh1 Date: Tue, 3 Mar 2026 00:03:38 +0000 Subject: [PATCH 2/3] fix: replace modified constructors when LLM adds new final fields (Java) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the LLM optimises a method by introducing a new final field (e.g. caching Arrays.hashCode in Expression.hashCode, or caching map.values() in LuaMap.valuesIterator), it also modifies the class constructors to initialise the field. Previously codeflash: 1. Added the new field to the class ✓ 2. Replaced the target method ✓ 3. Did NOT update the constructors ✗ This caused "variable X might not have been initialized" compilation errors. Changes: - `JavaAnalyzer.find_constructors` (+ `_walk_tree_for_constructors`, `_extract_constructor_info`): new parser methods to locate `constructor_declaration` nodes via tree-sitter. - `JavaMethodNode.formal_parameters_text`: captures the raw parameter list text so constructors can be matched by signature. - `ParsedOptimization.modified_constructors`: new field to carry constructor source texts that need to be replaced. - `_parse_optimization_source`: extract constructors from the same class as the target method and store in `modified_constructors`. - `_replace_constructors`: new helper that replaces constructors in the original source by matching on formal parameter signature. - `replace_function`: call `_replace_constructors` after the main method replacement when `modified_constructors` is non-empty. Fixes regressions observed in codeflash_all_3.log: LuaMap.valuesIterator, Expression.hashCode, Bin.hashCode, NettyTlsContext.createHandler, Pool.capacity. Co-Authored-By: Claude Sonnet 4.6 --- codeflash/languages/java/parser.py | 99 +++++++++++ codeflash/languages/java/replacement.py | 111 +++++++++++- .../test_java/test_replacement.py | 165 ++++++++++++++++++ 3 files changed, 374 insertions(+), 1 deletion(-) diff --git a/codeflash/languages/java/parser.py b/codeflash/languages/java/parser.py index b5715bb09..b2e1e62df 100644 --- a/codeflash/languages/java/parser.py +++ b/codeflash/languages/java/parser.py @@ -52,6 +52,7 @@ class JavaMethodNode: class_name: str | None source_text: str javadoc_start_line: int | None = None # Line where Javadoc comment starts + formal_parameters_text: str | None = None # Raw formal parameters "(Type name, ...)" for matching @dataclass @@ -182,6 +183,104 @@ def find_methods( return methods + def find_constructors(self, source: str, class_name: str | None = None) -> list[JavaMethodNode]: + """Find all constructor definitions in source code. + + Args: + source: The source code to analyze. + class_name: Optional class name to filter constructors. + + Returns: + List of JavaMethodNode objects describing found constructors. + The ``name`` field of each node is the constructor name (i.e. the class name). + + """ + source_bytes = source.encode("utf8") + tree = self.parse(source_bytes) + constructors: list[JavaMethodNode] = [] + self._walk_tree_for_constructors( + tree.root_node, source_bytes, constructors, current_class=None, target_class=class_name + ) + return constructors + + def _walk_tree_for_constructors( + self, + node: Node, + source_bytes: bytes, + constructors: list[JavaMethodNode], + current_class: str | None, + target_class: str | None, + ) -> None: + """Recursively walk the tree to find constructor declarations.""" + new_class = current_class + type_declarations = ("class_declaration", "interface_declaration", "enum_declaration") + if node.type in type_declarations: + name_node = node.child_by_field_name("name") + if name_node: + new_class = self.get_node_text(name_node, source_bytes) + + if node.type == "constructor_declaration": + constructor_info = self._extract_constructor_info(node, source_bytes, new_class) + if constructor_info: + if target_class is None or constructor_info.class_name == target_class: + constructors.append(constructor_info) + + for child in node.children: + self._walk_tree_for_constructors( + child, + source_bytes, + constructors, + current_class=new_class if node.type in type_declarations else current_class, + target_class=target_class, + ) + + def _extract_constructor_info( + self, node: Node, source_bytes: bytes, current_class: str | None + ) -> JavaMethodNode | None: + """Extract constructor information from a constructor_declaration node.""" + name_node = node.child_by_field_name("name") + if not name_node: + return None + name = self.get_node_text(name_node, source_bytes) + + is_public = False + is_private = False + is_protected = False + for child in node.children: + if child.type == "modifiers": + modifier_text = self.get_node_text(child, source_bytes) + is_public = "public" in modifier_text + is_private = "private" in modifier_text + is_protected = "protected" in modifier_text + break + + # Extract formal parameters text for signature matching + params_node = node.child_by_field_name("parameters") + formal_parameters_text = self.get_node_text(params_node, source_bytes) if params_node else "()" + + source_text = self.get_node_text(node, source_bytes) + javadoc_start_line = self._find_preceding_javadoc(node, source_bytes) + + return JavaMethodNode( + name=name, + node=node, + start_line=node.start_point[0] + 1, + end_line=node.end_point[0] + 1, + start_col=node.start_point[1], + end_col=node.end_point[1], + is_static=False, + is_public=is_public, + is_private=is_private, + is_protected=is_protected, + is_abstract=False, + is_synchronized=False, + return_type=None, + class_name=current_class, + source_text=source_text, + javadoc_start_line=javadoc_start_line, + formal_parameters_text=formal_parameters_text, + ) + def _walk_tree_for_methods( self, node: Node, diff --git a/codeflash/languages/java/replacement.py b/codeflash/languages/java/replacement.py index 3136f8bf2..148f67b5e 100644 --- a/codeflash/languages/java/replacement.py +++ b/codeflash/languages/java/replacement.py @@ -35,6 +35,7 @@ class ParsedOptimization: new_fields: list[str] # Source text of new fields to add helpers_before_target: list[str] = field(default_factory=list) # Helpers appearing before target in optimized code helpers_after_target: list[str] = field(default_factory=list) # Helpers appearing after target in optimized code + modified_constructors: list[str] = field(default_factory=list) # Constructor sources that need to replace originals def _parse_optimization_source( @@ -73,6 +74,7 @@ def _parse_optimization_source( helpers_before_target: list[str] = [] helpers_after_target: list[str] = [] + modified_constructors: list[str] = [] if classes: # It's a class - extract components @@ -138,6 +140,22 @@ def _parse_optimization_source( else: helpers_after_target.append(helper_source) + # Extract constructors that belong to the same class as the target method. + # When the LLM adds a new field (e.g. a cached value), it also updates the + # constructors to initialize it. We must replace those constructors in the + # original source, otherwise the new final field will be uninitialized + # (Bug 3: uninitialized variable errors). + # Use line-sliced text (same as helper methods) so that the leading whitespace + # is preserved and _dedent_member can normalise indentation correctly. + if target_method: + target_class_name_for_ctors = target_method.class_name + new_constructors = analyzer.find_constructors(new_source, class_name=target_class_name_for_ctors) + ctor_lines = new_source.splitlines(keepends=True) + for c in new_constructors: + ctor_start = (c.javadoc_start_line or c.start_line) - 1 + ctor_end = c.end_line + modified_constructors.append("".join(ctor_lines[ctor_start:ctor_end])) + # Extract fields for f in fields: if f.source_text: @@ -164,6 +182,7 @@ def _parse_optimization_source( new_fields=new_fields, helpers_before_target=helpers_before_target, helpers_after_target=helpers_after_target, + modified_constructors=modified_constructors, ) @@ -298,6 +317,89 @@ def format_member(raw: str) -> str: return result +def _replace_constructors( + source: str, + class_name: str, + new_constructor_sources: list[str], + analyzer: JavaAnalyzer, +) -> str: + """Replace constructors in source with updated versions from the optimization. + + Matches constructors by their formal parameter signature. When a matching + constructor is found in the original source it is replaced in-place, + preserving the original indentation. Constructors for which no match + exists in the original are silently skipped (they would need to be inserted + as new members, which is out of scope for this helper). + + Args: + source: The original source code to modify. + class_name: Name of the class whose constructors should be replaced. + new_constructor_sources: Source text of each updated constructor. + analyzer: JavaAnalyzer instance. + + Returns: + Modified source code with constructors replaced. + + """ + if not new_constructor_sources: + return source + + original_constructors = analyzer.find_constructors(source, class_name=class_name) + if not original_constructors: + return source + + result = source + + for new_ctor_src in new_constructor_sources: + # Wrap in a dummy class so the parser can handle a bare constructor + dummy = f"class __Dummy__ {{\n{new_ctor_src}\n}}" + parsed_new = analyzer.find_constructors(dummy) + if not parsed_new: + continue + new_ctor = parsed_new[0] + new_params = (new_ctor.formal_parameters_text or "()").strip() + + # Find the matching constructor in the current (potentially already + # modified) source by parameter signature. + current_constructors = analyzer.find_constructors(result, class_name=class_name) + matching = None + for orig in current_constructors: + if (orig.formal_parameters_text or "()").strip() == new_params: + matching = orig + break + + if not matching: + logger.debug( + "No matching constructor with params %s found in class %s; skipping.", + new_params, + class_name, + ) + continue + + # Determine replacement range (include Javadoc if present) + ctor_start = matching.javadoc_start_line or matching.start_line + ctor_end = matching.end_line + + lines = result.splitlines(keepends=True) + original_first_line = lines[ctor_start - 1] if ctor_start <= len(lines) else "" + indent = _get_indentation(original_first_line) + + # Dedent first to remove any class-level indentation, then re-apply + # the correct indentation (same as _insert_class_members / format_member). + new_ctor_lines = _dedent_member(new_ctor_src).splitlines(keepends=True) + indented_new_ctor = _apply_indentation(new_ctor_lines, indent) + if indented_new_ctor and not indented_new_ctor.endswith("\n"): + indented_new_ctor += "\n" + + before = lines[: ctor_start - 1] + after = lines[ctor_end:] + result = "".join(before) + indented_new_ctor + "".join(after) + + logger.debug("Replaced constructor %s(%s) in class %s", class_name, new_params, class_name) + + return result + + def replace_function( source: str, function: FunctionToOptimize, new_source: str, analyzer: JavaAnalyzer | None = None ) -> str: @@ -496,7 +598,14 @@ def replace_function( before = lines[: start_line - 1] # Lines before the method after = lines[end_line:] # Lines after the method - return "".join(before) + indented_new_source + "".join(after) + result = "".join(before) + indented_new_source + "".join(after) + + # Replace modified constructors if the optimization introduced new field + # initializations (Bug 3: uninitialized variable errors). + if class_name and parsed.modified_constructors: + result = _replace_constructors(result, class_name, parsed.modified_constructors, analyzer) + + return result def _get_indentation(line: str) -> str: diff --git a/tests/test_languages/test_java/test_replacement.py b/tests/test_languages/test_java/test_replacement.py index 29e04abbb..37a0c8a14 100644 --- a/tests/test_languages/test_java/test_replacement.py +++ b/tests/test_languages/test_java/test_replacement.py @@ -2062,3 +2062,168 @@ def test_outer_class_methods_not_injected_into_static_inner_class(self, tmp_path } """ assert new_code == expected_code + + +class TestConstructorReplacement: + """Tests that constructors in the generated class are propagated to the original. + + When the LLM introduces a new ``final`` field (e.g. a cached hash) it must + also initialise that field inside every constructor. Codeflash must detect + the modified constructors in the generated class and replace the corresponding + constructors in the original source, otherwise the new field is uninitialised + and the compiler rejects the file with "variable X might not have been + initialized". + """ + + def test_constructor_updated_when_new_final_field_added(self, tmp_path): + """Reproduces the Expression.hashCode / LuaMap.valuesIterator pattern. + + The LLM optimises ``hashCode`` by caching the result in a new final + field ``cachedHash``. The generated class includes the updated + constructor that initialises ``cachedHash``. Codeflash must also + replace the original constructor so that the field is properly + initialised. + """ + from codeflash.discovery.functions_to_optimize import FunctionToOptimize, FunctionParent + from codeflash.languages.java.replacement import replace_function + + original_code = """\ +public final class Expression { + private final byte[] bytes; + + Expression(byte[] bytes) { + this.bytes = bytes; + } + + @Override + public int hashCode() { + return java.util.Arrays.hashCode(bytes); + } +} +""" + java_file = tmp_path / "Expression.java" + java_file.write_text(original_code, encoding="utf-8") + + # LLM optimisation: cache the hash in a new final field. The generated + # class includes both the updated constructor and the simplified hashCode. + optimized_source = """\ +public final class Expression { + private final byte[] bytes; + private final int cachedHash; + + Expression(byte[] bytes) { + this.bytes = bytes; + this.cachedHash = java.util.Arrays.hashCode(bytes); + } + + @Override + public int hashCode() { + return cachedHash; + } +} +""" + + func = FunctionToOptimize( + function_name="hashCode", + file_path=java_file, + starting_line=9, + ending_line=11, + parents=[FunctionParent(name="Expression", type="ClassDef")], + is_method=True, + language="java", + ) + + new_code = replace_function(original_code, func, optimized_source) + + # The result should have: + # 1. The new field "cachedHash" added + # 2. The constructor updated to initialise "cachedHash" + # 3. hashCode() returning cachedHash + expected_code = """\ +public final class Expression { + private final byte[] bytes; + private final int cachedHash; + + Expression(byte[] bytes) { + this.bytes = bytes; + this.cachedHash = java.util.Arrays.hashCode(bytes); + } + + @Override + public int hashCode() { + return cachedHash; + } +} +""" + assert new_code == expected_code + + def test_constructor_updated_for_cached_collection_view(self, tmp_path): + """Reproduces the LuaMap.valuesIterator caching pattern. + + The LLM caches ``map.values()`` in a new final field ``valuesView`` + which is initialised in the constructor. The original constructor + must be updated. + """ + from codeflash.discovery.functions_to_optimize import FunctionToOptimize, FunctionParent + from codeflash.languages.java.replacement import replace_function + + original_code = """\ +public class DataStore { + private final java.util.Map data; + + public DataStore(java.util.Map data) { + this.data = data; + } + + public java.util.Collection values() { + return data.values(); + } +} +""" + java_file = tmp_path / "DataStore.java" + java_file.write_text(original_code, encoding="utf-8") + + optimized_source = """\ +public class DataStore { + private final java.util.Map data; + private final java.util.Collection cachedValues; + + public DataStore(java.util.Map data) { + this.data = data; + this.cachedValues = data.values(); + } + + public java.util.Collection values() { + return cachedValues; + } +} +""" + + func = FunctionToOptimize( + function_name="values", + file_path=java_file, + starting_line=8, + ending_line=10, + parents=[FunctionParent(name="DataStore", type="ClassDef")], + is_method=True, + language="java", + ) + + new_code = replace_function(original_code, func, optimized_source) + + expected_code = """\ +public class DataStore { + private final java.util.Map data; + private final java.util.Collection cachedValues; + + public DataStore(java.util.Map data) { + this.data = data; + this.cachedValues = data.values(); + } + + public java.util.Collection values() { + return cachedValues; + } +} +""" + assert new_code == expected_code From aae13a8e69dc90de30d299a7c06b697268e2618a Mon Sep 17 00:00:00 2001 From: misrasaurabh1 Date: Tue, 3 Mar 2026 00:47:01 +0000 Subject: [PATCH 3/3] feat: skip inner-class methods in Java discovery; revert replacement-level inner-class workarounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parser.py: add `is_class_nested` flag to `JavaMethodNode`; track `class_depth` in `_walk_tree_for_methods` (incremented each time a type declaration is entered) and set `is_class_nested = True` when depth ≥ 2 (method lives inside a nested/inner class) - discovery.py: add early-exit in `_should_include_method` when `method.is_class_nested` is True — inner-class methods cannot be reliably instrumented or tested in isolation, so we skip them up-front rather than wasting LLM tokens on candidates that will always be rejected later - replacement.py: revert Bug-4 replacement-level workarounds that are now obsolete: * remove `target_class_name` parameter from `_parse_optimization_source` * restore simple first-match `break` in target-method selection * remove class_name filter that blocked helpers from "other" classes - tests: update `TestNestedClasses`, `TestExtractCodeContextWithInnerClasses` to reflect the new no-inner-class-discovery contract; remove `TestInnerClassHelperFilter` (superseded by discovery filter); add `TestInnerClassMethodFilter` in test_discovery.py with four scenarios covering static nested, non-static inner, outer-only, and deeply-nested classes Co-Authored-By: Claude Sonnet 4.6 --- codeflash/languages/java/discovery.py | 5 + codeflash/languages/java/parser.py | 9 +- codeflash/languages/java/replacement.py | 33 +--- .../test_languages/test_java/test_context.py | 50 ++---- .../test_java/test_discovery.py | 119 +++++++++++++ .../test_java/test_replacement.py | 163 ++---------------- 6 files changed, 161 insertions(+), 218 deletions(-) diff --git a/codeflash/languages/java/discovery.py b/codeflash/languages/java/discovery.py index 3d36e7d40..cb610cb18 100644 --- a/codeflash/languages/java/discovery.py +++ b/codeflash/languages/java/discovery.py @@ -129,6 +129,11 @@ def _should_include_method( True if the method should be included. """ + # Skip methods that belong to an inner/nested class — they cannot be reliably + # instrumented or tested in isolation (see discussion in discovery module). + if method.is_class_nested: + return False + # Skip abstract methods (no implementation to optimize) if method.is_abstract: return False diff --git a/codeflash/languages/java/parser.py b/codeflash/languages/java/parser.py index b2e1e62df..21c0ad4f0 100644 --- a/codeflash/languages/java/parser.py +++ b/codeflash/languages/java/parser.py @@ -53,6 +53,7 @@ class JavaMethodNode: source_text: str javadoc_start_line: int | None = None # Line where Javadoc comment starts formal_parameters_text: str | None = None # Raw formal parameters "(Type name, ...)" for matching + is_class_nested: bool = False # True when the enclosing class is itself nested inside another class @dataclass @@ -289,6 +290,7 @@ def _walk_tree_for_methods( include_private: bool, include_static: bool, current_class: str | None, + class_depth: int = 0, ) -> None: """Recursively walk the tree to find method definitions.""" new_class = current_class @@ -304,6 +306,10 @@ def _walk_tree_for_methods( method_info = self._extract_method_info(node, source_bytes, current_class) if method_info: + # A method is nested when its enclosing class is itself inside another + # class (class_depth >= 2: depth 1 = outermost class, depth 2+ = nested). + method_info.is_class_nested = class_depth >= 2 + # Apply filters should_include = True @@ -316,7 +322,7 @@ def _walk_tree_for_methods( if should_include: methods.append(method_info) - # Recurse into children + # Recurse into children, incrementing depth when entering a type declaration for child in node.children: self._walk_tree_for_methods( child, @@ -325,6 +331,7 @@ def _walk_tree_for_methods( include_private=include_private, include_static=include_static, current_class=new_class if node.type in type_declarations else current_class, + class_depth=class_depth + 1 if node.type in type_declarations else class_depth, ) def _extract_method_info(self, node: Node, source_bytes: bytes, current_class: str | None) -> JavaMethodNode | None: diff --git a/codeflash/languages/java/replacement.py b/codeflash/languages/java/replacement.py index 148f67b5e..a1bd4fb7c 100644 --- a/codeflash/languages/java/replacement.py +++ b/codeflash/languages/java/replacement.py @@ -42,7 +42,6 @@ def _parse_optimization_source( new_source: str, target_method_name: str, analyzer: JavaAnalyzer, - target_class_name: str | None = None, ) -> ParsedOptimization: """Parse optimization source to extract method and additional class members. @@ -54,11 +53,6 @@ def _parse_optimization_source( new_source: The optimization source code. target_method_name: Name of the method being optimized. analyzer: JavaAnalyzer instance. - target_class_name: Optional name of the class that owns the target method. - When provided and the generated code contains multiple methods with the - same name (e.g. an abstract method in an outer class AND the concrete - override in an inner class), the method whose ``class_name`` matches - this value is preferred as the actual replacement target. Returns: ParsedOptimization with the method and any additional members. @@ -86,17 +80,9 @@ def _parse_optimization_source( target_method_index: int | None = None for i, method in enumerate(methods): if method.name == target_method_name: - # When a target_class_name is known, prefer the method in that class - # (e.g. ObjectUnpacker.getString over the abstract outer getString). - # Still accept any match as fallback if no class-specific one is found. - if target_class_name is None or method.class_name == target_class_name: - target_method = method - target_method_index = i - break - elif target_method is None: - # Keep as tentative fallback (class didn't match yet) - target_method = method - target_method_index = i + target_method = method + target_method_index = i + break if target_method: # Extract target method source (including Javadoc if present) @@ -116,11 +102,6 @@ def _parse_optimization_source( # Skip methods whose line range falls entirely inside the target method's # range, as these belong to anonymous/inner classes inside the target body # and must not be hoisted out as top-level class members. - # Also skip methods that belong to a different class than the target — this - # handles the case where the target is in an inner class and the generated - # code also contains outer-class methods that must not be injected into the - # inner class (outer-class methods would reference type parameters or instance - # variables that are not in scope inside a static inner class). lines = new_source.splitlines(keepends=True) for i, method in enumerate(methods): if method.name != target_method_name: @@ -129,9 +110,6 @@ def _parse_optimization_source( method.start_line >= target_method.start_line and method.end_line <= target_method.end_line ): continue - # Skip methods from a different class than the target method - if target_method and method.class_name != target_method.class_name: - continue start = (method.javadoc_start_line or method.start_line) - 1 end = method.end_line helper_source = "".join(lines[start:end]) @@ -434,10 +412,7 @@ def replace_function( func_end_line = function.ending_line # Parse the optimization to extract components. - # Pass the class name so that when the generated code contains multiple - # methods with the same name (outer abstract + inner concrete), the correct - # one is selected as the replacement target. - parsed = _parse_optimization_source(new_source, func_name, analyzer, target_class_name=function.class_name) + parsed = _parse_optimization_source(new_source, func_name, analyzer) # If the parsed optimization has no valid target source (e.g., the LLM generated # a method with a different name), skip this candidate entirely. diff --git a/tests/test_languages/test_java/test_context.py b/tests/test_languages/test_java/test_context.py index 41c8b7714..27a3eb3a7 100644 --- a/tests/test_languages/test_java/test_context.py +++ b/tests/test_languages/test_java/test_context.py @@ -1486,7 +1486,11 @@ class TestExtractCodeContextWithInnerClasses: """Tests for extract_code_context with inner/nested classes.""" def test_static_nested_class_method(self, tmp_path: Path): - """Test context extraction for static nested class method.""" + """Inner class methods are excluded from discovery and cannot be context-extracted. + + Methods of static nested classes are skipped in discovery because they + cannot be reliably instrumented or tested in isolation. + """ java_file = tmp_path / "Container.java" java_file.write_text("""public class Container { public static class Nested { @@ -1498,26 +1502,15 @@ def test_static_nested_class_method(self, tmp_path: Path): """) functions = discover_functions_from_source(java_file.read_text(), file_path=java_file) compute_func = next((f for f in functions if f.function_name == "compute"), None) - assert compute_func is not None - - context = extract_code_context(compute_func, tmp_path) - - # Inner class wrapped in outer class skeleton - assert ( - context.target_code - == """public class Container { - public static class Nested { - public int compute(int x) { - return x * 2; - } - } -} -""" - ) - assert context.read_only_context == "" + # Inner class method must NOT be discovered + assert compute_func is None def test_inner_class_method(self, tmp_path: Path): - """Test context extraction for inner class method.""" + """Inner class methods are excluded from discovery and cannot be context-extracted. + + Methods of non-static inner classes are skipped in discovery because they + require an outer instance and cannot be instrumented independently. + """ java_file = tmp_path / "Outer.java" java_file.write_text("""public class Outer { private int value = 10; @@ -1531,23 +1524,8 @@ def test_inner_class_method(self, tmp_path: Path): """) functions = discover_functions_from_source(java_file.read_text(), file_path=java_file) get_func = next((f for f in functions if f.function_name == "getValue"), None) - assert get_func is not None - - context = extract_code_context(get_func, tmp_path) - - # Inner class wrapped in outer class skeleton - assert ( - context.target_code - == """public class Outer { - public class Inner { - public int getValue() { - return value; - } - } -} -""" - ) - assert context.read_only_context == "" + # Inner class method must NOT be discovered + assert get_func is None class TestExtractCodeContextWithEnumAndInterface: diff --git a/tests/test_languages/test_java/test_discovery.py b/tests/test_languages/test_java/test_discovery.py index 9411a30c4..e42cfe8c2 100644 --- a/tests/test_languages/test_java/test_discovery.py +++ b/tests/test_languages/test_java/test_discovery.py @@ -333,3 +333,122 @@ def test_discover_tests_from_fixture(self, java_fixture_path: Path): tests = discover_test_methods(test_file) assert len(tests) > 0 + + +class TestInnerClassMethodFilter: + """Tests that methods of nested/inner classes are excluded from discovery. + + Inner class methods cannot be reliably instrumented or tested in isolation: + - Non-static inner classes require an outer instance + - Protected methods are inaccessible from external test code + - The instrumentation layer is not class-aware (wraps by method name only) + + Discovery must skip all methods whose enclosing class is itself nested inside + another class. + """ + + def test_static_inner_class_methods_are_excluded(self): + """Methods in a static nested class must not be discovered.""" + source = """\ +public abstract class Unpacker { + protected abstract T getString(String value); + + public T unpackString() { + return getString(null); + } + + public static final class ObjectUnpacker extends Unpacker { + public ObjectUnpacker() {} + + @Override + protected Object getString(String value) { + return value; + } + + public Object helper() { + return null; + } + } +} +""" + functions = discover_functions_from_source(source) + # Only the outer class method unpackString() should be discovered. + # ObjectUnpacker.getString and ObjectUnpacker.helper are inner-class methods + # and must be excluded. + function_names = {f.function_name for f in functions} + assert "unpackString" in function_names + assert "getString" not in function_names + assert "helper" not in function_names + + def test_non_static_inner_class_methods_are_excluded(self): + """Methods in a non-static inner class must not be discovered.""" + source = """\ +public class Outer { + private int value; + + public int getValue() { + return value; + } + + public class Inner { + public int doubleValue() { + return value * 2; + } + } +} +""" + functions = discover_functions_from_source(source) + function_names = {f.function_name for f in functions} + assert "getValue" in function_names + assert "doubleValue" not in function_names + + def test_outer_class_methods_are_still_discovered(self): + """Outer-class methods must be discovered normally even when inner classes exist.""" + source = """\ +public class Container { + public int size() { + return 0; + } + + public boolean isEmpty() { + return true; + } + + private static class InnerHelper { + public void doWork() {} + } +} +""" + functions = discover_functions_from_source(source) + function_names = {f.function_name for f in functions} + assert "size" in function_names + assert "isEmpty" in function_names + # Inner class method must be excluded + assert "doWork" not in function_names + + def test_deeply_nested_class_methods_are_excluded(self): + """Methods in classes nested more than two levels deep must also be excluded.""" + source = """\ +public class Level1 { + public int method1() { + return 1; + } + + public static class Level2 { + public int method2() { + return 2; + } + + public static class Level3 { + public int method3() { + return 3; + } + } + } +} +""" + functions = discover_functions_from_source(source) + function_names = {f.function_name for f in functions} + assert "method1" in function_names + assert "method2" not in function_names + assert "method3" not in function_names diff --git a/tests/test_languages/test_java/test_replacement.py b/tests/test_languages/test_java/test_replacement.py index 37a0c8a14..7d2461e33 100644 --- a/tests/test_languages/test_java/test_replacement.py +++ b/tests/test_languages/test_java/test_replacement.py @@ -832,8 +832,12 @@ def test_replace_multiple_methods(self, tmp_path: Path): class TestNestedClasses: """Tests for nested class scenarios.""" - def test_replace_method_in_nested_class(self, tmp_path: Path): - """Test replacing a method in a nested class.""" + def test_inner_class_method_is_not_replaced(self, tmp_path: Path): + """Inner-class methods are not supported for optimization and must be skipped. + + Methods of static nested or non-static inner classes are excluded from + discovery and therefore cannot be replaced via the high-level API. + """ java_file = tmp_path / "Outer.java" original_code = """public class Outer { public int outerMethod() { @@ -865,6 +869,8 @@ def test_replace_method_in_nested_class(self, tmp_path: Path): optimized_code = CodeStringsMarkdown.parse_markdown_code(optimized_markdown, expected_language="java") + # Inner class methods are excluded from discovery, so the replacement + # is a no-op and the original file must remain unchanged. result = replace_function_definitions_for_language( function_names=["innerMethod"], optimized_code=optimized_code, @@ -872,21 +878,9 @@ def test_replace_method_in_nested_class(self, tmp_path: Path): project_root_path=tmp_path, ) - assert result is True - new_code = java_file.read_text(encoding="utf-8") - expected = """public class Outer { - public int outerMethod() { - return 1; - } - - public static class Inner { - public int innerMethod() { - return 2 + 0; - } - } -} -""" - assert new_code == expected + assert result is False + # File must be unchanged + assert java_file.read_text(encoding="utf-8") == original_code class TestPreservesStructure: @@ -1929,141 +1923,6 @@ def test_anonymous_iterator_methods_not_hoisted_to_class(self, tmp_path): assert new_code == expected_code -class TestInnerClassHelperFilter: - """Tests that outer-class methods are not injected into a static inner class. - - When the target method lives in a *static* inner class (e.g. ObjectUnpacker), - the generated optimisation class typically wraps the inner class inside the - outer class. Methods that belong to the outer class must NOT be extracted as - helpers and inserted into the inner class — they would reference outer-class - type parameters or instance variables that are unavailable in a static context. - """ - - def test_outer_class_methods_not_injected_into_static_inner_class(self, tmp_path): - """Reproduces the Unpacker.ObjectUnpacker.getString bug. - - The outer class ``Unpacker`` has a method ``getString(String)``. - When the LLM generates an optimisation for ``ObjectUnpacker.getString``, - the generated file still contains the outer ``Unpacker`` skeleton. - Codeflash must NOT inject the outer ``getString`` helper into the - ``ObjectUnpacker`` inner class. - """ - from codeflash.discovery.functions_to_optimize import FunctionToOptimize, FunctionParent - from codeflash.languages.java.replacement import replace_function - - original_code = """\ -public abstract class Unpacker { - protected byte[] buffer; - protected int offset; - protected int length; - - public Unpacker(byte[] buffer, int offset, int length) { - this.buffer = buffer; - this.offset = offset; - this.length = length; - } - - protected abstract T getString(String value); - - public T unpackString() { - return getString(new String(buffer, offset, length)); - } - - public static final class ObjectUnpacker extends Unpacker { - public ObjectUnpacker(byte[] buffer, int offset, int length) { - super(buffer, offset, length); - } - - @Override - protected Object getString(String value) { - return value; - } - } -} -""" - java_file = tmp_path / "Unpacker.java" - java_file.write_text(original_code, encoding="utf-8") - - # LLM-generated optimisation: the outer class is present in the generated - # code, but only ObjectUnpacker.getString is the actual optimisation target. - optimized_source = """\ -public abstract class Unpacker { - protected byte[] buffer; - protected int offset; - protected int length; - - public Unpacker(byte[] buffer, int offset, int length) { - this.buffer = buffer; - this.offset = offset; - this.length = length; - } - - protected abstract T getString(String value); - - public T unpackString() { - return getString(new String(buffer, offset, length)); - } - - public static final class ObjectUnpacker extends Unpacker { - public ObjectUnpacker(byte[] buffer, int offset, int length) { - super(buffer, offset, length); - } - - @Override - protected Object getString(String value) { - return value.intern(); - } - } -} -""" - - func = FunctionToOptimize( - function_name="getString", - file_path=java_file, - starting_line=21, - ending_line=23, - parents=[FunctionParent(name="ObjectUnpacker", type="ClassDef")], - is_method=True, - language="java", - ) - - new_code = replace_function(original_code, func, optimized_source) - - # The outer-class unpackString() method must NOT be inserted into ObjectUnpacker. - # The result should only differ from the original in the ObjectUnpacker.getString body. - expected_code = """\ -public abstract class Unpacker { - protected byte[] buffer; - protected int offset; - protected int length; - - public Unpacker(byte[] buffer, int offset, int length) { - this.buffer = buffer; - this.offset = offset; - this.length = length; - } - - protected abstract T getString(String value); - - public T unpackString() { - return getString(new String(buffer, offset, length)); - } - - public static final class ObjectUnpacker extends Unpacker { - public ObjectUnpacker(byte[] buffer, int offset, int length) { - super(buffer, offset, length); - } - - @Override - protected Object getString(String value) { - return value.intern(); - } - } -} -""" - assert new_code == expected_code - - class TestConstructorReplacement: """Tests that constructors in the generated class are propagated to the original.