Repository: python-lsp/python-lsp-server
Pull request #28 (2021-05-01 23:13:54)

Add support for flake8 per-file-ignores
This PR implements quick and dirty support for flake8's `per-file-ignores` option.

Changes to the `list`-type config parsing were necessary.  Specifically, the parsing for multiline config settings, e.g.
```config
[flake8]
ignores =
    E101,
    E102
```
needed to be altered to allow for multiple commas on a single line, because `per-file-ignores` takes a multiline list containing file names and comma separated lists of errors to be ignored:
```config
[flake8]
per-file-ignores =
    foo.py: E101, E102
    bar.py: E101
```

brandonwillard: What is this [`test_debounce` that's failing](https://github.com/python-lsp/python-lsp-server/pull/28/checks?check_run_id=2612748573#step:8:149)?
andfoy: That is a flaky test that is unrelated to this PR, I've just re ran the CIs
andfoy: Thanks @brandonwillard!

Patch:
diff --git a/pylsp/config/flake8_conf.py b/pylsp/config/flake8_conf.py
index e2c9009..eed9a31 100644
--- a/pylsp/config/flake8_conf.py
+++ b/pylsp/config/flake8_conf.py
@@ -28,6 +28,7 @@ OPTIONS = [
     ('ignore', 'plugins.flake8.ignore', list),
     ('max-line-length', 'plugins.flake8.maxLineLength', int),
     ('select', 'plugins.flake8.select', list),
+    ('per-file-ignores', 'plugins.flake8.perFileIgnores', list),
 ]
 
 
@@ -48,3 +49,9 @@ class Flake8Config(ConfigSource):
         files = find_parents(self.root_path, document_path, PROJECT_CONFIGS)
         config = self.read_config_from_files(files)
         return self.parse_config(config, CONFIG_KEY, OPTIONS)
+
+    @classmethod
+    def _parse_list_opt(cls, string):
+        if string.startswith("\n"):
+            return [s.strip().rstrip(",") for s in string.split("\n") if s.strip()]
+        return [s.strip() for s in string.split(",") if s.strip()]
diff --git a/pylsp/config/source.py b/pylsp/config/source.py
index 87fe715..6a21a84 100644
--- a/pylsp/config/source.py
+++ b/pylsp/config/source.py
@@ -27,8 +27,8 @@ class ConfigSource:
         """Return project-level (i.e. workspace directory) configuration."""
         raise NotImplementedError()
 
-    @staticmethod
-    def read_config_from_files(files):
+    @classmethod
+    def read_config_from_files(cls, files):
         config = configparser.RawConfigParser()
         for filename in files:
             if os.path.exists(filename) and not os.path.isdir(filename):
@@ -36,53 +36,53 @@ class ConfigSource:
 
         return config
 
-    @staticmethod
-    def parse_config(config, key, options):
+    @classmethod
+    def parse_config(cls, config, key, options):
         """Parse the config with the given options."""
         conf = {}
         for source, destination, opt_type in options:
-            opt_value = _get_opt(config, key, source, opt_type)
+            opt_value = cls._get_opt(config, key, source, opt_type)
             if opt_value is not None:
-                _set_opt(conf, destination, opt_value)
+                cls._set_opt(conf, destination, opt_value)
         return conf
 
+    @classmethod
+    def _get_opt(cls, config, key, option, opt_type):
+        """Get an option from a configparser with the given type."""
+        for opt_key in [option, option.replace('-', '_')]:
+            if not config.has_option(key, opt_key):
+                continue
 
-def _get_opt(config, key, option, opt_type):
-    """Get an option from a configparser with the given type."""
-    for opt_key in [option, option.replace('-', '_')]:
-        if not config.has_option(key, opt_key):
-            continue
+            if opt_type == bool:
+                return config.getboolean(key, opt_key)
 
-        if opt_type == bool:
-            return config.getboolean(key, opt_key)
+            if opt_type == int:
+                return config.getint(key, opt_key)
 
-        if opt_type == int:
-            return config.getint(key, opt_key)
+            if opt_type == str:
+                return config.get(key, opt_key)
 
-        if opt_type == str:
-            return config.get(key, opt_key)
+            if opt_type == list:
+                return cls._parse_list_opt(config.get(key, opt_key))
 
-        if opt_type == list:
-            return _parse_list_opt(config.get(key, opt_key))
+            raise ValueError("Unknown option type: %s" % opt_type)
 
-        raise ValueError("Unknown option type: %s" % opt_type)
+    @classmethod
+    def _parse_list_opt(cls, string):
+        return [s.strip() for s in string.split(",") if s.strip()]
 
+    @classmethod
+    def _set_opt(cls, config_dict, path, value):
+        """Set the value in the dictionary at the given path if the value is not None."""
+        if value is None:
+            return
 
-def _parse_list_opt(string):
-    return [s.strip() for s in string.split(",") if s.strip()]
+        if '.' not in path:
+            config_dict[path] = value
+            return
 
+        key, rest = path.split(".", 1)
+        if key not in config_dict:
+            config_dict[key] = {}
 
-def _set_opt(config_dict, path, value):
-    """Set the value in the dictionary at the given path if the value is not None."""
-    if value is None:
-        return
-
-    if '.' not in path:
-        config_dict[path] = value
-        return
-
-    key, rest = path.split(".", 1)
-    if key not in config_dict:
-        config_dict[key] = {}
-
-    _set_opt(config_dict[key], rest, value)
+        cls._set_opt(config_dict[key], rest, value)
diff --git a/pylsp/plugins/flake8_lint.py b/pylsp/plugins/flake8_lint.py
index 03504ef..7ac8c62 100644
--- a/pylsp/plugins/flake8_lint.py
+++ b/pylsp/plugins/flake8_lint.py
@@ -5,7 +5,9 @@
 import logging
 import os.path
 import re
-from subprocess import Popen, PIPE
+from pathlib import PurePath
+from subprocess import PIPE, Popen
+
 from pylsp import hookimpl, lsp
 
 log = logging.getLogger(__name__)
@@ -24,12 +26,21 @@ def pylsp_lint(workspace, document):
     settings = config.plugin_settings('flake8', document_path=document.path)
     log.debug("Got flake8 settings: %s", settings)
 
+    ignores = settings.get("ignore", [])
+    per_file_ignores = settings.get("perFileIgnores")
+
+    if per_file_ignores:
+        for path in per_file_ignores:
+            file_pat, errors = path.split(":")
+            if PurePath(document.path).match(file_pat):
+                ignores.extend(errors.split(","))
+
     opts = {
         'config': settings.get('config'),
         'exclude': settings.get('exclude'),
         'filename': settings.get('filename'),
         'hang-closing': settings.get('hangClosing'),
-        'ignore': settings.get('ignore'),
+        'ignore': ignores or None,
         'max-line-length': settings.get('maxLineLength'),
         'select': settings.get('select'),
     }


Test patch:
diff --git a/test/plugins/test_flake8_lint.py b/test/plugins/test_flake8_lint.py
index 4faf0dd..046127c 100644
--- a/test/plugins/test_flake8_lint.py
+++ b/test/plugins/test_flake8_lint.py
@@ -85,3 +85,77 @@ def test_flake8_executable_param(workspace):
 
         (call_args,) = popen_mock.call_args[0]
         assert flake8_executable in call_args
+
+
+def get_flake8_cfg_settings(workspace, config_str):
+    """Write a ``setup.cfg``, load it in the workspace, and return the flake8 settings.
+
+    This function creates a ``setup.cfg``; you'll have to delete it yourself.
+    """
+
+    with open(os.path.join(workspace.root_path, "setup.cfg"), "w+") as f:
+        f.write(config_str)
+
+    workspace.update_config({"pylsp": {"configurationSources": ["flake8"]}})
+
+    return workspace._config.plugin_settings("flake8")
+
+
+def test_flake8_multiline(workspace):
+    config_str = r"""[flake8]
+exclude =
+    blah/,
+    file_2.py
+    """
+
+    doc_str = "print('hi')\nimport os\n"
+
+    doc_uri = uris.from_fs_path(os.path.join(workspace.root_path, "blah/__init__.py"))
+    workspace.put_document(doc_uri, doc_str)
+
+    flake8_settings = get_flake8_cfg_settings(workspace, config_str)
+
+    assert "exclude" in flake8_settings
+    assert len(flake8_settings["exclude"]) == 2
+
+    with patch('pylsp.plugins.flake8_lint.Popen') as popen_mock:
+        mock_instance = popen_mock.return_value
+        mock_instance.communicate.return_value = [bytes(), bytes()]
+
+        doc = workspace.get_document(doc_uri)
+        flake8_lint.pylsp_lint(workspace, doc)
+
+    call_args = popen_mock.call_args[0][0]
+    assert call_args == ["flake8", "-", "--exclude=blah/,file_2.py"]
+
+    os.unlink(os.path.join(workspace.root_path, "setup.cfg"))
+
+
+def test_flake8_per_file_ignores(workspace):
+    config_str = r"""[flake8]
+ignores = F403
+per-file-ignores =
+    **/__init__.py:F401,E402
+    test_something.py:E402,
+exclude =
+    file_1.py
+    file_2.py
+    """
+
+    doc_str = "print('hi')\nimport os\n"
+
+    doc_uri = uris.from_fs_path(os.path.join(workspace.root_path, "blah/__init__.py"))
+    workspace.put_document(doc_uri, doc_str)
+
+    flake8_settings = get_flake8_cfg_settings(workspace, config_str)
+
+    assert "perFileIgnores" in flake8_settings
+    assert len(flake8_settings["perFileIgnores"]) == 2
+    assert "exclude" in flake8_settings
+    assert len(flake8_settings["exclude"]) == 2
+
+    doc = workspace.get_document(doc_uri)
+    res = flake8_lint.pylsp_lint(workspace, doc)
+    assert not res
+
+    os.unlink(os.path.join(workspace.root_path, "setup.cfg"))