Run ruff format when lsp formatting is invoked

Adds the Subcommand enum to indicate which `ruff` subcommand should be
executed by `run_ruff`. At this time, only `check` and `format` are
supported. As different subcommands support different parameters,
argument generation is delegated based on the specific subcommand value.

The `ruff format` subcommand does not currently organize imports and
there does not appear to be a way to convince it to do so. Until a
unified command exists the approach taken here is to format and then
make a second run of `ruff check` that _only_ performs import
formatting.

Author: mmcshane

pylsp_ruff/plugin.py

@@ -1,3 +1,4 @@
+import enum
 import json
 import logging
 import re
@@ -45,6 +46,7 @@
     r"(?::\s?(?P<codes>([A-Z]+[0-9]+(?:[,\s]+)?)+))?"
 )
 
+
 UNNECESSITY_CODES = {
     "F401",  # `module` imported but unused
     "F504",  # % format unused named arguments
@@ -60,6 +62,31 @@
     "H": DiagnosticSeverity.Hint,
 }
 
+ISORT_FIXES = "I"
+
+
+class Subcommand(str, enum.Enum):
+    CHECK = "check"
+    FORMAT = "format"
+
+    def __str__(self) -> str:
+        return self.value
+
+    def build_args(
+        self,
+        document_path: str,
+        settings: PluginSettings,
+        fix: bool = False,
+        extra_arguments: Optional[List[str]] = None,
+    ) -> List[str]:
+        if self == Subcommand.CHECK:
+            return build_check_arguments(document_path, settings, fix, extra_arguments)
+        elif self == Subcommand.FORMAT:
+            return build_format_arguments(document_path, settings, extra_arguments)
+        else:
+            logging.warn(f"subcommand without argument builder '{self}'")
+        return []
+
 
 @hookimpl
 def pylsp_settings():
@@ -103,8 +130,16 @@ def pylsp_format_document(workspace: Workspace, document: Document) -> Generator
         settings=settings, document_path=document.path, document_source=source
     )
 
+    settings.select = [ISORT_FIXES]  # clobber to just run import sorting
+    new_text = run_ruff(
+        settings=settings,
+        document_path=document.path,
+        document_source=new_text,
+        fix=True,
+    )
+
     # Avoid applying empty text edit
