Fix flake8 per-file settings, the fallback interpreter, and the undocumented flake8 settings

pyls/plugins/flake8_lint.py

@@ -3,6 +3,7 @@
 import logging
 import os.path
 import re
+import sys
 from subprocess import Popen, PIPE
 from pyls import hookimpl, lsp
 
@@ -30,6 +31,12 @@ def pyls_lint(workspace, document):
         'ignore': settings.get('ignore'),
         'max-line-length': settings.get('maxLineLength'),
         'select': settings.get('select'),
+        # The document is piped over stdin so that unsaved changes are linted, but that
+        # leaves flake8 believing the file is called "stdin". Any setting it resolves per
+        # filename then cannot match, which silently disables per-file-ignores and
+        # exclude. Telling it the real name costs nothing and makes those work.
+        # Skipped when there is no path, such as an unsaved or non-file document.
+        'stdin-display-name': document.path or None,
     }
 
     # flake takes only absolute path to the config. So we should check and
@@ -68,10 +75,24 @@ def run_flake8(flake8_executable, args, document):
         cmd.extend(args)
         p = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
     except IOError:
-        log.debug("Can't execute %s. Trying with 'python -m flake8'", flake8_executable)
-        cmd = ['python', '-m', 'flake8']
+        # Fall back to the interpreter running this server, which is where
+        # `pip install python-language-server[flake8]` installs flake8. Bare "python" was
+        # not a safe fallback: it does not exist on a Python 3 only system, so the second
+        # call raised as well and the user saw a FileNotFoundError naming flake8 rather
+        # than anything they could act on.
+        log.debug("Can't execute %s. Trying with '%s -m flake8'", flake8_executable, sys.executable)
+        cmd = [sys.executable, '-m', 'flake8']
         cmd.extend(args)
-        p = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+        try:
+            p = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+        except IOError:
+            log.error(
+                "Could not run flake8: neither %s nor '%s -m flake8' could be executed. "
+                "Install flake8 in the environment running this server, or set "
+                "pyls.plugins.flake8.executable to its path.",
+                flake8_executable, sys.executable,
+            )
+            return ''
     (stdout, stderr) = p.communicate(document.source.encode())
     if stderr:
         log.error("Error while running flake8 '%s'", stderr.decode())

test/plugins/test_flake8_lint.py

@@ -1,4 +1,6 @@
 # Copyright 2019 Palantir Technologies, Inc.
+import logging
+import sys
 import tempfile
 import os
 from mock import patch
@@ -84,3 +86,87 @@ def test_flake8_executable_param(workspace):
 
         call_args = popen_mock.call_args.args[0]
         assert flake8_executable in call_args
