reslove PR comments

django/aiservice/languages/java/optimizer.py

@@ -94,7 +94,7 @@ def extract_code_and_explanation(
     return "", content
 
 
-def _remove_strings_and_comments(code: str) -> str:
+def _remove_strings_and_comments(code: str) -> str | None:
     """Remove string literals and comments from Java code.
 
     This allows accurate counting of braces/brackets/parentheses
@@ -104,7 +104,8 @@ def _remove_strings_and_comments(code: str) -> str:
         code: The Java source code
 
     Returns:
-        Code with strings and comments replaced by spaces
+        Code with strings and comments replaced by spaces,
+        or None if unterminated string/comment is detected
 
     """
     result = []
@@ -131,6 +132,9 @@ def _remove_strings_and_comments(code: str) -> str:
                 result.append(" ")
                 result.append(" ")
                 i += 2
+            else:
+                # Unterminated multi-line comment
+                return None
         # Check for string literal (double quotes)
         elif code[i] == '"':
             result.append(" ")
@@ -147,6 +151,9 @@ def _remove_strings_and_comments(code: str) -> str:
             if i < length:
                 result.append(" ")
                 i += 1
+            else:
+                # Unterminated string literal
+                return None
         # Check for character literal (single quotes)
         elif code[i] == "'":
             result.append(" ")
@@ -163,6 +170,9 @@ def _remove_strings_and_comments(code: str) -> str:
             if i < length:
                 result.append(" ")
                 i += 1
+            else:
+                # Unterminated character literal
+                return None
         else:
             result.append(code[i])
             i += 1
@@ -178,6 +188,7 @@ def validate_java_syntax(code: str) -> bool:
 
     Checks performed:
     - Non-empty code
+    - No unterminated strings, comments, or character literals
     - Balanced and properly nested braces {}, brackets [], and parentheses ()
     - Strings and comments are ignored when checking balance
 
@@ -195,6 +206,10 @@ def validate_java_syntax(code: str) -> bool:
     # Remove strings and comments to avoid false positives
     clean_code = _remove_strings_and_comments(code)
 
+    # Check for unterminated strings/comments
+    if clean_code is None:
+        return False
+
     # Check for balanced and properly nested delimiters using a stack
     stack: list[str] = []
     matching = {")": "(", "]": "[", "}": "{"}

django/aiservice/tests/optimizer/test_optimizer_java.py

@@ -1,183 +1,11 @@
-"""Tests for Java optimizer module.
+"""Tests for Java optimizer module."""
 
-Tests the code extraction and normalization functions without importing
-the full optimizer module (to avoid LLM dependencies in tests).
-"""
-
-import re
-
-# Pattern to extract code blocks from LLM response (handles both ```java and ```java:filename.java)
-JAVA_CODE_PATTERN = re.compile(r"```(?:java)(?::[^\n]*)?\s*\n(.*?)```", re.MULTILINE | re.DOTALL)
-
-# Pattern to extract code blocks with file paths (multi-file context)
-JAVA_CODE_WITH_PATH_PATTERN = re.compile(r"```(?:java):([^\n]+)\n(.*?)```", re.MULTILINE | re.DOTALL)
-
-
-def extract_code_and_explanation(
-    content: str, is_multi_file: bool = False
-) -> tuple[str | dict[str, str], str]:
-    """Extract code and explanation from LLM response.
-
-    Args:
-        content: The raw LLM response content
-        is_multi_file: Whether to expect multi-file format
-
-    Returns:
-        Tuple of (code, explanation) where code is a string for single file
-        or dict[str, str] for multi-file
-
-    """
-    if is_multi_file:
-        # Extract all code blocks with file paths
-        matches = JAVA_CODE_WITH_PATH_PATTERN.findall(content)
-        if matches:
-            file_to_code: dict[str, str] = {}
-            first_match_pos = content.find("```")
-            explanation = content[:first_match_pos].strip() if first_match_pos > 0 else ""
-
-            for file_path, code in matches:
-                file_to_code[file_path.strip()] = code.strip()
-
-            return file_to_code, explanation
-
-        # Fall back to single file extraction
-        return extract_code_and_explanation(content, is_multi_file=False)
-
-    # Single file extraction
-    match = JAVA_CODE_PATTERN.search(content)
-    if match:
-        code = match.group(1).strip()
-        # Explanation is everything before the code block
-        explanation_end = match.start()
-        explanation = content[:explanation_end].strip()
-        return code, explanation
-
-    # No code block found, return empty code
-    return "", content
-
-
-def is_multi_context_java(source_code: str) -> bool:
-    """Check if source code contains multiple Java file blocks."""
-    return source_code.count("```java:") >= 1
-
-
-def _remove_strings_and_comments(code: str) -> str:
-    """Remove string literals and comments from Java code.
-
-    This allows accurate counting of braces/brackets/parentheses
-    without being affected by characters inside strings or comments.
-
-    Args:
-        code: The Java source code
-
-    Returns:
-        Code with strings and comments replaced by spaces
-
-    """
-    result = []
-    i = 0
-    length = len(code)
-
-    while i < length:
-        # Check for single-line comment
-        if i + 1 < length and code[i] == "/" and code[i + 1] == "/":
-            # Skip until end of line
-            while i < length and code[i] != "\n":
-                result.append(" ")
-                i += 1
-        # Check for multi-line comment
-        elif i + 1 < length and code[i] == "/" and code[i + 1] == "*":
-            result.append(" ")
-            result.append(" ")
-            i += 2
-            # Skip until */
-            while i + 1 < length and not (code[i] == "*" and code[i + 1] == "/"):
-                result.append(" " if code[i] != "\n" else "\n")
-                i += 1
-            if i + 1 < length:
-                result.append(" ")
-                result.append(" ")
-                i += 2
-        # Check for string literal (double quotes)
-        elif code[i] == '"':
-            result.append(" ")
-            i += 1
-            # Skip until closing quote, handling escapes
-            while i < length and code[i] != '"':
-                if code[i] == "\\" and i + 1 < length:
-                    result.append(" ")
-                    result.append(" ")
-                    i += 2
-                else:
-                    result.append(" ")
-                    i += 1
-            if i < length:
-                result.append(" ")
-                i += 1
-        # Check for character literal (single quotes)
-        elif code[i] == "'":
-            result.append(" ")
-            i += 1
-            # Skip until closing quote, handling escapes
-            while i < length and code[i] != "'":
-                if code[i] == "\\" and i + 1 < length:
-                    result.append(" ")
-                    result.append(" ")
-                    i += 2
-                else:
-                    result.append(" ")
-                    i += 1
-            if i < length:
-                result.append(" ")
-                i += 1
-        else:
-            result.append(code[i])
-            i += 1
-
-    return "".join(result)
-
-
-def validate_java_syntax(code: str) -> bool:
-    """Basic Java syntax validation.
-
-    This is a simple check that the code has basic Java structure.
-    Full validation happens at the client side with the Java compiler.
-
-    Checks performed:
-    - Non-empty code
-    - Balanced and properly nested braces {}, brackets [], and parentheses ()
-    - Strings and comments are ignored when checking balance
-
-    Args:
-        code: The Java source code to validate
-
-    Returns:
-        True if the code passes basic validation
-
-    """
-    # Check for basic Java structure
-    if not code.strip():
-        return False
-
-    # Remove strings and comments to avoid false positives
-    clean_code = _remove_strings_and_comments(code)
-
-    # Check for balanced and properly nested delimiters using a stack
-    stack: list[str] = []
-    matching = {")": "(", "]": "[", "}": "{"}
-    opening = set(matching.values())
-    closing = set(matching.keys())
-
-    for char in clean_code:
-        if char in opening:
-            stack.append(char)
-        elif char in closing:
-            if not stack or stack[-1] != matching[char]:
-                return False
-            stack.pop()
-
-    # Stack should be empty if all delimiters are balanced
-    return len(stack) == 0
+from languages.java.optimizer import (
+    _remove_strings_and_comments,
+    extract_code_and_explanation,
+    is_multi_context_java,
+    validate_java_syntax,
+)
 
 
 class TestExtractCodeAndExplanation:
@@ -885,6 +713,45 @@ public class Test {
         """Test newlines-only code fails."""
         assert not validate_java_syntax("\n\n\n")
 
+    def test_unterminated_string_fails(self) -> None:
+        """Test that unterminated string literal fails validation."""
+        code = '''
+public class Test {
+    String s = "this string never ends
+}
+'''
+        assert not validate_java_syntax(code)
+
+    def test_unterminated_char_literal_fails(self) -> None:
+        """Test that unterminated character literal fails validation."""
+        code = """
+public class Test {
+    char c = 'x
+}
+"""
+        assert not validate_java_syntax(code)
+
+    def test_unterminated_multiline_comment_fails(self) -> None:
+        """Test that unterminated multi-line comment fails validation."""
+        code = """
+public class Test {
+    /* This comment never ends
+    public void method() {
+    }
+}
+"""
+        assert not validate_java_syntax(code)
+
+    def test_unterminated_string_with_balanced_braces_fails(self) -> None:
+        """Test that unterminated string fails even if braces would be balanced."""
+        # Without the fix, this would incorrectly return True because
+        # the unterminated string would consume everything to EOF
+        code = '''
+public class Test {
+    String s = "unterminated
+'''
+        assert not validate_java_syntax(code)
+
 
 class TestRemoveStringsAndComments:
     """Test the _remove_strings_and_comments helper function."""
@@ -938,3 +805,28 @@ class TestRemoveStringsAndComments:
         result = _remove_strings_and_comments(code)
         # The code after the string should be preserved
         assert "int x = 1;" in result
+
+    def test_returns_none_for_unterminated_string(self) -> None:
+        """Test that unterminated string returns None."""
+        code = 'String s = "unterminated'
+        result = _remove_strings_and_comments(code)
+        assert result is None
+
+    def test_returns_none_for_unterminated_char_literal(self) -> None:
+        """Test that unterminated character literal returns None."""
+        code = "char c = 'x"
+        result = _remove_strings_and_comments(code)
+        assert result is None
+
+    def test_returns_none_for_unterminated_multiline_comment(self) -> None:
+        """Test that unterminated multi-line comment returns None."""
+        code = "int x = 1; /* comment never ends"
+        result = _remove_strings_and_comments(code)
+        assert result is None
+
+    def test_single_line_comment_at_eof_is_valid(self) -> None:
+        """Test that single-line comment at EOF is valid (no newline needed)."""
+        code = "int x = 1; // comment at end"
+        result = _remove_strings_and_comments(code)
+        assert result is not None
+        assert "int x = 1;" in result