From 2f9d6783573fef82c90d8b6784abb2d4cc724bb6 Mon Sep 17 00:00:00 2001 From: Jonas Bulik Date: Tue, 1 Aug 2023 12:05:39 +0200 Subject: [PATCH 01/10] add regex pattern matching to exclude documents from type checking --- pylsp_mypy/plugin.py | 9 +++++++++ test/test_plugin.py | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/pylsp_mypy/plugin.py b/pylsp_mypy/plugin.py index 6558db3..900a24f 100644 --- a/pylsp_mypy/plugin.py +++ b/pylsp_mypy/plugin.py @@ -218,6 +218,15 @@ def get_diagnostics( document.path, is_saved, ) + exclude = settings.get("exclude", []) + for pattern in exclude: + if re.match(pattern, document.path): + log.debug( + "Not running because %s matches exclude pattern %s", + document.path, + settings.get("exclude"), + ) + return [] live_mode = settings.get("live_mode", True) dmypy = settings.get("dmypy", False) diff --git a/test/test_plugin.py b/test/test_plugin.py index 6f9a3e6..237d8f5 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -328,3 +328,14 @@ def foo(): diag = diags[0] assert diag["message"] == DOC_ERR_MSG assert diag["code"] == "unreachable" + + +def test_exclude_path_match_mypy_not_run(tmpdir, workspace): + """When exclude is set in config then mypy should not run for that file.""" + doc = Document(DOC_URI, workspace, DOC_TYPE_ERR) + + plugin.pylsp_settings(workspace._config) + workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {"exclude": [doc.path]}}}}) + diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) + + assert not diags From cbe87e95c35cbc1e975218a647bafd1c54bf9047 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Fri, 6 Oct 2023 11:11:04 +0200 Subject: [PATCH 02/10] Move exclude code to pylsp_lint so mypy is not invoked unnecessarily --- pylsp_mypy/plugin.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/pylsp_mypy/plugin.py b/pylsp_mypy/plugin.py index 900a24f..817eea4 100644 --- a/pylsp_mypy/plugin.py +++ b/pylsp_mypy/plugin.py @@ -181,6 +181,18 @@ def pylsp_lint( didSettingsChange(workspace.root_path, settings) + # Running mypy with a single file (document) ignores any exclude pattern + # configured with mypy. We can now add our own exclude section like so: + # [tool.pylsp-mypy] + # exclude = ["tests/*"] + exclude = settings.get("exclude", []) + for pattern in exclude: + if re.match(pattern, document.path): + log.debug( + f"Not running because {document.path} matches " f"exclude pattern '{pattern}'" + ) + return [] + if settings.get("report_progress", False): with workspace.report_progress("lint: mypy"): return get_diagnostics(workspace, document, settings, is_saved) @@ -218,15 +230,6 @@ def get_diagnostics( document.path, is_saved, ) - exclude = settings.get("exclude", []) - for pattern in exclude: - if re.match(pattern, document.path): - log.debug( - "Not running because %s matches exclude pattern %s", - document.path, - settings.get("exclude"), - ) - return [] live_mode = settings.get("live_mode", True) dmypy = settings.get("dmypy", False) From 5888f7474b3ce7e5f13051075742edcc0c5ab720 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Sat, 21 Oct 2023 14:08:00 +0200 Subject: [PATCH 03/10] Use re.search so pattern is matched across the whole path --- pylsp_mypy/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylsp_mypy/plugin.py b/pylsp_mypy/plugin.py index 817eea4..4cfe23f 100644 --- a/pylsp_mypy/plugin.py +++ b/pylsp_mypy/plugin.py @@ -187,7 +187,7 @@ def pylsp_lint( # exclude = ["tests/*"] exclude = settings.get("exclude", []) for pattern in exclude: - if re.match(pattern, document.path): + if re.search(pattern, document.path): log.debug( f"Not running because {document.path} matches " f"exclude pattern '{pattern}'" ) From 22d4b724ed8b75044e2c89a6a893922fd43d7a91 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Sat, 21 Oct 2023 14:09:34 +0200 Subject: [PATCH 04/10] Encode patterns so patterns like "d:\\a\..." do work Especially useful when using Windows paths --- pylsp_mypy/plugin.py | 31 ++++++++++++++++++++++++------- test/test_plugin.py | 28 +++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/pylsp_mypy/plugin.py b/pylsp_mypy/plugin.py index 4cfe23f..7aa2d9f 100644 --- a/pylsp_mypy/plugin.py +++ b/pylsp_mypy/plugin.py @@ -142,6 +142,23 @@ def didSettingsChange(workspace: str, settings: Dict[str, Any]) -> None: settingsCache[workspace] = settings.copy() +def match_exclude_patterns(document_path: str, exclude_patterns: list) -> bool: + for pattern in exclude_patterns: + # This makes sure that \\ characters (and other unicode characters) are + # escaped first so the regex matcher will be able to parse them correctly. + # Especially useful for matching windows paths without any bad escape errors + pattern = pattern.encode("unicode-escape").decode() + + try: + if re.search(pattern, document_path): + log.debug(f"{document_path} matches " f"exclude pattern '{pattern}'") + return True + except re.error as e: + log.error(f"pattern {pattern} is not a valid regular expression: {e}") + + return False + + @hookimpl def pylsp_lint( config: Config, workspace: Workspace, document: Document, is_saved: bool @@ -185,13 +202,13 @@ def pylsp_lint( # configured with mypy. We can now add our own exclude section like so: # [tool.pylsp-mypy] # exclude = ["tests/*"] - exclude = settings.get("exclude", []) - for pattern in exclude: - if re.search(pattern, document.path): - log.debug( - f"Not running because {document.path} matches " f"exclude pattern '{pattern}'" - ) - return [] + exclude_patterns = settings.get("exclude", []) + + if match_exclude_patterns(document_path=document.path, exclude_patterns=exclude_patterns): + log.debug( + f"Not running because {document.path} matches " f"exclude patterns '{exclude_patterns}'" + ) + return [] if settings.get("report_progress", False): with workspace.report_progress("lint: mypy"): diff --git a/test/test_plugin.py b/test/test_plugin.py index 237d8f5..6a9515e 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -330,7 +330,33 @@ def foo(): assert diag["code"] == "unreachable" -def test_exclude_path_match_mypy_not_run(tmpdir, workspace): +@pytest.mark.parametrize( + "document_path,pattern,pattern_matched", + ( + ("/workspace/my-file.py", "/someting-else", False), + ("/workspace/my-file.py", "^/workspace$", False), + ("/workspace/my-file.py", "/workspace", True), + ("/workspace/my-file.py", "^/workspace(.*)$", True), + # This is a broken regex (missing ')'), but should not choke + ("/workspace/my-file.py", "/((workspace)", False), + # Windows paths are tricky with all those \\ and unintended escape, + # characters but they should 'just' work + ("d:\\a\\my-file.py", "\\a", True), + ( + "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", + "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", + True, + ), + ), +) +def test_match_exclude_patterns(document_path, pattern, pattern_matched): + assert ( + plugin.match_exclude_patterns(document_path=document_path, exclude_patterns=[pattern]) + is pattern_matched + ) + + +def test_config_exclude(tmpdir, workspace): """When exclude is set in config then mypy should not run for that file.""" doc = Document(DOC_URI, workspace, DOC_TYPE_ERR) From 7eb31baa306160084b38101326183a0181d95aa4 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Fri, 6 Oct 2023 11:12:39 +0200 Subject: [PATCH 05/10] Improve test so we are sure the exclude section has this side-effect --- test/test_plugin.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/test_plugin.py b/test/test_plugin.py index 6a9515e..f7439e8 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -361,7 +361,10 @@ def test_config_exclude(tmpdir, workspace): doc = Document(DOC_URI, workspace, DOC_TYPE_ERR) plugin.pylsp_settings(workspace._config) - workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {"exclude": [doc.path]}}}}) + workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {}}}}) diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) + assert diags[0]["message"] == TYPE_ERR_MSG - assert not diags + workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {"exclude": [doc.path]}}}}) + diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) + assert diags == [] From 00226fa4178faa3c413c52ce627de142cff6403a Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Fri, 6 Oct 2023 11:43:36 +0200 Subject: [PATCH 06/10] Document the exclude config option --- README.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 6e173a2..0e409c4 100644 --- a/README.rst +++ b/README.rst @@ -45,6 +45,9 @@ Configuration ``report_progress`` (default is ``False``) report basic progress to the LSP client. With this option, pylsp-mypy will report when mypy is running, given your editor supports LSP progress reporting. For small files this might produce annoying flashing in your editor, especially in with ``live_mode``. For large projects, enabling this can be helpful to assure yourself whether mypy is still running. +``exclude`` (default is ``[]``) A list of regular expressions which should be ignored. + The ``mypy`` runner wil not be invoked when a document path is matched by one of the expressions. Note that this differs from the ``exclude`` directive of a ``mypy`` config which is only used for recursively discovering files when mypy is invoked on a whole directory. + This project supports the use of ``pyproject.toml`` for configuration. It is in fact the preferred way. Using that your configuration could look like this: :: @@ -53,6 +56,7 @@ This project supports the use of ``pyproject.toml`` for configuration. It is in enabled = true live_mode = true strict = true + exclude = ["tests/*"] A ``pyproject.toml`` does not conflict with the legacy config file given that it does not contain a ``pylsp-mypy`` section. The following explanation uses the syntax of the legacy config file. However, all these options also apply to the ``pyproject.toml`` configuration (note the lowercase bools). Depending on your editor, the configuration (found in a file called pylsp-mypy.cfg in your workspace or a parent directory) should be roughly like this for a standard configuration: @@ -62,7 +66,8 @@ Depending on your editor, the configuration (found in a file called pylsp-mypy.c { "enabled": True, "live_mode": True, - "strict": False + "strict": False, + "exclude": ["tests/*"] } With ``dmypy`` enabled your config should look like this: From 363bd6b8b22226619f764ce9931b8e5d7179f401 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Wed, 15 Nov 2023 15:23:04 +0100 Subject: [PATCH 07/10] Make the match_exclude_patterns logic os independent Just convert windows paths to unix (a ilke) paths, so d:\My Documents\my_file.py becomes d:/My Documents/my_file.py Then you can reuse the configured exclude patterns for both windows and unix (a like) platforms. fds --- README.rst | 2 +- pylsp_mypy/plugin.py | 8 +++----- test/test_plugin.py | 34 +++++++++++++++++++--------------- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/README.rst b/README.rst index 0e409c4..d51ea0b 100644 --- a/README.rst +++ b/README.rst @@ -46,7 +46,7 @@ Configuration With this option, pylsp-mypy will report when mypy is running, given your editor supports LSP progress reporting. For small files this might produce annoying flashing in your editor, especially in with ``live_mode``. For large projects, enabling this can be helpful to assure yourself whether mypy is still running. ``exclude`` (default is ``[]``) A list of regular expressions which should be ignored. - The ``mypy`` runner wil not be invoked when a document path is matched by one of the expressions. Note that this differs from the ``exclude`` directive of a ``mypy`` config which is only used for recursively discovering files when mypy is invoked on a whole directory. + The ``mypy`` runner wil not be invoked when a document path is matched by one of the expressions. Note that this differs from the ``exclude`` directive of a ``mypy`` config which is only used for recursively discovering files when mypy is invoked on a whole directory. For both windows or unix platforms you should use forward slashes (``/``) to indicate paths. This project supports the use of ``pyproject.toml`` for configuration. It is in fact the preferred way. Using that your configuration could look like this: diff --git a/pylsp_mypy/plugin.py b/pylsp_mypy/plugin.py index 7aa2d9f..6642d30 100644 --- a/pylsp_mypy/plugin.py +++ b/pylsp_mypy/plugin.py @@ -143,12 +143,10 @@ def didSettingsChange(workspace: str, settings: Dict[str, Any]) -> None: def match_exclude_patterns(document_path: str, exclude_patterns: list) -> bool: - for pattern in exclude_patterns: - # This makes sure that \\ characters (and other unicode characters) are - # escaped first so the regex matcher will be able to parse them correctly. - # Especially useful for matching windows paths without any bad escape errors - pattern = pattern.encode("unicode-escape").decode() + """Check if the current document path matches any of the configures exlude patterns.""" + document_path = document_path.replace(os.sep, "/") + for pattern in exclude_patterns: try: if re.search(pattern, document_path): log.debug(f"{document_path} matches " f"exclude pattern '{pattern}'") diff --git a/test/test_plugin.py b/test/test_plugin.py index f7439e8..06d2432 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -4,7 +4,7 @@ import sys from pathlib import Path from typing import Dict -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest from mypy import api as mypy_api @@ -331,29 +331,31 @@ def foo(): @pytest.mark.parametrize( - "document_path,pattern,pattern_matched", + "document_path,pattern,os_sep,pattern_matched", ( - ("/workspace/my-file.py", "/someting-else", False), - ("/workspace/my-file.py", "^/workspace$", False), - ("/workspace/my-file.py", "/workspace", True), - ("/workspace/my-file.py", "^/workspace(.*)$", True), + ("/workspace/my-file.py", "/someting-else", "/", False), + ("/workspace/my-file.py", "^/workspace$", "/", False), + ("/workspace/my-file.py", "/workspace", "/", True), + ("/workspace/my-file.py", "^/workspace(.*)$", "/", True), # This is a broken regex (missing ')'), but should not choke - ("/workspace/my-file.py", "/((workspace)", False), + ("/workspace/my-file.py", "/((workspace)", "/", False), # Windows paths are tricky with all those \\ and unintended escape, # characters but they should 'just' work - ("d:\\a\\my-file.py", "\\a", True), + ("d:\\a\\my-file.py", "/a", "\\", True), ( "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", - "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", + "/a/pylsp-mypy/pylsp-mypy/test/test_plugin.py", + "\\", True, ), ), ) -def test_match_exclude_patterns(document_path, pattern, pattern_matched): - assert ( - plugin.match_exclude_patterns(document_path=document_path, exclude_patterns=[pattern]) - is pattern_matched - ) +def test_match_exclude_patterns(document_path, pattern, os_sep, pattern_matched): + with patch("os.sep", new=os_sep): + assert ( + plugin.match_exclude_patterns(document_path=document_path, exclude_patterns=[pattern]) + is pattern_matched + ) def test_config_exclude(tmpdir, workspace): @@ -365,6 +367,8 @@ def test_config_exclude(tmpdir, workspace): diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) assert diags[0]["message"] == TYPE_ERR_MSG - workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {"exclude": [doc.path]}}}}) + # Add the path of our document to the exclude patterns + exclude_path = doc.path.replace(os.sep, "/") + workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {"exclude": [exclude_path]}}}}) diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) assert diags == [] From d32b45285f5bcd8345128e7a1e311132dfa4dcc4 Mon Sep 17 00:00:00 2001 From: Martijn Jacobs Date: Wed, 15 Nov 2023 16:01:50 +0100 Subject: [PATCH 08/10] Update the TYPE_ERR_MSG to be compatible with mypy 1.7 --- test/test_plugin.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/test_plugin.py b/test/test_plugin.py index 06d2432..f0d878a 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -1,5 +1,6 @@ import collections import os +import re import subprocess import sys from pathlib import Path @@ -18,7 +19,10 @@ DOC_URI = f"file:/{Path(__file__)}" DOC_TYPE_ERR = """{}.append(3) """ -TYPE_ERR_MSG = '"Dict[, ]" has no attribute "append"' + +# Mypy 1.7 changed into "Never", so make this a regex to be compatible +# with multiple versions of mypy +TYPE_ERR_MSG_REGEX = r"\"Dict\[(.+)]\" has no attribute \"append\"" TEST_LINE = 'test_plugin.py:279:8:279:16: error: "Request" has no attribute "id" [attr-defined]' TEST_LINE_NOTE = ( @@ -66,7 +70,7 @@ def test_plugin(workspace, last_diagnostics_monkeypatch): assert len(diags) == 1 diag = diags[0] - assert diag["message"] == TYPE_ERR_MSG + assert re.search(TYPE_ERR_MSG_REGEX, diag["message"]) assert diag["range"]["start"] == {"line": 0, "character": 0} # Running mypy in 3.7 produces wrong error ends this can be removed when 3.7 reaches EOL if sys.version_info < (3, 8): @@ -365,7 +369,7 @@ def test_config_exclude(tmpdir, workspace): plugin.pylsp_settings(workspace._config) workspace.update_config({"pylsp": {"plugins": {"pylsp_mypy": {}}}}) diags = plugin.pylsp_lint(workspace._config, workspace, doc, is_saved=False) - assert diags[0]["message"] == TYPE_ERR_MSG + assert re.search(TYPE_ERR_MSG_REGEX, diags[0]["message"]) # Add the path of our document to the exclude patterns exclude_path = doc.path.replace(os.sep, "/") From 35670f15d55a6fa2c78daf2302959df3d2fea812 Mon Sep 17 00:00:00 2001 From: Richard Kellnberger Date: Wed, 15 Nov 2023 19:05:37 +0100 Subject: [PATCH 09/10] Explicit regex and path --- test/test_plugin.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/test_plugin.py b/test/test_plugin.py index f0d878a..b37b892 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -22,7 +22,9 @@ # Mypy 1.7 changed into "Never", so make this a regex to be compatible # with multiple versions of mypy -TYPE_ERR_MSG_REGEX = r"\"Dict\[(.+)]\" has no attribute \"append\"" +TYPE_ERR_MSG_REGEX = ( + r'"Dict\[(?:(?:)|(?:Never)), (?:(?:)|(?:Never))\]" has no attribute "append"' +) TEST_LINE = 'test_plugin.py:279:8:279:16: error: "Request" has no attribute "id" [attr-defined]' TEST_LINE_NOTE = ( @@ -70,7 +72,7 @@ def test_plugin(workspace, last_diagnostics_monkeypatch): assert len(diags) == 1 diag = diags[0] - assert re.search(TYPE_ERR_MSG_REGEX, diag["message"]) + assert re.fullmatch(TYPE_ERR_MSG_REGEX, diag["message"]) assert diag["range"]["start"] == {"line": 0, "character": 0} # Running mypy in 3.7 produces wrong error ends this can be removed when 3.7 reaches EOL if sys.version_info < (3, 8): @@ -345,11 +347,11 @@ def foo(): ("/workspace/my-file.py", "/((workspace)", "/", False), # Windows paths are tricky with all those \\ and unintended escape, # characters but they should 'just' work - ("d:\\a\\my-file.py", "/a", "\\", True), + (r"d:\\a\\my-file.py", "/a", r"\\", True), ( - "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", + r"d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", "/a/pylsp-mypy/pylsp-mypy/test/test_plugin.py", - "\\", + r"\\", True, ), ), From 604f1510eb1789da49eb5ec5c556eafe92ac2393 Mon Sep 17 00:00:00 2001 From: Richard Kellnberger Date: Wed, 15 Nov 2023 19:14:46 +0100 Subject: [PATCH 10/10] revert path --- test/test_plugin.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_plugin.py b/test/test_plugin.py index b37b892..9c08c8d 100644 --- a/test/test_plugin.py +++ b/test/test_plugin.py @@ -347,11 +347,11 @@ def foo(): ("/workspace/my-file.py", "/((workspace)", "/", False), # Windows paths are tricky with all those \\ and unintended escape, # characters but they should 'just' work - (r"d:\\a\\my-file.py", "/a", r"\\", True), + ("d:\\a\\my-file.py", "/a", "\\", True), ( - r"d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", + "d:\\a\\pylsp-mypy\\pylsp-mypy\\test\\test_plugin.py", "/a/pylsp-mypy/pylsp-mypy/test/test_plugin.py", - r"\\", + "\\", True, ), ),