-    if new_text == source:
+    if not new_text or new_text == source:
         return
 
     range = Range(
@@ -395,6 +430,7 @@ def run_ruff_check(document: Document, settings: PluginSettings) -> List[RuffChe
         document_path=document.path,
         document_source=document.source,
         settings=settings,
+        subcommand=Subcommand.CHECK,
     )
     try:
         result = json.loads(result)
@@ -418,26 +454,19 @@ def run_ruff_format(
     document_path: str,
     document_source: str,
 ) -> str:
-    fixable_codes = ["I"]
-    if settings.format:
-        fixable_codes.extend(settings.format)
-    extra_arguments = [
-        f"--fixable={','.join(fixable_codes)}",
-    ]
-    result = run_ruff(
+    return run_ruff(
         settings=settings,
         document_path=document_path,
         document_source=document_source,
-        fix=True,
-        extra_arguments=extra_arguments,
+        subcommand=Subcommand.FORMAT,
     )
-    return result
 
 
 def run_ruff(
     settings: PluginSettings,
     document_path: str,
     document_source: str,
+    subcommand: Subcommand = Subcommand.CHECK,
     fix: bool = False,
     extra_arguments: Optional[List[str]] = None,
 ) -> str:
@@ -453,6 +482,8 @@ def run_ruff(
     document_source : str
         Document source or to apply ruff on.
         Needed when the source differs from the file source, e.g. during formatting.
+    subcommand: Subcommand
+        The ruff subcommand to run. Default = Subcommand.CHECK.
     fix : bool
         Whether to run fix or no-fix.
     extra_arguments : List[str]
@@ -463,7 +494,8 @@ def run_ruff(
     String containing the result in json format.
     """
     executable = settings.executable
-    arguments = build_arguments(document_path, settings, fix, extra_arguments)
+
+    arguments = subcommand.build_args(document_path, settings, fix, extra_arguments)
 
     if executable is not None:
         log.debug(f"Calling {executable} with args: {arguments} on '{document_path}'")
@@ -474,7 +506,7 @@ def run_ruff(
         except Exception:
             log.error(f"Can't execute ruff with given executable '{executable}'.")
     else:
-        cmd = [sys.executable, "-m", "ruff"]
+        cmd = [sys.executable, "-m", "ruff", str(subcommand)]
         cmd.extend(arguments)
         p = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
     (stdout, stderr) = p.communicate(document_source.encode())
@@ -485,14 +517,14 @@ def run_ruff(
     return stdout.decode()
 
 
-def build_arguments(
+def build_check_arguments(
     document_path: str,
     settings: PluginSettings,
     fix: bool = False,
     extra_arguments: Optional[List[str]] = None,
 ) -> List[str]:
     """
-    Build arguments for ruff.
+    Build arguments for ruff check.
 
     Parameters
     ----------
@@ -565,6 +597,51 @@ def build_arguments(
     return args
 
 
+def build_format_arguments(
+    document_path: str,
+    settings: PluginSettings,
+    extra_arguments: Optional[List[str]] = None,
+) -> List[str]:
+    """
+    Build arguments for ruff format.
+
+    Parameters
+    ----------
+    document : pylsp.workspace.Document
+        Document to apply ruff on.
+    settings : PluginSettings
+        Settings to use for arguments to pass to ruff.
+    extra_arguments : List[str]
+        Extra arguments to pass to ruff.
+
+    Returns
+    -------
+    List containing the arguments.
+    """
+    args = []
+    # Suppress update announcements
+    args.append("--quiet")
+
+    # Always force excludes
+    args.append("--force-exclude")
+    # Pass filename to ruff for per-file-ignores, catch unsaved
+    if document_path != "":
+        args.append(f"--stdin-filename={document_path}")
+
+    if settings.config:
+        args.append(f"--config={settings.config}")
+
+    if settings.exclude:
+        args.append(f"--exclude={','.join(settings.exclude)}")
+
+    if extra_arguments:
+        args.extend(extra_arguments)
+
+    args.extend(["--", "-"])
+
+    return args
+
+
 def load_settings(workspace: Workspace, document_path: str) -> PluginSettings:
     """
     Load settings from pyproject.toml file in the project path.

tests/test_code_actions.py

@@ -148,37 +148,3 @@ def f():
     settings = ruff_lint.load_settings(workspace, doc.path)
     fixed_str = ruff_lint.run_ruff_fix(doc, settings)
     assert fixed_str == expected_str_safe
-
-
-def test_format_document_default_settings(workspace):
-    _, doc = temp_document(import_str, workspace)
-    settings = ruff_lint.load_settings(workspace, doc.path)
-    formatted_str = ruff_lint.run_ruff_format(
-        settings, document_path=doc.path, document_source=doc.source
-    )
-    assert formatted_str == import_str
-
-
-def test_format_document_settings(workspace):
-    expected_str = dedent(
-        """
-        import os
-        import pathlib
-        """
-    )
-    workspace._config.update(
-        {
-            "plugins": {
-                "ruff": {
-                    "select": ["I"],
-                    "format": ["I001"],
-                }
-            }
-        }
-    )
-    _, doc = temp_document(import_str, workspace)
-    settings = ruff_lint.load_settings(workspace, doc.path)
-    formatted_str = ruff_lint.run_ruff_format(
-        settings, document_path=doc.path, document_source=doc.source
-    )
-    assert formatted_str == expected_str

tests/test_ruff_format.py

@@ -0,0 +1,93 @@
+import contextlib
+import tempfile
+import textwrap as tw
+from typing import Any, List, Mapping, Optional
+from unittest.mock import Mock
+
+import pytest
+from pylsp import uris
+from pylsp.config.config import Config
+from pylsp.workspace import Document, Workspace
+
+import pylsp_ruff.plugin as plugin
+
+
+@pytest.fixture()
+def workspace(tmp_path):
+    """Return a workspace."""
+    ws = Workspace(tmp_path.absolute().as_uri(), Mock())
+    ws._config = Config(ws.root_uri, {}, 0, {})
+    return ws
+
+
+def temp_document(doc_text, workspace):
+    with tempfile.NamedTemporaryFile(
+        mode="w", dir=workspace.root_path, delete=False
+    ) as temp_file:
+        name = temp_file.name
+        temp_file.write(doc_text)
+    doc = Document(uris.from_fs_path(name), workspace)
+    return name, doc
+
+
+def run_plugin_format(workspace: Workspace, doc: Document) -> str:
+    class TestResult:
+        result: Optional[List[Mapping[str, Any]]]
+
+        def __init__(self):
+            self.result = None
+
+        def get_result(self):
+            return self.result
+
+        def force_result(self, r):
+            self.result = r
+
+    generator = plugin.pylsp_format_document(workspace, doc)
+    result = TestResult()
+    with contextlib.suppress(StopIteration):
+        generator.send(None)
+        generator.send(result)
+
+    if result.result:
+        return result.result[0]["newText"]
+    return pytest.fail()
+
+
+def test_ruff_format(workspace):
+    # imports incorrectly ordered,
+    # body of foo has a line that's too long
+    # def bar() line missing whitespace above
+    txt = tw.dedent(
+        """
+        from thirdparty import x
+        import io
+        import asyncio
+
+        def foo():
+            print("this is a looooooooooooooooooooooooooooooooooooooooooooong line that should exceed the usual line-length limit which is normally eighty-eight columns")
+        def bar():
+            pass
+        """  # noqa: E501
+    ).lstrip()
+    want = tw.dedent(
+        """
+        import asyncio
+        import io
+
+        from thirdparty import x
+
+
+        def foo():
+            print(
+                "this is a looooooooooooooooooooooooooooooooooooooooooooong line that should exceed the usual line-length limit which is normally eighty-eight columns"
+            )
+
+
+        def bar():
+            pass
+        """  # noqa: E501
+    ).lstrip()
+    _, doc = temp_document(txt, workspace)
+    got = run_plugin_format(workspace, doc)
+    assert want == got, f"want:\n{want}\n\ngot:\n{got}"

tests/test_ruff_lint.py

@@ -178,6 +178,7 @@ def f():
         str(sys.executable),
         "-m",
         "ruff",
+        "check",
         "--quiet",
         "--exit-zero",
         "--output-format=json",