diff --git a/pyls/plugins/flake8_lint.py b/pyls/plugins/flake8_lint.py index 4f2e054e..45ad4224 100644 --- a/pyls/plugins/flake8_lint.py +++ b/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()) diff --git a/test/plugins/test_flake8_lint.py b/test/plugins/test_flake8_lint.py index 75adf4ea..f7b2d818 100644 --- a/test/plugins/test_flake8_lint.py +++ b/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' diff --git a/vscode-client/package.json b/vscode-client/package.json index f28437ca..9c87ba93 100644 --- a/vscode-client/package.json +++ b/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": [],