tawada / grass-grower

0 stars 0 forks source link

Improve `apply_modification` function logic for accurate code replacement and better handling of edge cases. #114

Open tawada opened 3 months ago

tawada commented 3 months ago

After examining your code, one notable issue is in the function apply_modification in the logic/code_modification.py module. The logic that ensures the actual modification of code in the file can be improved for better accuracy and to prevent potential issues when the code modification doesn't occur as expected.

Here's a concise explanation:

Issue:

In the apply_modification function, it checks if the file exists and replaces the before_code with the after_code. However, this code does not account for cases where multiple occurrences of before_code might exist or if the before_code does not match exactly due to white spaces or other subtle differences.

Code:

Here’s the existing logic:

def apply_modification(repo_name: str, modification: CodeModification) -> bool:
    # ... existing code

    if os.path.exists(file_path):
        before_code = logic_utils.get_file_content(file_path, newline="")
        after_code = before_code.replace(modification.before_code, modification.after_code)
        if before_code == after_code:
            raise logic_exceptions.CodeNotModifiedError("Code has no changes")
    else:
        after_code = modification.after_code

    logic_utils.write_to_file(file_path, after_code, newline="")
    return True

Recommendation:

Instead of directly using str.replace() which replaces all occurrences and does not notify us if the string was found, consider using regular expressions to look for exact matches and replace them. Also, handle cases where before_code might not exist exactly as expected by adding necessary logging or validation.

Suggested Refactor:

import re

def apply_modification(repo_name: str, modification: CodeModification) -> bool:
    # ... existing code

    if os.path.exists(file_path):
        before_code = logic_utils.get_file_content(file_path, newline="")

        # Use regex for accurate replacement and total-control over the replacement process
        pattern = re.compile(re.escape(modification.before_code))
        if not pattern.search(before_code):
            raise logic_exceptions.CodeNotModifiedError("Code has no changes")

        after_code = pattern.sub(modification.after_code, before_code, count=1)  # replace only the first occurrence
    else:
        after_code = modification.after_code

    logic_utils.write_to_file(file_path, after_code, newline="")
    return True

Reasoning:

  1. Regex Control: By using regular expressions, you have more control over how the replacement is performed, ensuring only the exact occurrences are modified.
  2. Detailed Conditions: This approach can better handle edge cases where before_code may have subtle differences that str.replace() might miss.
  3. Single Occurrence Replacement: It replaces only the first occurrence by specifying count=1, which might be beneficial depending on the intended behavior.

This adjustment improves reliability and correctness in applying code modifications, which is critical for maintaining code integrity and avoiding unexpected issues.