Add isort.check setting (#125)

* Add 'check' setting.

* Fix linting

* Apply suggestions from code review

bundled/tool/server.py

@@ -89,11 +89,16 @@ def did_close(params: lsp.DidCloseTextDocumentParams) -> None:
 
 
 def _linting_helper(document: workspace.Document) -> list[lsp.Diagnostic]:
+    # deep copy here to prevent accidentally updating global settings.
+    settings = copy.deepcopy(_get_settings_by_document(document))
+
+    if not settings.get("check", True):
+        # If sorting check is disabled, return empty diagnostics.
+        return []
+
     try:
         result = _run_tool_on_document(document, use_stdin=True, extra_args=["--check"])
         if result and result.stderr:
-            # deep copy here to prevent accidentally updating global settings.
-            settings = copy.deepcopy(_get_settings_by_document(document))
             return _parse_output(
                 document, result.stderr, severity=settings.get("severity", [])
             )

package.json

@@ -100,6 +100,15 @@
                     "scope": "resource",
                     "type": "array"
                 },
+                "isort.check": {
+                    "default": true,
+                    "description": "%settings.check.description%",
+                    "scope": "resource",
+                    "type": "boolean",
+                    "tags": [
+                        "experimental"
+                    ]
+                },
                 "isort.severity": {
                     "default": {
                         "W": "Warning",

package.nls.json

@@ -8,6 +8,7 @@
     "settings.logLevel.off.description": "Most logging is turned off, any information that is always logged might still be shown.",
     "settings.logLevel.warn.description": "Includes all messages in the error category and any additional warnings.",
     "settings.args.description": "Arguments passed in. Each argument is a separate string in the array.",
+    "settings.check.description": "Controls whether to run `isort` and report import sort issues.",
     "settings.severity.description": "Mapping from severity of `isort` message type to severity shown in problem window.",
     "settings.path.description": "When set to a path to `isort` binary, extension will use that for linting. NOTE: Using this option may slowdown linting.",
     "settings.importStrategy.description": "Defines where `isort` is imported from. This setting may be ignored if `isort.path` is set.",

src/common/settings.ts

@@ -7,6 +7,7 @@ import { LoggingLevelSettingType } from './log/types';
 import { getConfiguration, getWorkspaceFolders } from './vscodeapi';
 
 export interface ISettings {
+    check: boolean;
     workspace: string;
     logLevel: LoggingLevelSettingType;
     args: string[];
@@ -84,20 +85,22 @@ export async function getWorkspaceSettings(
     const args = getArgs(namespace, workspace).map((s) => resolveWorkspace(workspace, s));
     const path = getPath(namespace, workspace).map((s) => resolveWorkspace(workspace, s));
     const workspaceSetting = {
+        check: config.get<boolean>('check', true),
         workspace: workspace.uri.toString(),
-        logLevel: config.get<LoggingLevelSettingType>(`logLevel`) ?? 'error',
+        logLevel: config.get<LoggingLevelSettingType>('logLevel', 'error'),
         args,
-        severity: config.get<Record<string, string>>(`severity`) ?? {},
+        severity: config.get<Record<string, string>>('severity', {}),
         path,
         interpreter: (interpreter ?? []).map((s) => resolveWorkspace(workspace, s)),
-        importStrategy: config.get<string>(`importStrategy`) ?? 'fromEnvironment',
-        showNotifications: config.get<string>(`showNotifications`) ?? 'off',
+        importStrategy: config.get<string>('importStrategy', 'fromEnvironment'),
+        showNotifications: config.get<string>('showNotifications', 'off'),
     };
     return workspaceSetting;
 }
 
 export function checkIfConfigurationChanged(e: ConfigurationChangeEvent, namespace: string): boolean {
     const settings = [
+        `${namespace}.check`,
         `${namespace}.trace`,
         `${namespace}.args`,
         `${namespace}.severity`,

src/test/python_tests/test_path_specialization.py

@@ -27,10 +27,10 @@ def check_for_argv_duplication(self, argv):
 
 
 def test_path():
-    """Test linting using pylint bin path set."""
+    """Test linting using isort bin path set."""
 
     init_params = copy.deepcopy(defaults.VSCODE_DEFAULT_INITIALIZE)
-    init_params["initializationOptions"]["settings"][0]["path"] = ["pylint"]
+    init_params["initializationOptions"]["settings"][0]["path"] = ["isort"]
 
     argv_callback_object = CallbackObject()
     contents = TEST_FILE.read_text()

src/test/python_tests/test_sort.py

@@ -3,12 +3,14 @@
 """
 Test for formatting over LSP.
 """
+import copy
+import os
 from threading import Event
 
 import pytest
 from hamcrest import assert_that, is_
 
-from .lsp_test_client import constants, session, utils
+from .lsp_test_client import constants, defaults, session, utils
 
 FORMATTER = utils.get_server_info_defaults()
 TIMEOUT = 10  # 10 seconds
@@ -323,3 +325,43 @@ def _handler(params):
                     }
                 ),
             )
+
+
+def test_check_disabled():
+    """Test sort checking disabled."""
+    init_params = copy.deepcopy(defaults.VSCODE_DEFAULT_INITIALIZE)
+    init_params["initializationOptions"]["settings"][0]["check"] = False
+
+    UNFORMATTED_TEST_FILE_PATH = constants.TEST_DATA / "sample1" / "sample.unformatted"
+    uri = utils.as_uri(os.fspath(UNFORMATTED_TEST_FILE_PATH))
+
+    contents = UNFORMATTED_TEST_FILE_PATH.read_text()
+    actual_diagnostics = []
+
+    with session.LspSession() as ls_session:
+        ls_session.initialize(init_params)
+
+        done = Event()
+
+        def _handler(params):
+            nonlocal actual_diagnostics
+            actual_diagnostics = params
+            done.set()
+
+        ls_session.set_notification_callback(session.PUBLISH_DIAGNOSTICS, _handler)
+
+        ls_session.notify_did_open(
+            {
+                "textDocument": {
+                    "uri": uri,
+                    "languageId": "python",
+                    "version": 1,
+                    "text": contents,
+                }
+            }
+        )
+
+        # wait for some time to receive all notifications
+        done.wait(TIMEOUT)
+
+        assert_that(actual_diagnostics, is_({"uri": uri, "diagnostics": []}))