WIP for issue All-Hands-AI#2220

Updated aider linter to:
    * Always return text and line numbers
    * Moved extract line number more consistently
    * Changed pylint to stop after first linter detects errors
Updated agentskills
    * To get back a LintResult object and then use lines and text for error message and related line number
    * Moved code for extracting line number to aider linter
Tests:
* Added additional unit tests for aider to test for
* Return values from lint failures
* Confirm linter works for non-configured languages like Ruby

opendevin/runtime/aider/linter.py

@@ -14,6 +14,11 @@
 # tree_sitter is throwing a FutureWarning
 warnings.simplefilter('ignore', category=FutureWarning)
 
+@dataclass
+class LintResult:
+    text: str
+    lines: list
+
 
 class Linter:
     def __init__(self, encoding='utf-8', root=None):
@@ -54,13 +59,8 @@ def run_cmd(self, cmd, rel_fname, code):
         cmd = ' '.join(cmd)
         res = ''
         res += errors
-
-        linenums = []
-        filenames_linenums = find_filenames_and_linenums(errors, [rel_fname])
-        if filenames_linenums:
-            filename, linenums = next(iter(filenames_linenums.items()))
-            linenums = [num - 1 for num in linenums]
-        return LintResult(text=res, lines=linenums)
+        line_num = extract_error_line_from(res)
+        return LintResult(text=res, lines=[line_num])
 
     def get_abs_fname(self, fname):
         if os.path.isabs(fname): 
@@ -71,7 +71,7 @@ def get_abs_fname(self, fname):
         else: # if a temp file
             return self.get_rel_fname(fname)
 
-    def lint(self, fname, cmd=None):
+    def lint(self, fname, cmd=None) -> LintResult | None:
         code = Path(fname).read_text(self.encoding)
         absolute_fname = self.get_abs_fname(fname)
         if cmd:
@@ -84,41 +84,32 @@ def lint(self, fname, cmd=None):
                 cmd = self.all_lint_cmd
             else:
                 cmd = self.languages.get(lang)
-
         if callable(cmd):
             linkres = cmd(fname, absolute_fname, code)
         elif cmd:
             linkres = self.run_cmd(cmd, absolute_fname, code)
         else:
             linkres = basic_lint(absolute_fname, code)
+        return linkres
 
-        if not linkres:
-            return
-        return linkres.text
-
-    def py_lint(self, fname, rel_fname, code):
-        basic_res = basic_lint(rel_fname, code)
-        compile_res = lint_python_compile(fname, code)
 
+    def flake_lint(self, rel_fname, code):
         fatal = 'E9,F821,F823,F831,F406,F407,F701,F702,F704,F706'
         flake8 = f'flake8 --select={fatal} --isolated'
 
         try:
             flake_res = self.run_cmd(flake8, rel_fname, code)
         except FileNotFoundError:
             flake_res = None
-        possible_results = [flake_res, basic_res, compile_res]
-        result_set = [result for result in possible_results if result]
-        if result_set:
-            first_result = result_set[0]
-            return LintResult(first_result.text, first_result.lines)
-
-
-@dataclass
-class LintResult:
-    text: str
-    lines: list
+        return flake_res
 
+    def py_lint(self, fname, rel_fname, code):
+        error = self.flake_lint(rel_fname, code)
+        if not error:
+            error = lint_python_compile(fname, code)
+        if not error:
+            error = basic_lint(rel_fname, code)
+        return error
 
 def lint_python_compile(fname, code):
     try:
@@ -137,7 +128,6 @@ def lint_python_compile(fname, code):
             if target in tb_lines[i]:
                 last_file_i = i
                 break
-
         tb_lines = tb_lines[:1] + tb_lines[last_file_i + 1 :]
 
     res = ''.join(tb_lines)
@@ -159,9 +149,21 @@ def basic_lint(fname, code):
     errors = traverse_tree(tree.root_node)
     if not errors:
         return
