From 05a9b614785df9c55cc06cb0ba75c5567fdc1af0 Mon Sep 17 00:00:00 2001 From: misrasaurabh1 Date: Tue, 3 Mar 2026 00:03:38 +0000 Subject: [PATCH] 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