From 9cf6c09caf5f30e5adc17676782d7ed4198b4378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Tue, 26 Jun 2018 10:35:02 +0200 Subject: [PATCH 1/3] mccabe: fix line numbers The Language Server Protocol specifies that line numbers start at 0, while mccabe starts at 1. Fix both the plugin and its test to match this. --- pyls/plugins/mccabe_lint.py | 4 ++-- test/plugins/test_mccabe_lint.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyls/plugins/mccabe_lint.py b/pyls/plugins/mccabe_lint.py index 46e3ee72..31fb39a9 100644 --- a/pyls/plugins/mccabe_lint.py +++ b/pyls/plugins/mccabe_lint.py @@ -30,8 +30,8 @@ def pyls_lint(config, document): diags.append({ 'source': 'mccabe', 'range': { - 'start': {'line': graph.lineno, 'character': graph.column}, - 'end': {'line': graph.lineno, 'character': len(document.lines[graph.lineno])}, + 'start': {'line': graph.lineno - 1, 'character': graph.column}, + 'end': {'line': graph.lineno - 1, 'character': len(document.lines[graph.lineno])}, }, 'message': 'Cyclomatic complexity too high: %s (threshold %s)' % (graph.complexity(), threshold), 'severity': lsp.DiagnosticSeverity.Warning diff --git a/test/plugins/test_mccabe_lint.py b/test/plugins/test_mccabe_lint.py index 9fa6f788..e5bc27f4 100644 --- a/test/plugins/test_mccabe_lint.py +++ b/test/plugins/test_mccabe_lint.py @@ -26,8 +26,8 @@ def test_mccabe(config): mod_import = [d for d in diags if d['message'] == msg][0] assert mod_import['severity'] == lsp.DiagnosticSeverity.Warning - assert mod_import['range']['start'] == {'line': 1, 'character': 0} - assert mod_import['range']['end'] == {'line': 1, 'character': 6} + assert mod_import['range']['start'] == {'line': 0, 'character': 0} + assert mod_import['range']['end'] == {'line': 0, 'character': 6} finally: config._settings = old_settings From 31e50e838f5f0c02bf6eff2e9b5f152061ddcf82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Wed, 11 Jul 2018 17:22:29 +0200 Subject: [PATCH 2/3] Differenciate linting between didSave and didChange Some linters like pylint are unable to handle in-memory documents. In order to use them, we need to be able to run lints differently on didChange and didSave events. --- pyls/hookspecs.py | 2 +- pyls/python_ls.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index d094dcb5..d1277cd6 100644 --- a/pyls/hookspecs.py +++ b/pyls/hookspecs.py @@ -88,7 +88,7 @@ def pyls_initialize(config, workspace): @hookspec -def pyls_lint(config, workspace, document): +def pyls_lint(config, workspace, document, on_change): pass diff --git a/pyls/python_ls.py b/pyls/python_ls.py index eda0e02f..2e9ea62e 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -194,10 +194,10 @@ def hover(self, doc_uri, position): return self._hook('pyls_hover', doc_uri, position=position) or {'contents': ''} @_utils.debounce(LINT_DEBOUNCE_S, keyed_by='doc_uri') - def lint(self, doc_uri): + def lint(self, doc_uri, on_change=True): # Since we're debounced, the document may no longer be open if doc_uri in self.workspace.documents: - self.workspace.publish_diagnostics(doc_uri, flatten(self._hook('pyls_lint', doc_uri))) + self.workspace.publish_diagnostics(doc_uri, flatten(self._hook('pyls_lint', doc_uri, on_change=on_change))) def references(self, doc_uri, position, exclude_declaration): return flatten(self._hook( @@ -217,7 +217,7 @@ def m_text_document__did_close(self, textDocument=None, **_kwargs): def m_text_document__did_open(self, textDocument=None, **_kwargs): self.workspace.put_document(textDocument['uri'], textDocument['text'], version=textDocument.get('version')) self._hook('pyls_document_did_open', textDocument['uri']) - self.lint(textDocument['uri']) + self.lint(textDocument['uri'], on_change=False) def m_text_document__did_change(self, contentChanges=None, textDocument=None, **_kwargs): for change in contentChanges: @@ -229,7 +229,7 @@ def m_text_document__did_change(self, contentChanges=None, textDocument=None, ** self.lint(textDocument['uri']) def m_text_document__did_save(self, textDocument=None, **_kwargs): - self.lint(textDocument['uri']) + self.lint(textDocument['uri'], on_change=False) def m_text_document__code_action(self, textDocument=None, range=None, context=None, **_kwargs): return self.code_actions(textDocument['uri'], range, context) From a20b9dfa15e8eff4384caa911873587271efb674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= Date: Tue, 26 Jun 2018 14:40:32 +0200 Subject: [PATCH 3/3] Add pylint plugin pylint is used in a big number of python projects, let's support it. --- README.rst | 2 + pyls/plugins/pylint_lint.py | 55 ++++++++++++++++++++++ setup.py | 3 ++ test/plugins/test_pylint_lint.py | 80 ++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+) create mode 100644 pyls/plugins/pylint_lint.py create mode 100644 test/plugins/test_pylint_lint.py diff --git a/README.rst b/README.rst index b7ff0174..a8cbcea6 100644 --- a/README.rst +++ b/README.rst @@ -29,6 +29,7 @@ If the respective dependencies are found, the following optional providers will * pydocstyle_ linter for docstring style checking * autopep8_ for code formatting * YAPF_ for code formatting (preferred over autopep8) +* pylint_ linter to detect various errors. Optional providers can be installed using the `extras` syntax. To install YAPF_ formatting for example: @@ -155,3 +156,4 @@ This project is made available under the MIT License. .. _pyls-isort: https://github.com/paradoxxxzero/pyls-isort .. _pyls-black: https://github.com/rupert/pyls-black .. _Black: https://github.com/ambv/black +.. _pylint: https://www.pylint.org/ diff --git a/pyls/plugins/pylint_lint.py b/pyls/plugins/pylint_lint.py new file mode 100644 index 00000000..d493bdd4 --- /dev/null +++ b/pyls/plugins/pylint_lint.py @@ -0,0 +1,55 @@ +# Copyright 2018 SUSE, Inc. +import os +import logging +import pylint.config +import pylint.lint +import pylint.reporters +from pyls import hookimpl, lsp + +log = logging.getLogger(__name__) + + +@hookimpl +def pyls_lint(config, document, on_change): + settings = config.plugin_settings('pylint') + log.debug("Got pylint settings: %s", settings) + + collector = DiagCollector() + if not on_change: + log.debug('Running pylint on \'%s\' in \'%s\'', document.path, os.getcwd()) + pylint.lint.Run(args=[document.path], reporter=collector, exit=False) + + return [map_diagnostic(diag, document.lines) for diag in collector.messages] + + +class DiagCollector(pylint.reporters.CollectingReporter): + + def display_reports(self, layout): + """do nothing""" + + def _display(self, layout): + """do nothing""" + + +def map_diagnostic(message, lines): + severity = lsp.DiagnosticSeverity.Warning + if message.category in ['fatal', 'error']: + severity = lsp.DiagnosticSeverity.Error + + # LSP lines start at 0, while pylint starts at 1 + err_range = { + 'start': {'line': message.line - 1, 'character': message.column}, + 'end': { + # FIXME: It's a little naive to mark until the end of the line, can we not easily do better? + 'line': message.line - 1, + 'character': len(lines[message.line - 1]) - 1 + }, + } + + return { + 'source': 'pylint', + 'range': err_range, + 'message': message.msg.split('\n')[0], + 'code': message.symbol, + 'severity': severity + } diff --git a/setup.py b/setup.py index 432c4615..3052a817 100755 --- a/setup.py +++ b/setup.py @@ -51,6 +51,7 @@ 'pycodestyle', 'pydocstyle>=2.0.0', 'pyflakes>=1.6.0', + 'pylint', 'rope>-0.10.5', 'yapf', ], @@ -59,6 +60,7 @@ 'pycodestyle': ['pycodestyle'], 'pydocstyle': ['pydocstyle>=2.0.0'], 'pyflakes': ['pyflakes>=1.6.0'], + 'pylint': ['pylint'], 'rope': ['rope>0.10.5'], 'yapf': ['yapf'], 'test': ['tox', 'versioneer', 'pytest', 'mock', 'pytest-cov', 'coverage'], @@ -85,6 +87,7 @@ 'pycodestyle = pyls.plugins.pycodestyle_lint', 'pydocstyle = pyls.plugins.pydocstyle_lint', 'pyflakes = pyls.plugins.pyflakes_lint', + 'pylint = pyls.plugins.pylint_lint', 'rope_completion = pyls.plugins.rope_completion', 'rope_rename = pyls.plugins.rope_rename', 'yapf = pyls.plugins.yapf_format', diff --git a/test/plugins/test_pylint_lint.py b/test/plugins/test_pylint_lint.py new file mode 100644 index 00000000..668124dd --- /dev/null +++ b/test/plugins/test_pylint_lint.py @@ -0,0 +1,80 @@ +# Copyright 2018 SUSE, Inc +import tempfile +import os +import pytest +from pyls import lsp, uris +from pyls.workspace import Document +from pyls.plugins import pylint_lint + +DOC_URI = uris.from_fs_path(__file__) +DOC = """import sys + +def hello(): +\tpass + +import json +""" + +DOC_SYNTAX_ERR = """def hello() + pass +""" + +DOC_UNDEFINED_NAME_ERR = "a = b\n" + + +@pytest.fixture +def make_document(): + created_files = [] + + def _make_document(content): + tmp = tempfile.NamedTemporaryFile(prefix='pylstest', mode='w', delete=False) + tmp.write(content) + tmp.close() + created_files.append(tmp.name) + return Document(uris.from_fs_path(tmp.name), content) + + yield _make_document + + for path in created_files: + os.remove(path) + + +def test_pylint(config, make_document): # pylint: disable=redefined-outer-name + doc = make_document(DOC) + diags = pylint_lint.pyls_lint(config, doc, on_change=False) + + # One we're expecting is: + msg = 'Unused import sys' + unused_import = [d for d in diags if d['message'] == msg][0] + + assert unused_import['range']['start'] == {'line': 0, 'character': 0} + assert unused_import['severity'] == lsp.DiagnosticSeverity.Warning + assert unused_import['code'] == 'unused-import' + + +def test_pylint_onchange(config, make_document): # pylint: disable=redefined-outer-name + doc = make_document(DOC) + diags = pylint_lint.pyls_lint(config, doc, on_change=True) + + assert diags == [] + + +def test_syntax_error_pylint(config, make_document): # pylint: disable=redefined-outer-name + doc = make_document(DOC_SYNTAX_ERR) + diag = pylint_lint.pyls_lint(config, doc, on_change=False)[0] + + assert diag['message'] == 'invalid syntax (, line 1)' + # sadly, pylint, always outputs column to 0 for these errors... + assert diag['range']['start'] == {'line': 0, 'character': 0} + assert diag['severity'] == lsp.DiagnosticSeverity.Error + assert diag['code'] == 'syntax-error' + + +def test_undefined_name_pylint(config, make_document): # pylint: disable=redefined-outer-name + doc = make_document(DOC_UNDEFINED_NAME_ERR) + diag = pylint_lint.pyls_lint(config, doc, on_change=False)[0] + + assert diag['message'] == 'Undefined variable \'b\'' + assert diag['range']['start'] == {'line': 0, 'character': 4} + assert diag['severity'] == lsp.DiagnosticSeverity.Error + assert diag['code'] == 'undefined-variable'