-
-    return LintResult(text='', lines=errors)
-
+    return LintResult(text=f'{fname}:{errors[0]}', lines=errors)
+
+def extract_error_line_from(lint_error):
+    # moved from opendevin.agentskills#_lint_file
+    for line in lint_error.splitlines(True):
+        if line.strip():
+            # The format of the error message is: <filename>:<line>:<column>: <error code> <error message>
+            parts = line.split(':')
+            if len(parts) >= 2:
+                try:
+                    first_error_line = int(parts[1])
+                    break
+                except ValueError:
+                    continue
+    return first_error_line
 
 def tree_context(fname, code, line_nums):
     context = TreeContext(

opendevin/runtime/plugins/agent_skills/agentskills.py

@@ -127,25 +127,12 @@ def _lint_file(file_path: str) -> tuple[Optional[str], Optional[int]]:
         tuple[str, Optional[int]]: (lint_error, first_error_line_number)
     """
     linter = Linter(root=os.getcwd())
-    first_error_line = None
-    error_message = linter.lint(file_path)
+    lint_result = linter.lint(file_path)
     if linter.returncode == 0:
         # Linting successful. No issues found.
         return None, None
-    lint_error = 'ERRORS:\n' + error_message
-
-    for line in lint_error.splitlines(True):
-        if line.strip():
-            # The format of the error message is: <filename>:<line>:<column>: <error code> <error message>
-            parts = line.split(':')
-            if len(parts) >= 2:
-                try:
-                    first_error_line = int(parts[1])
-                    break
-                except ValueError:
-                    # Not a valid line number, continue to the next line
-                    continue
-    return lint_error, first_error_line
+    lint_error = 'ERRORS:\n' + lint_result.text
+    return lint_error, lint_result.lines[0]
 
 
 def _print_window(file_path, targeted_line, WINDOW, return_str=False):
@@ -433,6 +420,7 @@ def _edit_or_append_file(
                     '[Your proposed edit has introduced new syntax error(s). Please understand the errors and retry your edit command.]'
                 )
                 print(lint_error)
+
                 print('[This is how your edit would have looked if applied]')
                 print('-------------------------------------------------')
                 _print_window(file_name, CURRENT_LINE, 10)

tests/unit/test_aider_linter.py

@@ -18,6 +18,32 @@ def temp_file(tmp_path):
     yield temp_name
     os.remove(temp_name)
 
+@pytest.fixture
+def temp_ruby_file_errors(tmp_path):
+    # Fixture to create a temporary file
+    temp_name = os.path.join(tmp_path, 'lint-test.rb')
+    with open(temp_name, 'w', encoding='utf-8') as tmp_file:
+        tmp_file.write("""def foo():
+    print("Hello, World!")
+foo()
+""")
+    tmp_file.close()
+    yield temp_name
+    os.remove(temp_name)
+
+@pytest.fixture
+def temp_ruby_file_correct(tmp_path):
+    # Fixture to create a temporary file
+    temp_name = os.path.join(tmp_path, 'lint-test.rb')
+    with open(temp_name, 'w', encoding='utf-8') as tmp_file:
+        tmp_file.write("""def foo
+  puts "Hello, World!"
+end
+foo
+""")
+    tmp_file.close()
+    yield temp_name
+    os.remove(temp_name)
 
 @pytest.fixture
 def linter(tmp_path):
@@ -38,13 +64,6 @@ def test_run_cmd(linter, temp_file):
     assert result is None  # echo command should return zero exit status
 
 
-def test_py_lint(linter, temp_file):
-    # Test py_lint method
-    result = linter.py_lint(
-        temp_file, linter.get_rel_fname(temp_file), "print('Hello, World!')\n"
-    )
-
-    assert result is None  # No lint errors expected for this simple code
 
 
 def test_set_linter(linter):
@@ -57,21 +76,72 @@ def custom_linter(fname, rel_fname, code):
     assert 'custom' in linter.languages
     assert linter.languages['custom'] == custom_linter
 
+def test_py_lint(linter, temp_file):
+    # Test py_lint method
+    result = linter.py_lint(
+        temp_file, linter.get_rel_fname(temp_file), "print('Hello, World!')\n"
+    )
+
+    assert result is None  # No lint errors expected for this simple code
+
+
+def test_py_lint_fail(linter, temp_file):
+    # Test py_lint method
+    result = linter.py_lint(
+        temp_file, linter.get_rel_fname(temp_file), "print('Hello, World!')\n"
+    )
+
+    assert result is None
 
 def test_basic_lint(temp_file):
     from opendevin.runtime.aider.linter import basic_lint
+    poorly_formatted_code = """
+        def foo()
+            print("Hello, World!")
+        print("Wrong indent")
+        foo(
+        """
+    result = basic_lint(temp_file, poorly_formatted_code)
+
+    assert isinstance(result, LintResult)
+    assert result.text == f"{temp_file}:1"
+    assert 1 in result.lines
+
+def test_basic_lint_fail_returns_text_and_lines(temp_file):
+    from opendevin.runtime.aider.linter import basic_lint
+    poorly_formatted_code = """
+        def foo()
+            print("Hello, World!")
+        print("Wrong indent")
+        foo(
+        """
 
-    result = basic_lint(temp_file, "print('Hello, World!')\n")
+    result = basic_lint(temp_file, poorly_formatted_code)
 
-    assert result is None  # No basic lint errors expected for this simple code
+    assert isinstance(result, LintResult)
+    assert result.text == f"{temp_file}:1"
+    assert 1 in result.lines
 
 
 def test_lint_python_compile(temp_file):
     from opendevin.runtime.aider.linter import lint_python_compile
 
     result = lint_python_compile(temp_file, "print('Hello, World!')\n")
 
-    assert result is None  # No compile errors expected for this simple code
+    assert result is None
+
+def test_lint_python_compile_fail_returns_text_and_lines(temp_file):
+    from opendevin.runtime.aider.linter import lint_python_compile
+    poorly_formatted_code = """
+        def foo()
+            print("Hello, World!")
+        print("Wrong indent")
+        foo(
+        """
+    result = lint_python_compile(temp_file, poorly_formatted_code)
+
+    assert temp_file in result.text
+    assert 1 in result.lines
 
 
 def test_lint(linter, temp_file):
@@ -92,4 +162,17 @@ def foo()
 """)
     errors = linter.lint(temp_file)
 
-    assert errors is not None
+    assert errors is not None
+
+def test_lint_pass_ruby(linter, temp_ruby_file_correct):
+    result = linter.lint(temp_ruby_file_correct)
+    assert result is None
+
+def test_lint_fail_ruby(linter, temp_ruby_file_errors):
+    errors = linter.lint(temp_ruby_file_errors)
+    assert errors is not None
+
+
+
+
+