[Agent, resolve #4636]: Add linting support to file editor

openhands/runtime/plugins/agent_skills/file_editor/impl.py

@@ -1,7 +1,10 @@
+import tempfile
 from collections import defaultdict
 from pathlib import Path
 from typing import Literal, get_args
 
+from openhands.linter import DefaultLinter, LintResult
+
 from .base import CLIResult, ToolError, ToolResult
 from .run import maybe_truncate, run
 
@@ -27,6 +30,7 @@ class EditTool:
 
     def __init__(self):
         self._file_history = defaultdict(list)
+        self._linter = DefaultLinter()
         super().__init__()
 
     def __call__(
@@ -175,6 +179,9 @@ def str_replace(self, path: Path, old_str: str, new_str: str | None):
         # Save the content to history
         self._file_history[path].append(file_content)
 
+        # Run linting on the changes
+        lint_results = self._run_linting(file_content, new_file_content, path)
+
         # Create a snippet of the edited section
         replacement_line = file_content.split(old_str)[0].count('\n')
         start_line = max(0, replacement_line - SNIPPET_LINES)
@@ -186,6 +193,7 @@ def str_replace(self, path: Path, old_str: str, new_str: str | None):
         success_msg += self._make_output(
             snippet, f'a snippet of {path}', start_line + 1
         )
+        success_msg += '\n' + lint_results + '\n'
         success_msg += 'Review the changes and make sure they are as expected. Edit the file again if necessary.'
 
         return CLIResult(output=success_msg)
@@ -217,6 +225,9 @@ def insert(self, path: Path, insert_line: int, new_str: str):
         new_file_text = '\n'.join(new_file_text_lines)
         snippet = '\n'.join(snippet_lines)
 
+        # Run linting on the changes
+        lint_results = self._run_linting(file_text, new_file_text, path)
+
         self.write_file(path, new_file_text)
         self._file_history[path].append(file_text)
 
@@ -226,6 +237,7 @@ def insert(self, path: Path, insert_line: int, new_str: str):
             'a snippet of the edited file',
             max(1, insert_line - SNIPPET_LINES + 1),
         )
+        success_msg += '\n' + lint_results + '\n'
         success_msg += 'Review the changes and make sure they are as expected (correct indentation, no duplicate lines, etc). Edit the file again if necessary.'
         return CLIResult(output=success_msg)
 
@@ -255,6 +267,34 @@ def write_file(self, path: Path, file: str):
         except Exception as e:
             raise ToolError(f'Ran into {e} while trying to write to {path}') from None
 
+    def _run_linting(self, old_content: str, new_content: str, path: Path) -> str:
+        """Run linting on file changes and return formatted results."""
+        # Create a temporary directory
+        with tempfile.TemporaryDirectory() as temp_dir:
+            # Create paths with exact filenames in temp directory
+            temp_old = Path(temp_dir) / f'old.{path.name}'
+            temp_new = Path(temp_dir) / f'new.{path.name}'
+
+            # Write content to temporary files
+            temp_old.write_text(old_content)
+            temp_new.write_text(new_content)
+
+            # Run linting on the changes
+            results: list[LintResult] = self._linter.lint_file_diff(
+                str(temp_old), str(temp_new)
+            )
+
+            if not results:
+                return 'No linting issues found in the changes.'
+
+            # Format results
+            output = ['Linting issues found in the changes:']
+            for result in results:
+                output.append(
+                    f'- Line {result.line}, Column {result.column}: {result.message}'
+                )
+            return '\n'.join(output) + '\n'
+
     def _make_output(
         self,
         file_content: str,

tests/unit/test_agent_skill.py

@@ -869,6 +869,8 @@ def test_file_editor_str_replace(setup_file):
      1\tLine 1
      2\tNew Line 2
      3\tLine 3
+
+No linting issues found in the changes.
 Review the changes and make sure they are as expected. Edit the file again if necessary."""
     )
 
@@ -923,6 +925,8 @@ def test_file_editor_insert(setup_file):
      2\tLine 2
      3\tInserted Line
      4\tLine 3
+
+No linting issues found in the changes.
 Review the changes and make sure they are as expected (correct indentation, no duplicate lines, etc). Edit the file again if necessary."""
     )
 
@@ -980,6 +984,8 @@ def test_file_editor_undo_edit(setup_file):
      1\tLine 1
      2\tNew Line 2
      3\tLine 3
+
+No linting issues found in the changes.
 Review the changes and make sure they are as expected. Edit the file again if necessary."""
     )
 
@@ -1015,3 +1021,40 @@ def test_file_editor_undo_edit_no_edits(tmp_path):
     result = file_editor(command='undo_edit', path=str(random_file))
     print(result)
     assert result == f'ERROR:\nNo edit history found for {random_file}.'
+
+
+def test_file_editor_linting(tmp_path):
+    """Test that the file editor performs linting checks on edits."""
+    # Create a Python file with valid code
+    test_file = tmp_path / 'test.py'
+    initial_content = """def greet(name):
+    print(f"Hello, {name}!")
+
+greet("World")
+"""
+    test_file.write_text(initial_content)
+
+    # Use str_replace to introduce a linting error (undefined variable)
+    result = file_editor(
+        command='str_replace',
+        path=str(test_file),
+        old_str='greet("World")',
+        new_str='greet(UNDEFINED_VARIABLE)',
+    )
+    print(result)
+    # Verify that the linting error is reported
+    assert 'Linting issues found in the changes:' in result
+    assert "F821 undefined name 'UNDEFINED_VARIABLE'" in result
+
+    # Test that insert also triggers linting
+    result = file_editor(
+        command='insert',
+        path=str(test_file),
+        insert_line=2,
+        new_str='    x = ANOTHER_UNDEFINED_VAR\n',
+    )
+    print(result)
+
+    # Verify that the linting error is reported
+    assert 'Linting issues found in the changes:' in result
+    assert "F821 undefined name 'ANOTHER_UNDEFINED_VAR'" in result