From 401a8f10c71d1508192d2a1b5c034e2e0bb5f93c Mon Sep 17 00:00:00 2001 From: Matt McShane Date: Tue, 7 Nov 2023 10:21:48 -0500 Subject: [PATCH 1/3] 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. --- pylsp_ruff/plugin.py | 107 +++++++++++++++++++++++++++++++------ tests/test_code_actions.py | 34 ------------ tests/test_ruff_format.py | 93 ++++++++++++++++++++++++++++++++ tests/test_ruff_lint.py | 1 + 4 files changed, 186 insertions(+), 49 deletions(-) create mode 100644 tests/test_ruff_format.py diff --git a/pylsp_ruff/plugin.py b/pylsp_ruff/plugin.py index 16a9c8f..8e2b79c 100644 --- a/pylsp_ruff/plugin.py +++ b/pylsp_ruff/plugin.py @@ -1,3 +1,4 @@ +import enum import json import logging import re @@ -45,6 +46,7 @@ r"(?::\s?(?P([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( @@ -399,6 +434,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) @@ -422,26 +458,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: @@ -457,6 +486,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] @@ -467,7 +498,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}'") @@ -478,7 +510,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()) @@ -489,14 +521,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 ---------- @@ -569,6 +601,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. diff --git a/tests/test_code_actions.py b/tests/test_code_actions.py index acfb160..f304e7a 100644 --- a/tests/test_code_actions.py +++ b/tests/test_code_actions.py @@ -150,37 +150,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 diff --git a/tests/test_ruff_format.py b/tests/test_ruff_format.py new file mode 100644 index 0000000..9b60fb3 --- /dev/null +++ b/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}" diff --git a/tests/test_ruff_lint.py b/tests/test_ruff_lint.py index 41026fb..ac60ccb 100644 --- a/tests/test_ruff_lint.py +++ b/tests/test_ruff_lint.py @@ -178,6 +178,7 @@ def f(): str(sys.executable), "-m", "ruff", + "check", "--quiet", "--exit-zero", "--output-format=json", From d1aabc2194317d24457142fbe1ce2a614e299e74 Mon Sep 17 00:00:00 2001 From: Matt McShane Date: Fri, 10 Nov 2023 11:54:41 -0500 Subject: [PATCH 2/3] Preserve compatibility with `format` settings Codes listed in this setting should be included in fixes performed as part of a formatting pass. --- pylsp_ruff/plugin.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pylsp_ruff/plugin.py b/pylsp_ruff/plugin.py index 8e2b79c..9624f93 100644 --- a/pylsp_ruff/plugin.py +++ b/pylsp_ruff/plugin.py @@ -5,7 +5,7 @@ import sys from pathlib import PurePath from subprocess import PIPE, Popen -from typing import Dict, Generator, List, Optional +from typing import Dict, Final, Generator, List, Optional if sys.version_info >= (3, 11): import tomllib @@ -62,7 +62,7 @@ "H": DiagnosticSeverity.Hint, } -ISORT_FIXES = "I" +ISORT_FIXES: Final = "I" class Subcommand(str, enum.Enum): @@ -130,7 +130,9 @@ 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 + settings.select = [ISORT_FIXES] + if settings.format: + settings.select.extend(settings.format) new_text = run_ruff( settings=settings, document_path=document.path, From d236f9d2f723792321e99a7b2c2d37d2cf7de1a5 Mon Sep 17 00:00:00 2001 From: Matt McShane Date: Mon, 13 Nov 2023 09:52:55 -0500 Subject: [PATCH 3/3] Make import sorting opt-in --- README.md | 2 +- pylsp_ruff/plugin.py | 21 +++++---- tests/test_ruff_format.py | 91 ++++++++++++++++++++++++--------------- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index ec7a5bc..80cd40a 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ the valid configuration keys: - `pylsp.plugins.ruff.perFileIgnores`: File-specific error codes to be ignored. - `pylsp.plugins.ruff.select`: List of error codes to enable. - `pylsp.plugins.ruff.extendSelect`: Same as select, but append to existing error codes. - - `pylsp.plugins.ruff.format`: List of error codes to fix during formatting. The default is `["I"]`, any additional codes are appended to this list. + - `pylsp.plugins.ruff.format`: List of error codes to fix during formatting. Empty by default, use `["I"]` here to get import sorting as part of formatting. - `pylsp.plugins.ruff.unsafeFixes`: boolean that enables/disables fixes that are marked "unsafe" by `ruff`. `false` by default. - `pylsp.plugins.ruff.severities`: Dictionary of custom severity levels for specific codes, see [below](#custom-severities). diff --git a/pylsp_ruff/plugin.py b/pylsp_ruff/plugin.py index 9624f93..8698a6d 100644 --- a/pylsp_ruff/plugin.py +++ b/pylsp_ruff/plugin.py @@ -5,7 +5,7 @@ import sys from pathlib import PurePath from subprocess import PIPE, Popen -from typing import Dict, Final, Generator, List, Optional +from typing import Dict, Generator, List, Optional if sys.version_info >= (3, 11): import tomllib @@ -62,8 +62,6 @@ "H": DiagnosticSeverity.Hint, } -ISORT_FIXES: Final = "I" - class Subcommand(str, enum.Enum): CHECK = "check" @@ -130,15 +128,16 @@ def pylsp_format_document(workspace: Workspace, document: Document) -> Generator settings=settings, document_path=document.path, document_source=source ) - settings.select = [ISORT_FIXES] if settings.format: - settings.select.extend(settings.format) - new_text = run_ruff( - settings=settings, - document_path=document.path, - document_source=new_text, - fix=True, - ) + # A second pass through the document with `ruff check` and only the rules + # enabled via the format config property. This allows for things like + # specifying `format = ["I"]` to get import sorting as part of formatting. + new_text = run_ruff( + settings=PluginSettings(ignore=["ALL"], select=settings.format), + document_path=document.path, + document_source=new_text, + fix=True, + ) # Avoid applying empty text edit if not new_text or new_text == source: diff --git a/tests/test_ruff_format.py b/tests/test_ruff_format.py index 9b60fb3..817d7be 100644 --- a/tests/test_ruff_format.py +++ b/tests/test_ruff_format.py @@ -11,6 +11,41 @@ import pylsp_ruff.plugin as plugin +_UNSORTED_IMPORTS = tw.dedent( + """ + from thirdparty import x + import io + import asyncio + """ +).strip() + +_SORTED_IMPORTS = tw.dedent( + """ + import asyncio + import io + + from thirdparty import x + """ +).strip() + +_UNFORMATTED_CODE = tw.dedent( + """ + def foo(): pass + def bar(): pass + """ +).strip() + +_FORMATTED_CODE = tw.dedent( + """ + def foo(): + pass + + + def bar(): + pass + """ +).strip() + @pytest.fixture() def workspace(tmp_path): @@ -54,40 +89,26 @@ def force_result(self, r): 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() +def test_ruff_format_only(workspace): + txt = f"{_UNSORTED_IMPORTS}\n{_UNFORMATTED_CODE}" + want = f"{_UNSORTED_IMPORTS}\n\n\n{_FORMATTED_CODE}\n" + _, doc = temp_document(txt, workspace) + got = run_plugin_format(workspace, doc) + assert want == got + + +def test_ruff_format_and_sort_imports(workspace): + txt = f"{_UNSORTED_IMPORTS}\n{_UNFORMATTED_CODE}" + want = f"{_SORTED_IMPORTS}\n\n\n{_FORMATTED_CODE}\n" _, doc = temp_document(txt, workspace) + workspace._config.update( + { + "plugins": { + "ruff": { + "format": ["I001"], + } + } + } + ) got = run_plugin_format(workspace, doc) - assert want == got, f"want:\n{want}\n\ngot:\n{got}" + assert want == got