Browse Source

fix potential flake8 miss checking (#3124)

* fix potential flake8 miss checking

* Add unit test for edit_file_by_replace function with problematic file

* Add unit test for edit_file_by_replace function with problematic file

* Add unit test for edit_file_by_replace function with problematic file

* Add unit test for edit_file_by_replace function with problematic file

* Add unit test for edit_file function with problematic file

* Add unit test for edit_file function with problematic file

* Add unit test for edit_file function with problematic file

* Add unit test for edit_file function with problematic file

* Add unit test for edit_file function with problematic file

* Update opendevin/runtime/plugins/agent_skills/agentskills.py

Co-authored-by: Boxuan Li <liboxuan@connect.hku.hk>

* add test intention description

* fix potential flake8 miss checking

* fix potential flake8 miss checking

* fix potential flake8 miss checking

* fix potential flake8 miss checking

* fix potential flake8 miss checking

---------

Co-authored-by: Boxuan Li <liboxuan@connect.hku.hk>
Co-authored-by: Graham Neubig <neubig@gmail.com>
Co-authored-by: tobitege <tobitege@gmx.de>
Bin Lei 1 year ago
parent
commit
ae5f130881

+ 30 - 1
opendevin/runtime/plugins/agent_skills/agentskills.py

@@ -467,6 +467,12 @@ def _edit_file_impl(
 
     try:
         n_added_lines = None
+
+        # lint the original file
+        enable_auto_lint = os.getenv('ENABLE_AUTO_LINT', 'false').lower() == 'true'
+        if enable_auto_lint:
+            original_lint_error, _ = _lint_file(file_name)
+
         # Create a temporary file
         with tempfile.NamedTemporaryFile('w', delete=False) as temp_file:
             temp_file_path = temp_file.name
@@ -502,7 +508,6 @@ def _edit_file_impl(
         # Handle linting
         # NOTE: we need to get env var inside this function
         # because the env var will be set AFTER the agentskills is imported
-        enable_auto_lint = os.getenv('ENABLE_AUTO_LINT', 'false').lower() == 'true'
         if enable_auto_lint:
             # BACKUP the original file
             original_file_backup_path = os.path.join(
@@ -513,6 +518,30 @@ def _edit_file_impl(
                 f.writelines(lines)
 
             lint_error, first_error_line = _lint_file(file_name)
+            
+            # Select the errors caused by the modification
+            def extract_last_part(line):
+                parts = line.split(':')
+                if len(parts) > 1:
+                    return parts[-1].strip()
+                return line.strip()
+
+            def subtract_strings(str1, str2) -> str:
+                lines1 = str1.splitlines()
+                lines2 = str2.splitlines()
+
+                last_parts1 = [extract_last_part(line) for line in lines1]
+
+                remaining_lines = [line for line in lines2 if extract_last_part(line) not in last_parts1]
+
+                result = '\n'.join(remaining_lines)
+                return result
+            if original_lint_error and lint_error:
+                lint_error = subtract_strings(original_lint_error, lint_error)
+                if lint_error == "":
+                    lint_error = None
+                    first_error_line = None
+
             if lint_error is not None:
                 if first_error_line is not None:
                     show_line = int(first_error_line)

+ 44 - 0
tests/unit/test_agent_skill.py

@@ -604,6 +604,50 @@ check(any_int)"""
             assert result == expected
 
 
+def test_edit_file_by_replace_with_multiple_errors(tmp_path):
+    # If the file has multiple errors, but the suggested modification can only fix one error, make sure it is applied.
+    with patch.dict(os.environ, {'ENABLE_AUTO_LINT': 'True'}):
+        content = """def Sum(a,b):
+    try:
+        answer = a + b
+        return answer
+    except Exception:
+        answer = ANOTHER_CONSTANT
+        return answer
+Sum(1,1)
+"""
+
+        temp_file_path = tmp_path / 'problematic-file-test.py'
+        temp_file_path.write_text(content)
+
+        open_file(str(temp_file_path))
+
+        with io.StringIO() as buf:
+            with contextlib.redirect_stdout(buf):
+                edit_file_by_replace(
+                    str(temp_file_path),
+                    to_replace='        answer = a + b',
+                    new_content='        answer = a+b',
+                )
+            result = buf.getvalue()
+            expected = (
+                f'[File: {temp_file_path} (8 lines total after edit)]\n'
+                '(this is the beginning of the file)\n'
+                '1|def Sum(a,b):\n'
+                '2|    try:\n'
+                '3|        answer = a+b\n'
+                '4|        return answer\n'
+                '5|    except Exception:\n'
+                '6|        answer = ANOTHER_CONSTANT\n'
+                '7|        return answer\n'
+                '8|Sum(1,1)\n'
+                '(this is the end of the file)\n'
+                + MSG_FILE_UPDATED.format(line_number=3)
+                + '\n'
+            )
+            assert result.split('\n') == expected.split('\n')
+
+
 # ================================