Skip to content

Commit d477839

Browse files
DanAlbertDanAlbert
committed
Fix #129: Add pylint support.
This also adds an `is_saved` argument to the pyls_lint hookspec, since pylint doesn't expose an API for operating on in-memory contents, only files.
1 parent dace6e6 commit d477839

File tree

5 files changed

+254
-8
lines changed

5 files changed

+254
-8
lines changed

‎pyls/hookspecs.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def pyls_initialize(config, workspace):
8888

8989

9090
@hookspec
91-
def pyls_lint(config, workspace, document):
91+
def pyls_lint(config, workspace, document, is_saved):
9292
pass
9393

9494

‎pyls/plugins/pylint_lint.py‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# Copyright 2018 Google LLC.
2+
"""Linter plugin for pylint."""
3+
import collections
4+
import json
5+
import sys
6+
7+
from pylint.epylint import py_run
8+
from pyls import hookimpl, lsp
9+
10+
11+
class PylintLinter(object):
12+
last_diags = collections.defaultdict(list)
13+
14+
@classmethod
15+
def lint(cls, document, is_saved, flags=''):
16+
"""Plugin interface to pyls linter.
17+
18+
Args:
19+
document: The document to be linted.
20+
is_saved: Whether or not the file has been saved to disk.
21+
flags: Additional flags to pass to pylint. Not exposed to
22+
pyls_lint, but used for testing.
23+
24+
Returns:
25+
A list of dicts with the following format:
26+
27+
{
28+
'source': 'pylint',
29+
'range': {
30+
'start': {
31+
'line': start_line,
32+
'character': start_column,
33+
},
34+
'end': {
35+
'line': end_line,
36+
'character': end_column,
37+
},
38+
}
39+
'message': msg,
40+
'severity': lsp.DiagnosticSeverity.*,
41+
}
42+
"""
43+
if not is_saved:
44+
# Pylint can only be run on files that have been saved to disk.
45+
# Rather than return nothing, return the previous list of
46+
# diagnostics. If we return an empty list, any diagnostics we'd
47+
# previously shown will be cleared until the next save. Instead,
48+
# continue showing (possibly stale) diagnostics until the next
49+
# save.
50+
return cls.last_diags[document.path]
51+
52+
# py_run will call shlex.split on its arguments, and shlex.split does
53+
# not handle Windows paths (it will try to perform escaping). Turn
54+
# backslashes into forward slashes first to avoid this issue.
55+
path = document.path
56+
if sys.platform.startswith('win'):
57+
path = path.replace('\\', '/')
58+
out, _err = py_run(
59+
'{} -f json {}'.format(path, flags), return_std=True
60+
)
61+
62+
# pylint prints nothing rather than [] when there are no diagnostics.
63+
# json.loads will not parse an empty string, so just return.
64+
json_str = out.getvalue()
65+
if not json_str.strip():
66+
cls.last_diags[document.path] = []
67+
return []
68+
69+
# Pylint's JSON output is a list of objects with the following format.
70+
#
71+
# {
72+
# "obj": "main",
73+
# "path": "foo.py",
74+
# "message": "Missing function docstring",
75+
# "message-id": "C0111",
76+
# "symbol": "missing-docstring",
77+
# "column": 0,
78+
# "type": "convention",
79+
# "line": 5,
80+
# "module": "foo"
81+
# }
82+
#
83+
# The type can be any of:
84+
#
85+
# * convention
86+
# * error
87+
# * fatal
88+
# * refactor
89+
# * warning
90+
diagnostics = []
91+
for diag in json.loads(json_str):
92+
# pylint lines index from 1, pyls lines index from 0
93+
line = diag['line'] - 1
94+
# But both index columns from 0
95+
col = diag['column']
96+
97+
# It's possible that we're linting an empty file. Even an empty
98+
# file might fail linting if it isn't named properly.
99+
end_col = len(document.lines[line]) if document.lines else 0
100+
101+
err_range = {
102+
'start': {
103+
'line': line,
104+
'character': col,
105+
},
106+
'end': {
107+
'line': line,
108+
'character': end_col,
109+
},
110+
}
111+
112+
if diag['type'] == 'convention':
113+
severity = lsp.DiagnosticSeverity.Information
114+
elif diag['type'] == 'error':
115+
severity = lsp.DiagnosticSeverity.Error
116+
elif diag['type'] == 'fatal':
117+
severity = lsp.DiagnosticSeverity.Error
118+
elif diag['type'] == 'refactor':
119+
severity = lsp.DiagnosticSeverity.Hint
120+
elif diag['type'] == 'warning':
121+
severity = lsp.DiagnosticSeverity.Warning
122+
123+
diagnostics.append({
124+
'source': 'pylint',
125+
'range': err_range,
126+
'message': '[{}] {}'.format(diag['symbol'], diag['message']),
127+
'severity': severity,
128+
'code': diag['message-id']
129+
})
130+
cls.last_diags[document.path] = diagnostics
131+
return diagnostics
132+
133+
134+
@hookimpl
135+
def pyls_lint(document, is_saved):
136+
return PylintLinter.lint(document, is_saved)