+
+
+def test_flake8_passes_stdin_display_name(workspace):
+    """flake8 reads the document from stdin, so it has to be told the real filename.
+
+    Without it flake8 sees the file as "stdin" and any setting resolved per filename,
+    such as per-file-ignores or exclude, silently cannot match.
+    """
+    with patch('pyls.plugins.flake8_lint.Popen') as popen_mock:
+        mock_instance = popen_mock.return_value
+        mock_instance.communicate.return_value = [bytes(), bytes()]
+
+        name, doc = temp_document(DOC, workspace)
+        try:
+            flake8_lint.pyls_lint(workspace, doc)
+            call_args = popen_mock.call_args.args[0]
+            assert '--stdin-display-name={}'.format(doc.path) in call_args
+        finally:
+            os.remove(name)
+
+
+def test_flake8_omits_stdin_display_name_without_a_path(workspace):
+    """An unsaved document has no path, so there is no name to report."""
+    with patch('pyls.plugins.flake8_lint.Popen') as popen_mock:
+        mock_instance = popen_mock.return_value
+        mock_instance.communicate.return_value = [bytes(), bytes()]
+
+        doc = Document('', workspace, DOC)
+        flake8_lint.pyls_lint(workspace, doc)
+
+        call_args = popen_mock.call_args.args[0]
+        assert not [arg for arg in call_args if arg.startswith('--stdin-display-name')]
+
+
+def test_flake8_falls_back_to_the_running_interpreter(workspace):
+    """The fallback must not be a bare "python", which Python 3 only systems lack."""
+    with patch('pyls.plugins.flake8_lint.Popen') as popen_mock:
+        def fail_for_executable(cmd, **_kwargs):
+            if cmd[0] == 'flake8':
+                raise IOError('not found')
+            mock_instance = patch('pyls.plugins.flake8_lint.Popen').start()
+            mock_instance.communicate.return_value = [bytes(), bytes()]
+            return mock_instance
+
+        popen_mock.side_effect = fail_for_executable
+        _name, doc = temp_document(DOC, workspace)
+        flake8_lint.pyls_lint(workspace, doc)
+
+        fallback_cmd = popen_mock.call_args.args[0]
+        assert fallback_cmd[:3] == [sys.executable, '-m', 'flake8']
+        assert fallback_cmd[0] != 'python'
+
+
+def test_flake8_reports_when_it_cannot_run_at_all(workspace, caplog):
+    """Both attempts failing should log something actionable, not raise."""
+    with patch('pyls.plugins.flake8_lint.Popen', side_effect=IOError('not found')):
+        _name, doc = temp_document(DOC, workspace)
+        with caplog.at_level(logging.ERROR):
+            diags = flake8_lint.pyls_lint(workspace, doc)
+
+    assert not diags
+    errors = [r.getMessage() for r in caplog.records if r.levelno >= logging.ERROR]
+    assert len(errors) == 1
+    assert 'flake8.executable' in errors[0]
+
+
+def test_per_file_ignores_are_respected(workspace, tmpdir):
+    """End to end: the setting that this fix exists for.
+
+    Runs the real flake8 against a project whose config ignores a code for this file.
+    """
+    project = tmpdir.mkdir('proj')
+    project.join('setup.cfg').write(
+        '[flake8]\nper-file-ignores =\n    legacy.py: F841\n'
+    )
+    source_file = project.join('legacy.py')
+    source_file.write(DOC)
+
+    doc = Document(uris.from_fs_path(str(source_file)), workspace, DOC)
+    workspace._config.update({'plugins': {'flake8': {'config': str(project.join('setup.cfg'))}}})
+    diags = flake8_lint.pyls_lint(workspace, doc)
+
+    assert not [d for d in diags if d['code'] == 'F841'], \
+        'per-file-ignores should have suppressed F841 for legacy.py'

vscode-client/package.json

@@ -31,10 +31,67 @@
                     "description": "List of configuration sources to use.",
                     "items": {
                         "type": "string",
-                        "enum": ["pycodestyle", "pyflakes"]
+                        "enum": ["flake8", "pycodestyle"]
                     },
                     "uniqueItems": true
                 },
+                "pyls.plugins.flake8.enabled": {
+                    "type": "boolean",
+                    "default": false,
+                    "description": "Enable or disable the plugin."
+                },
+                "pyls.plugins.flake8.config": {
+                    "type": "string",
+                    "default": null,
+                    "description": "Path to the config file that will be the authoritative config source."
+                },
+                "pyls.plugins.flake8.exclude": {
+                    "type": "array",
+                    "default": null,
+                    "items": {
+                        "type": "string"
+                    },
+                    "uniqueItems": true,
+                    "description": "List of files or directories to exclude."
+                },
+                "pyls.plugins.flake8.executable": {
+                    "type": "string",
+                    "default": "flake8",
+                    "description": "Path to the flake8 executable."
+                },
+                "pyls.plugins.flake8.filename": {
+                    "type": "string",
+                    "default": null,
+                    "description": "Only check for filenames matching the patterns in this list."
+                },
+                "pyls.plugins.flake8.hangClosing": {
+                    "type": "boolean",
+                    "default": null,
+                    "description": "Hang closing bracket instead of matching indentation of opening bracket's line."
+                },
+                "pyls.plugins.flake8.ignore": {
+                    "type": "array",
+                    "default": null,
+                    "items": {
+                        "type": "string"
+                    },
+                    "uniqueItems": true,
+                    "description": "List of errors and warnings to ignore (or skip)."
+                },
+                "pyls.plugins.flake8.maxLineLength": {
+                    "type": "number",
+                    "default": null,
+                    "description": "Maximum allowed line length for the entirety of this run."
+                },
+                "pyls.plugins.flake8.select": {
+                    "type": "array",
+                    "default": null,
+                    "items": {
+                        "type": "string"
+                    },
+                    "uniqueItems": true,
+                    "description": "List of errors and warnings to enable."
+                },
                 "pyls.plugins.jedi.extra_paths": {
                     "type": "array",
                     "default": [],