Introduce preferDaemon option in order to use non-daemon mypy on pu…

…rpose (#143) * Introduce preferDaemon option * Add tests for preferDaemon option * Let isort use black profile * Update README.md according to @cwebster-99's suggestion Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com> * Update package.nls.json according to @cwebster-99's suggestion Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com> --------- Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com>
microsoft · Sep 12, 2023 · 1366475 · 1366475
1 parent faa32cb
commit 1366475
Show file tree

Hide file tree

Showing 7 changed files with 196 additions and 91 deletions.
diff --git a/README.md b/README.md
@@ -26,6 +26,7 @@ If you want to disable mypy, you can [disable this extension](https://code.visua
 | mypy-type-checker.importStrategy    | `useBundled`                                  | Setting to choose where to load `mypy` from. `useBundled` picks mypy bundled with the extension. `fromEnvironment` uses `mypy` available in the environment.                                                                                                      |
 | mypy-type-checker.showNotifications | `off`                                         | Setting to control when a notification is shown.                                                                                                                                                                                                                  |
 | mypy-type-checker.reportingScope    | `file`                                        | (experimental) Setting to control if problems are reported for files open in the editor (`file`) or for the entire workspace (`workspace`).                                                                                                                       |
+| mypy-type-checker.preferDaemon      | true                                          | (experimental) Setting to control how to invoke mypy. If true, dmypy is preferred over mypy; otherwise, mypy is preferred. Be aware, that the latter may slow down linting since it requires the `mypy` executable to be run whenever a file is saved or opened. Note that this setting will be overridden if `mypy-type-checker.path` is set. |
 
 ## Commands
 

diff --git a/bundled/tool/lsp_server.py b/bundled/tool/lsp_server.py
@@ -12,8 +12,12 @@
 import tempfile
 import traceback
 import uuid
+from dataclasses import dataclass
 from typing import Any, Dict, List, Optional, Sequence
 
+from packaging.version import Version
+from packaging.version import parse as parse_version
+
 
 # **********************************************************
 # Update sys.path before importing any bundled libraries.
@@ -73,8 +77,65 @@ def update_sys_path(path_to_add: str, strategy: str) -> None:
 # **********************************************************
 # Linting features start here
 # **********************************************************
-# Captures version of `mypy` in various workspaces.
-VERSION_TABLE: Dict[str, (int, int, int)] = {}
+
+
+@dataclass
+class MypyInfo:
+    version: Version
+    is_daemon: bool
+
+
+# Stores infomation of `mypy` executable in various workspaces.
+MYPY_INFO_TABLE: Dict[str, MypyInfo] = {}
+
+
+def get_mypy_info(settings: Dict[str, Any]) -> MypyInfo:
+    try:
+        code_workspace = settings["workspaceFS"]
+        if code_workspace not in MYPY_INFO_TABLE:
+            # This is text we get from running `mypy --version`
+            # mypy 1.0.0 (compiled: yes) <--- This is the version we want.
+            result = _run_unidentified_tool(["--version"], copy.deepcopy(settings))
+            log_to_output(
+                f"Version info for linter running for {code_workspace}:\r\n{result.stdout}"
+            )
+            first_line = result.stdout.splitlines(keepends=False)[0]
+            is_daemon = first_line.startswith("dmypy")
+            version_str = first_line.split(" ")[1]
+            version = parse_version(version_str)
+            MYPY_INFO_TABLE[code_workspace] = MypyInfo(version, is_daemon)
+        return MYPY_INFO_TABLE[code_workspace]
+    except:  # noqa: E722
+        log_to_output(
+            f"Error while checking mypy executable:\r\n{traceback.format_exc()}"
+        )
+
+
+def _run_unidentified_tool(
+    extra_args: Sequence[str], settings: Dict[str, Any]
+) -> utils.RunResult:
+    """Runs the tool given by the settings without knowing what it is.
+
+    This is supposed to be called only in `get_mypy_info`.
+    """
+    cwd = settings["cwd"]
+
+    if settings["path"]:
+        argv = settings["path"]
+    else:
+        argv = settings["interpreter"] or [sys.executable]
+        argv += ["-m", "mypy.dmypy" if settings["preferDaemon"] else "mypy"]
+
+    argv += extra_args
+    log_to_output(" ".join(argv))
+    log_to_output(f"CWD Server: {cwd}")
+
+    result = utils.run_path(argv=argv, cwd=cwd, env=_get_env_vars(settings))
+    if result.stderr:
+        log_to_output(result.stderr)
+
+    log_to_output(f"\r\n{result.stdout}\r\n")
+    return result
 
 
 @LSP_SERVER.feature(lsp.TEXT_DOCUMENT_DID_OPEN)
@@ -123,11 +184,9 @@ def _linting_helper(document: workspace.Document) -> None:
         # deep copy here to prevent accidentally updating global settings.
         settings = copy.deepcopy(_get_settings_by_document(document))
 
-        code_workspace = settings["workspaceFS"]
-        if VERSION_TABLE.get(code_workspace, None):
-            major, minor, _ = VERSION_TABLE[code_workspace]
-            if (major, minor) >= (0, 991) and sys.version_info >= (3, 8):
-                extra_args += ["--show-error-end"]
+        version = get_mypy_info(settings).version
+        if (version.major, version.minor) >= (0, 991) and sys.version_info >= (3, 8):
+            extra_args += ["--show-error-end"]
 
         result = _run_tool_on_document(document, extra_args=extra_args)
         if result and result.stdout:
@@ -336,67 +395,41 @@ def initialize(params: lsp.InitializeParams) -> None:
 @LSP_SERVER.feature(lsp.EXIT)
 def on_exit(_params: Optional[Any] = None) -> None:
     """Handle clean up on exit."""
-    for value in WORKSPACE_SETTINGS.values():
-        try:
-            settings = copy.deepcopy(value)
-            _run_tool([], settings, "kill")
-        except Exception:
-            pass
+    for settings in WORKSPACE_SETTINGS.values():
+        if get_mypy_info(settings).is_daemon:
+            try:
+                _run_dmypy_command([], copy.deepcopy(settings), "kill")
+            except Exception:
+                pass
 
 
 @LSP_SERVER.feature(lsp.SHUTDOWN)
 def on_shutdown(_params: Optional[Any] = None) -> None:
     """Handle clean up on shutdown."""
-    for value in WORKSPACE_SETTINGS.values():
-        try:
-            settings = copy.deepcopy(value)
-            _run_tool([], settings, "stop")
-        except Exception:
-            pass
+    for settings in WORKSPACE_SETTINGS.values():
+        if get_mypy_info(settings).is_daemon:
+            try:
+                _run_dmypy_command([], copy.deepcopy(settings), "stop")
+            except Exception:
+                pass
 
 
 def _log_version_info() -> None:
-    for value in WORKSPACE_SETTINGS.values():
-        try:
-            from packaging.version import parse as parse_version
-
-            settings = copy.deepcopy(value)
-            result = _run_tool(["--version"], settings, "version")
-            code_workspace = settings["workspaceFS"]
-            log_to_output(
-                f"Version info for linter running for {code_workspace}:\r\n{result.stdout}"
-            )
-
-            # This is text we get from running `mypy --version`
-            # mypy 1.0.0 (compiled: yes) <--- This is the version we want.
-            first_line = result.stdout.splitlines(keepends=False)[0]
-            actual_version = first_line.split(" ")[1]
-
-            # Update the key with a flag indicating `dmypy`
-            value["dmypy"] = first_line.startswith("dmypy")
-
-            version = parse_version(actual_version)
-            min_version = parse_version(MIN_VERSION)
-            VERSION_TABLE[code_workspace] = (
-                version.major,
-                version.minor,
-                version.micro,
+    for settings in WORKSPACE_SETTINGS.values():
+        code_workspace = settings["workspaceFS"]
+        actual_version = get_mypy_info(settings).version
+        min_version = parse_version(MIN_VERSION)
+
+        if actual_version < min_version:
+            log_error(
+                f"Version of linter running for {code_workspace} is NOT supported:\r\n"
+                f"SUPPORTED {TOOL_MODULE}>={min_version}\r\n"
+                f"FOUND {TOOL_MODULE}=={actual_version}\r\n"
             )
-
-            if version < min_version:
-                log_error(
-                    f"Version of linter running for {code_workspace} is NOT supported:\r\n"
-                    f"SUPPORTED {TOOL_MODULE}>={min_version}\r\n"
-                    f"FOUND {TOOL_MODULE}=={actual_version}\r\n"
-                )
-            else:
-                log_to_output(
-                    f"SUPPORTED {TOOL_MODULE}>={min_version}\r\n"
-                    f"FOUND {TOOL_MODULE}=={actual_version}\r\n"
-                )
-        except:  # noqa: E722
+        else:
             log_to_output(
-                f"Error while detecting mypy version:\r\n{traceback.format_exc()}"
+                f"SUPPORTED {TOOL_MODULE}>={min_version}\r\n"
+                f"FOUND {TOOL_MODULE}=={actual_version}\r\n"
             )
 
 
@@ -582,17 +615,11 @@ def _run_tool_on_document(
 
     if settings["path"]:
         argv = settings["path"]
-        if settings.get("dmypy"):
-            argv += _get_dmypy_args(settings, "run")
     else:
-        # Otherwise, we run mypy via dmypy.
-        if settings["interpreter"]:
-            argv = settings["interpreter"]
-        else:
-            argv = [sys.executable]
-        argv += ["-m", "mypy.dmypy"]
+        argv = settings["interpreter"] or [sys.executable]
+        argv += ["-m", "mypy.dmypy" if get_mypy_info(settings).is_daemon else "mypy"]
+    if get_mypy_info(settings).is_daemon:
         argv += _get_dmypy_args(settings, "run")
-
     argv += TOOL_ARGS + settings["args"] + extra_args
     if settings["reportingScope"] == "file":
         argv += [document.path]
@@ -613,36 +640,22 @@ def _run_tool_on_document(
     return result
 
 
-def _run_tool(
+def _run_dmypy_command(
     extra_args: Sequence[str], settings: Dict[str, Any], command: str
 ) -> utils.RunResult:
-    """Runs tool."""
+    if not get_mypy_info(settings).is_daemon:
+        log_error(f"dmypy command called in non-daemon context: {command}")
+        raise ValueError(f"dmypy command called in non-daemon context: {command}")
+
     cwd = settings["cwd"]
 
     if settings["path"]:
         argv = settings["path"]
-        if settings.get("dmypy"):
-            if command == "version":
-                # version check does not need dmypy command or
-                # status file arguments.
-                pass
-            else:
-                argv += _get_dmypy_args(settings, command)
     else:
-        # Otherwise, we run mypy via dmypy.
-        if settings["interpreter"]:
-            argv = settings["interpreter"]
-        else:
-            argv = [sys.executable]
-
+        argv = settings["interpreter"] or [sys.executable]
         argv += ["-m", "mypy.dmypy"]
-        if command == "version":
-            # version check does not need dmypy command or
-            # status file arguments.
-            pass
-        else:
-            argv += _get_dmypy_args(settings, command)
 
+    argv += _get_dmypy_args(settings, command)
     argv += extra_args
     log_to_output(" ".join(argv))
     log_to_output(f"CWD Server: {cwd}")

diff --git a/noxfile.py b/noxfile.py
@@ -160,9 +160,9 @@ def lint(session: nox.Session) -> None:
 
     # check import sorting using isort
     session.install("isort")
-    session.run("isort", "--check", "./bundled/tool")
-    session.run("isort", "--check", "./src/test/python_tests")
-    session.run("isort", "--check", "noxfile.py")
+    session.run("isort", "--check", "--profile", "black", "./bundled/tool")
+    session.run("isort", "--check", "--profile", "black", "./src/test/python_tests")
+    session.run("isort", "--check", "--profile", "black", "noxfile.py")
 
     # check typescript code
     session.run("npm", "run", "lint", external=True)

diff --git a/package.json b/package.json
@@ -106,6 +106,12 @@
                     },
                     "type": "array"
                 },
+                "mypy-type-checker.preferDaemon": {
+                    "default": true,
+                    "markdownDescription": "%settings.preferDaemon.description%",
+                    "scope": "resource",
+                    "type": "boolean"
+                },
                 "mypy-type-checker.reportingScope": {
                     "default": "file",
                     "markdownDescription": "%settings.reportingScope.description%",

diff --git a/package.nls.json b/package.nls.json
@@ -8,6 +8,7 @@
     "settings.importStrategy.useBundled.description": "Always use the bundled version of `mypy`.",
     "settings.importStrategy.fromEnvironment.description": "Use `mypy` from environment, fallback to bundled version only if `mypy` not available in the environment.",
     "settings.interpreter.description": "When set to a path to python executable, extension will use that to launch the server and any subprocess.",
+    "settings.preferDaemon.description": "When set to true, `dmypy` is preferred over `mypy`; otherwise, `mypy` is preferred. This setting will be overridden if `mypy-type-checker.path` is set. NOTE: Setting this option to false may slowdown linting.",
     "settings.reportingScope.description": "Defines the scope of the problems reported by the extension.",
     "settings.reportingScope.file.description": "Problems are reported for the files open in the editor only.",
     "settings.reportingScope.workspace.description": "Problems are reported for files in the workspace.",

diff --git a/src/common/settings.ts b/src/common/settings.ts
@@ -22,6 +22,7 @@ export interface ISettings {
     showNotifications: string;
     extraPaths: string[];
     reportingScope: string;
+    preferDaemon: boolean;
 }
 
 export function getExtensionSettings(namespace: string, includeInterpreter?: boolean): Promise<ISettings[]> {
@@ -143,6 +144,7 @@ export async function getWorkspaceSettings(
         showNotifications: config.get<string>('showNotifications', 'off'),
         extraPaths: resolveVariables(extraPaths, workspace),
         reportingScope: config.get<string>('reportingScope', 'file'),
+        preferDaemon: config.get<boolean>('preferDaemon', true),
     };
     return workspaceSetting;
 }
@@ -174,6 +176,7 @@ export async function getGlobalSettings(namespace: string, includeInterpreter?:
         showNotifications: getGlobalValue<string>(config, 'showNotifications', 'off'),
         extraPaths: getGlobalValue<string[]>(config, 'extraPaths', []),
         reportingScope: config.get<string>('reportingScope', 'file'),
+        preferDaemon: config.get<boolean>('preferDaemon', true),
     };
     return setting;
 }