‎pyls/python_ls.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,13 @@ def hover(self, doc_uri, position):
215215
return self._hook('pyls_hover', doc_uri, position=position) or {'contents': ''}
216216

217217
@_utils.debounce(LINT_DEBOUNCE_S, keyed_by='doc_uri')
218-
def lint(self, doc_uri):
218+
def lint(self, doc_uri, is_saved):
219219
# Since we're debounced, the document may no longer be open
220220
if doc_uri in self.workspace.documents:
221-
self.workspace.publish_diagnostics(doc_uri, flatten(self._hook('pyls_lint', doc_uri)))
221+
self.workspace.publish_diagnostics(
222+
doc_uri,
223+
flatten(self._hook('pyls_lint', doc_uri, is_saved=is_saved))
224+
)
222225

223226
def references(self, doc_uri, position, exclude_declaration):
224227
return flatten(self._hook(
@@ -238,7 +241,7 @@ def m_text_document__did_close(self, textDocument=None, **_kwargs):
238241
def m_text_document__did_open(self, textDocument=None, **_kwargs):
239242
self.workspace.put_document(textDocument['uri'], textDocument['text'], version=textDocument.get('version'))
240243
self._hook('pyls_document_did_open', textDocument['uri'])
241-
self.lint(textDocument['uri'])
244+
self.lint(textDocument['uri'], is_saved=False)
242245

243246
def m_text_document__did_change(self, contentChanges=None, textDocument=None, **_kwargs):
244247
for change in contentChanges:
@@ -247,10 +250,10 @@ def m_text_document__did_change(self, contentChanges=None, textDocument=None, **
247250
change,
248251
version=textDocument.get('version')
249252
)
250-
self.lint(textDocument['uri'])
253+
self.lint(textDocument['uri'], is_saved=False)
251254

252255
def m_text_document__did_save(self, textDocument=None, **_kwargs):
253-
self.lint(textDocument['uri'])
256+
self.lint(textDocument['uri'], is_saved=True)
254257

255258
def m_text_document__code_action(self, textDocument=None, range=None, context=None, **_kwargs):
256259
return self.code_actions(textDocument['uri'], range, context)
@@ -294,7 +297,7 @@ def m_text_document__signature_help(self, textDocument=None, position=None, **_k
294297
def m_workspace__did_change_configuration(self, settings=None):
295298
self.config.update((settings or {}).get('pyls', {}))
296299
for doc_uri in self.workspace.documents:
297-
self.lint(doc_uri)
300+
self.lint(doc_uri, is_saved=False)
298301

299302
def m_workspace__did_change_watched_files(self, changes=None, **_kwargs):
300303
changed_py_files = set(d['uri'] for d in changes if d['uri'].endswith(PYTHON_FILE_EXTENSIONS))
@@ -306,7 +309,7 @@ def m_workspace__did_change_watched_files(self, changes=None, **_kwargs):
306309
for doc_uri in self.workspace.documents:
307310
# Changes in doc_uri are already handled by m_text_document__did_save
308311
if doc_uri not in changed_py_files:
309-
self.lint(doc_uri)
312+
self.lint(doc_uri, is_saved=False)
310313

311314
def m_workspace__execute_command(self, command=None, arguments=None):
312315
return self.execute_command(command, arguments)

‎setup.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
'pycodestyle',
5252
'pydocstyle>=2.0.0',
5353
'pyflakes>=1.6.0',
54+
'pylint',
5455
'rope>=0.10.5',
5556
'yapf',
5657
],
@@ -59,6 +60,7 @@
5960
'pycodestyle': ['pycodestyle'],
6061
'pydocstyle': ['pydocstyle>=2.0.0'],
6162
'pyflakes': ['pyflakes>=1.6.0'],
63+
'pylint': ['pylint'],
6264
'rope': ['rope>0.10.5'],
6365
'yapf': ['yapf'],
6466
'test': ['versioneer', 'pylint', 'pytest', 'mock', 'pytest-cov', 'coverage'],
@@ -85,6 +87,7 @@
8587
'pycodestyle = pyls.plugins.pycodestyle_lint',
8688
'pydocstyle = pyls.plugins.pydocstyle_lint',
8789
'pyflakes = pyls.plugins.pyflakes_lint',
90+
'pylint = pyls.plugins.pylint_lint',
8891
'rope_completion = pyls.plugins.rope_completion',
8992
'rope_rename = pyls.plugins.rope_rename',
9093
'yapf = pyls.plugins.yapf_format',

‎test/plugins/test_pylint_lint.py‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# Copyright 2018 Google LLC.
2+
import contextlib
3+
import os
4+
import tempfile
5+
6+
from pyls import lsp, uris
7+
from pyls.workspace import Document
8+
from pyls.plugins import pylint_lint
9+
10+
DOC_URI = uris.from_fs_path(__file__)
11+
DOC = """import sys
12+
13+
def hello():
14+
\tpass
15+
16+
import json
17+
"""
18+
19+
DOC_SYNTAX_ERR = """def hello()
20+
pass
21+
"""
22+
23+
24+
@contextlib.contextmanager
25+
def temp_document(doc_text):
26+
try:
27+
temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False)
28+
name = temp_file.name
29+
temp_file.write(doc_text)
30+
temp_file.close()
31+
yield Document(uris.from_fs_path(name))
32+
finally:
33+
os.remove(name)
34+
35+
36+
def write_temp_doc(document, contents):
37+
with open(document.path, 'w') as temp_file:
38+
temp_file.write(contents)
39+
40+
41+
def test_pylint():
42+
with temp_document(DOC) as doc:
43+
diags = pylint_lint.pyls_lint(doc, True)
44+
45+
msg = '[unused-import] Unused import sys'
46+
unused_import = [d for d in diags if d['message'] == msg][0]
47+
48+
assert unused_import['range']['start'] == {'line': 0, 'character': 0}
49+
assert unused_import['severity'] == lsp.DiagnosticSeverity.Warning
50+
51+
52+
def test_syntax_error_pylint():
53+
with temp_document(DOC_SYNTAX_ERR) as doc:
54+
diag = pylint_lint.pyls_lint(doc, True)[0]
55+
56+
assert diag['message'].startswith('[syntax-error] invalid syntax')
57+
# Pylint doesn't give column numbers for invalid syntax.
58+
assert diag['range']['start'] == {'line': 0, 'character': 0}
59+
assert diag['severity'] == lsp.DiagnosticSeverity.Error
60+
61+
62+
def test_lint_free_pylint():
63+
# Can't use temp_document because it might give us a file that doesn't
64+
# match pylint's naming requirements. We should be keeping this file clean
65+
# though, so it works for a test of an empty lint.
66+
assert not pylint_lint.pyls_lint(
67+
Document(uris.from_fs_path(__file__)), True)
68+
69+
70+
def test_lint_caching():
71+
# Pylint can only operate on files, not in-memory contents. We cache the
72+
# diagnostics after a run so we can continue displaying them until the file
73+
# is saved again.
74+
#
75+
# We use PylintLinter.lint directly here rather than pyls_lint so we can
76+
# pass --disable=invalid-name to pylint, since we want a temporary file but
77+
# need to ensure that pylint doesn't give us invalid-name when our temp
78+
# file has capital letters in its name.
79+
80+
flags = '--disable=invalid-name'
81+
with temp_document(DOC) as doc:
82+
# Start with a file with errors.
83+
diags = pylint_lint.PylintLinter.lint(doc, True, flags)
84+
assert diags
85+
86+
# Fix lint errors and write the changes to disk. Run the linter in the
87+
# in-memory mode to check the cached diagnostic behavior.
88+
write_temp_doc(doc, '')
89+
assert pylint_lint.PylintLinter.lint(doc, False, flags) == diags
90+
91+
# Now check the on-disk behavior.
92+
assert not pylint_lint.PylintLinter.lint(doc, True, flags)
93+
94+
# Make sure the cache was properly cleared.
95+
assert not pylint_lint.PylintLinter.lint(doc, False, flags)
96+
97+
98+
def test_per_file_caching():
99+
# Ensure that diagnostics are cached per-file.
100+
with temp_document(DOC) as doc:
101+
assert pylint_lint.pyls_lint(doc, True)
102+
103+
assert not pylint_lint.pyls_lint(
104+
Document(uris.from_fs_path(__file__)), False)

0 commit comments

Comments
 (0)