Avoid removing all contents when formatting file with syntax error (#314

)

Exploring a resolution to
astral-sh/ruff-vscode#336

It looks like at
v0.0.42...v0.0.43#diff-9cc81288343e2240bb3387d88ab41ebf706e40c86af24cf96bad8919a48d5cceL762-L763
we no longer exit early if stdout is empty. This pull request does not
restore that logic, but rather adds a return code check and exits if
there is a bad return code.

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

- Add failing test case for formatting code with a syntax error
- Track return code of `ruff format`
- Do not format documents with non-zero return code from Ruff

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

<!-- How was it tested? -->

ruff_lsp/server.py

@@ -1095,10 +1095,14 @@ async def apply_organize_imports(arguments: tuple[TextDocument]):
 async def apply_format(arguments: tuple[TextDocument]):
     uri = arguments[0]["uri"]
     document = Document.from_uri(uri)
-    results = await _run_format_on_document(document)
-    workspace_edit = _result_to_workspace_edit(document, results)
+
+    result = await _run_format_on_document(document)
+    if result is None or result.exit_code != 0:
+        return None
+
+    workspace_edit = _result_to_workspace_edit(document, result)
     if workspace_edit is None:
-        return
+        return None
     LSP_SERVER.apply_edit(workspace_edit, "Ruff: Format document")
 
 
@@ -1108,8 +1112,9 @@ async def format_document(params: DocumentFormattingParams) -> list[TextEdit] |
     # request itself can only act on a text document. A cell in a Notebook is
     # represented as a text document.
     document = Document.from_cell_or_text_uri(params.text_document.uri)
+
     result = await _run_format_on_document(document)
-    if result is None:
+    if result is None or result.exit_code != 0:
         return None
 
     if document.kind is DocumentKind.Cell:
@@ -1330,7 +1335,10 @@ async def run_path(
         stdin=asyncio.subprocess.PIPE,
         cwd=cwd,
     )
-    result = RunResult(*await process.communicate(input=source.encode("utf-8")))
+    result = RunResult(
+        *await process.communicate(input=source.encode("utf-8")),
+        exit_code=await process.wait(),
+    )
 
     if result.stderr:
         log_to_output(result.stderr.decode("utf-8"))

ruff_lsp/utils.py

@@ -69,6 +69,7 @@ def version(executable: str) -> Version:
 class RunResult:
     """Object to hold result from running tool."""
 
-    def __init__(self, stdout: bytes, stderr: bytes):
+    def __init__(self, stdout: bytes, stderr: bytes, exit_code: int):
         self.stdout: bytes = stdout
         self.stderr: bytes = stderr
+        self.exit_code: int = exit_code

tests/test_format.py

@@ -40,7 +40,33 @@ async def test_format(tmp_path, ruff_version: Version):
     with handle_unsupported:
         result = await _run_format_on_document(document)
         assert result is not None
+        assert result.exit_code == 0
         [edit] = _fixed_source_to_edits(
             original_source=document.source, fixed_source=result.stdout.decode("utf-8")
         )
         assert edit.new_text == expected
+
+
+@pytest.mark.asyncio
+async def test_format_code_with_syntax_error(tmp_path, ruff_version: Version):
+    source = """
+foo =
+"""
+
+    test_file = tmp_path.joinpath("main.py")
+    test_file.write_text(source)
+    uri = utils.as_uri(str(test_file))
+
+    workspace = Workspace(str(tmp_path))
+    document = Document.from_text_document(workspace.get_text_document(uri))
+
+    handle_unsupported = (
+        pytest.raises(RuntimeError, match=f"Ruff .* required, but found {ruff_version}")
+        if not VERSION_REQUIREMENT_FORMATTER.contains(ruff_version)
+        else nullcontext()
+    )
+
+    with handle_unsupported:
+        result = await _run_format_on_document(document)
+        assert result is not None
+        assert result.exit_code == 2