diff --git a/README.md b/README.md index c4d9883..5e8d265 100644 --- 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 index f0586b0..e9d8d78 100644 --- 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 index 48fb26d..032b214 100644 --- 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 index d5889c4..76f4866 100644 --- 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 index 41d4a5c..51410fa 100644 --- 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 index 4180cb2..13479f0 100644 --- 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 { @@ -143,6 +144,7 @@ export async function getWorkspaceSettings( showNotifications: config.get('showNotifications', 'off'), extraPaths: resolveVariables(extraPaths, workspace), reportingScope: config.get('reportingScope', 'file'), + preferDaemon: config.get('preferDaemon', true), }; return workspaceSetting; } @@ -174,6 +176,7 @@ export async function getGlobalSettings(namespace: string, includeInterpreter?: showNotifications: getGlobalValue(config, 'showNotifications', 'off'), extraPaths: getGlobalValue(config, 'extraPaths', []), reportingScope: config.get('reportingScope', 'file'), + preferDaemon: config.get('preferDaemon', true), }; return setting; } diff --git a/src/test/python_tests/test_non_daemon.py b/src/test/python_tests/test_non_daemon.py new file mode 100644 index 0000000..63de223 --- /dev/null +++ b/src/test/python_tests/test_non_daemon.py @@ -0,0 +1,81 @@ +import copy +from threading import Event +from typing import Any, Dict, List + +from hamcrest import ( + assert_that, + contains_string, + greater_than, + has_item, + has_length, + is_, + not_, +) + +from .lsp_test_client import constants, defaults, session, utils + +TEST_FILE_PATH = constants.TEST_DATA / "sample1" / "sample.py" +TEST_FILE_URI = utils.as_uri(str(TEST_FILE_PATH)) +TIMEOUT = 30 # 30 seconds + + +def test_daemon_non_daemon_equivalence_for_file_with_errors(): + """Test that the results of mypy and dmypy are the same for a file with errors.""" + TEST_FILE1_PATH = constants.TEST_DATA / "sample1" / "sample.py" + contents = TEST_FILE1_PATH.read_text(encoding="utf-8") + + def run(prefer_daemon: bool): + init_params = copy.deepcopy(defaults.VSCODE_DEFAULT_INITIALIZE) + init_params["initializationOptions"]["settings"][0][ + "preferDaemon" + ] = prefer_daemon + + result: Dict[str, Any] = {} + log_messages: List[str] = [] + with session.LspSession() as ls_session: + ls_session.initialize(init_params) + + done = Event() + + def _handler(params): + nonlocal result + result = params + done.set() + + def _log_handler(params: Dict[str, str]): + if params["type"] == 4: # == lsprotocol.types.MessageType.Log + log_messages.append(params["message"]) + + ls_session.set_notification_callback( + session.WINDOW_LOG_MESSAGE, _log_handler + ) + ls_session.set_notification_callback(session.PUBLISH_DIAGNOSTICS, _handler) + + ls_session.notify_did_open( + { + "textDocument": { + "uri": TEST_FILE_URI, + "languageId": "python", + "version": 1, + "text": contents, + } + } + ) + + # wait for some time to receive all notifications + assert done.wait(TIMEOUT), "Timed out waiting for diagnostics" + + return result, log_messages + + dmypy_result, dmypy_logs = run(True) + mypy_result, mypy_logs = run(False) + # Check the consistenty and the sanity of the two results + assert_that(dmypy_result, is_(mypy_result)) + assert_that(dmypy_result["uri"], is_(TEST_FILE_URI)) + assert_that(dmypy_result["diagnostics"], has_length(greater_than(0))) + + # Check that the two procedures were really different + assert_that(dmypy_logs, has_item(contains_string(" -m mypy.dmypy "))) + assert_that(dmypy_logs, not_(has_item(contains_string(" -m mypy ")))) + assert_that(mypy_logs, has_item(contains_string(" -m mypy "))) + assert_that(mypy_logs, not_(has_item(contains_string(" -m mypy.dmypy "))))