From ca6dca3c57ec2cd6c9ac01ea4c3d616085bba9db Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Wed, 27 Jun 2018 15:35:30 -0700 Subject: [PATCH 01/13] 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. --- pyls/hookspecs.py | 2 +- pyls/plugins/pylint_lint.py | 140 +++++++++++++++++++++++++++++++ pyls/python_ls.py | 16 ++-- setup.py | 3 + test/plugins/test_pylint_lint.py | 104 +++++++++++++++++++++++ 5 files changed, 257 insertions(+), 8 deletions(-) create mode 100644 pyls/plugins/pylint_lint.py create mode 100644 test/plugins/test_pylint_lint.py diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index d094dcb5..880be1ff 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, is_saved): pass diff --git a/pyls/plugins/pylint_lint.py b/pyls/plugins/pylint_lint.py new file mode 100644 index 00000000..f836cd1a --- /dev/null +++ b/pyls/plugins/pylint_lint.py @@ -0,0 +1,140 @@ +# Copyright 2018 Google LLC. +"""Linter plugin for pylint.""" +import collections +import json +import logging +import sys + +from pylint.epylint import py_run +from pyls import hookimpl, lsp + + +log = logging.getLogger(__name__) + + +class PylintLinter(object): + last_diags = collections.defaultdict(list) + + @classmethod + def lint(cls, document, is_saved, flags=''): + """Plugin interface to pyls linter. + + Args: + document: The document to be linted. + is_saved: Whether or not the file has been saved to disk. + flags: Additional flags to pass to pylint. Not exposed to + pyls_lint, but used for testing. + + Returns: + A list of dicts with the following format: + + { + 'source': 'pylint', + 'range': { + 'start': { + 'line': start_line, + 'character': start_column, + }, + 'end': { + 'line': end_line, + 'character': end_column, + }, + } + 'message': msg, + 'severity': lsp.DiagnosticSeverity.*, + } + """ + log.debug('Running pylint on %s', document.path) + if not is_saved: + # Pylint can only be run on files that have been saved to disk. + # Rather than return nothing, return the previous list of + # diagnostics. If we return an empty list, any diagnostics we'd + # previously shown will be cleared until the next save. Instead, + # continue showing (possibly stale) diagnostics until the next + # save. + log.debug('Cannot run pylint, file has not been saved to disk.') + return cls.last_diags[document.path] + + # py_run will call shlex.split on its arguments, and shlex.split does + # not handle Windows paths (it will try to perform escaping). Turn + # backslashes into forward slashes first to avoid this issue. + path = document.path + if sys.platform.startswith('win'): + path = path.replace('\\', '/') + out, _err = py_run( + '{} -f json {}'.format(path, flags), return_std=True) + + # pylint prints nothing rather than [] when there are no diagnostics. + # json.loads will not parse an empty string, so just return. + json_str = out.getvalue() + if not json_str.strip(): + cls.last_diags[document.path] = [] + return [] + + # Pylint's JSON output is a list of objects with the following format. + # + # { + # "obj": "main", + # "path": "foo.py", + # "message": "Missing function docstring", + # "message-id": "C0111", + # "symbol": "missing-docstring", + # "column": 0, + # "type": "convention", + # "line": 5, + # "module": "foo" + # } + # + # The type can be any of: + # + # * convention + # * error + # * fatal + # * refactor + # * warning + diagnostics = [] + for diag in json.loads(json_str): + # pylint lines index from 1, pyls lines index from 0 + line = diag['line'] - 1 + # But both index columns from 0 + col = diag['column'] + + # It's possible that we're linting an empty file. Even an empty + # file might fail linting if it isn't named properly. + end_col = len(document.lines[line]) if document.lines else 0 + + err_range = { + 'start': { + 'line': line, + 'character': col, + }, + 'end': { + 'line': line, + 'character': end_col, + }, + } + + if diag['type'] == 'convention': + severity = lsp.DiagnosticSeverity.Information + elif diag['type'] == 'error': + severity = lsp.DiagnosticSeverity.Error + elif diag['type'] == 'fatal': + severity = lsp.DiagnosticSeverity.Error + elif diag['type'] == 'refactor': + severity = lsp.DiagnosticSeverity.Hint + elif diag['type'] == 'warning': + severity = lsp.DiagnosticSeverity.Warning + + diagnostics.append({ + 'source': 'pylint', + 'range': err_range, + 'message': '[{}] {}'.format(diag['symbol'], diag['message']), + 'severity': severity, + }) + cls.last_diags[document.path] = diagnostics + return diagnostics + + +@hookimpl +def pyls_lint(document, is_saved): + return PylintLinter.lint(document, is_saved) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index c9ff1e85..a5cae1b8 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -193,10 +193,12 @@ 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, is_saved): # 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, is_saved=is_saved))) def references(self, doc_uri, position, exclude_declaration): return flatten(self._hook( @@ -216,7 +218,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'], is_saved=True) def m_text_document__did_change(self, contentChanges=None, textDocument=None, **_kwargs): for change in contentChanges: @@ -225,10 +227,10 @@ def m_text_document__did_change(self, contentChanges=None, textDocument=None, ** change, version=textDocument.get('version') ) - self.lint(textDocument['uri']) + self.lint(textDocument['uri'], is_saved=False) def m_text_document__did_save(self, textDocument=None, **_kwargs): - self.lint(textDocument['uri']) + self.lint(textDocument['uri'], is_saved=True) def m_text_document__code_action(self, textDocument=None, range=None, context=None, **_kwargs): return self.code_actions(textDocument['uri'], range, context) @@ -272,12 +274,12 @@ def m_text_document__signature_help(self, textDocument=None, position=None, **_k def m_workspace__did_change_configuration(self, settings=None): self.config.update((settings or {}).get('pyls', {})) for doc_uri in self.workspace.documents: - self.lint(doc_uri) + self.lint(doc_uri, is_saved=False) def m_workspace__did_change_watched_files(self, **_kwargs): # Externally changed files may result in changed diagnostics for doc_uri in self.workspace.documents: - self.lint(doc_uri) + self.lint(doc_uri, is_saved=False) def m_workspace__execute_command(self, command=None, arguments=None): return self.execute_command(command, arguments) diff --git a/setup.py b/setup.py index a8657bb4..e8ac441b 100755 --- a/setup.py +++ b/setup.py @@ -50,6 +50,7 @@ 'pycodestyle', 'pydocstyle>=2.0.0', 'pyflakes>=1.6.0', + 'pylint', 'rope>-0.10.5', 'yapf', ], @@ -58,6 +59,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'], @@ -84,6 +86,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..90d40025 --- /dev/null +++ b/test/plugins/test_pylint_lint.py @@ -0,0 +1,104 @@ +# Copyright 2018 Google LLC. +import contextlib +import os +import tempfile + +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 +""" + + +@contextlib.contextmanager +def temp_document(doc_text): + try: + temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False) + name = temp_file.name + temp_file.write(doc_text) + temp_file.close() + yield Document(uris.from_fs_path(name)) + finally: + os.remove(name) + + +def write_temp_doc(document, contents): + with open(document.path, 'w') as temp_file: + temp_file.write(contents) + + +def test_pylint(): + with temp_document(DOC) as doc: + diags = pylint_lint.pyls_lint(doc, True) + + msg = '[unused-import] 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 + + +def test_syntax_error_pylint(): + with temp_document(DOC_SYNTAX_ERR) as doc: + diag = pylint_lint.pyls_lint(doc, True)[0] + + assert diag['message'].startswith('[syntax-error] invalid syntax') + # Pylint doesn't give column numbers for invalid syntax. + assert diag['range']['start'] == {'line': 0, 'character': 0} + assert diag['severity'] == lsp.DiagnosticSeverity.Error + + +def test_lint_free_pylint(): + # Can't use temp_document because it might give us a file that doesn't + # match pylint's naming requirements. We should be keeping this file clean + # though, so it works for a test of an empty lint. + assert not pylint_lint.pyls_lint( + Document(uris.from_fs_path(__file__)), True) + + +def test_lint_caching(): + # Pylint can only operate on files, not in-memory contents. We cache the + # diagnostics after a run so we can continue displaying them until the file + # is saved again. + # + # We use PylintLinter.lint directly here rather than pyls_lint so we can + # pass --disable=invalid-name to pylint, since we want a temporary file but + # need to ensure that pylint doesn't give us invalid-name when our temp + # file has capital letters in its name. + + flags = '--disable=invalid-name' + with temp_document(DOC) as doc: + # Start with a file with errors. + diags = pylint_lint.PylintLinter.lint(doc, True, flags) + assert diags + + # Fix lint errors and write the changes to disk. Run the linter in the + # in-memory mode to check the cached diagnostic behavior. + write_temp_doc(doc, '') + assert pylint_lint.PylintLinter.lint(doc, False, flags) == diags + + # Now check the on-disk behavior. + assert not pylint_lint.PylintLinter.lint(doc, True, flags) + + # Make sure the cache was properly cleared. + assert not pylint_lint.PylintLinter.lint(doc, False, flags) + + +def test_per_file_caching(): + # Ensure that diagnostics are cached per-file. + with temp_document(DOC) as doc: + assert pylint_lint.pyls_lint(doc, True) + + assert not pylint_lint.pyls_lint( + Document(uris.from_fs_path(__file__)), False) From 075c5da8bf595d3f8cf4bb3681c2b7c9ba2b2926 Mon Sep 17 00:00:00 2001 From: Andrew Christianson Date: Sun, 26 Aug 2018 10:05:21 -0700 Subject: [PATCH 02/13] Use .get_cached_default_environment() from jedi (#406) In some situations, pyls can call Document.sys_path _very_ frequently. That method calls jedi.api.environment.get_default_environment(), which can call jedi.api.environment.find_system_pythons(). On systems with pyenv and many python versions installed, this can be expensive, as jedi._compatibilty.which returns a shim path for python major version. Each found version spawns a subprocess to check the specific version of the python found at the path (via creation of a jedi Environment). However, if it's only found the pyenv shim, when a subprocess is spawned, the shim fails to find the version, and pyenv spends significant time looking for the requested command in other installed pythons. Calling .get_cached_default_environment insures jedi goes through the above process no more than once per 10 minutes per instance of pyls. Even on systems without pyenv, calling get_default_environment() directly results in at least one subprocess invocation, and a new jedi Environment object for each invocation. As Document.sys_path can be called as often as once per keypress (depending on the client, though this seems to be the case for lsp-mode) reducing work on that path provides noticeable improvement in performance. --- pyls/workspace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/workspace.py b/pyls/workspace.py index c731f670..eaebc12d 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -218,7 +218,7 @@ def sys_path(self): path = list(self._extra_sys_path) # TODO(gatesn): #339 - make better use of jedi environments, they seem pretty powerful - environment = jedi.api.environment.get_default_environment() + environment = jedi.api.environment.get_cached_default_environment() path.extend(environment.get_sys_path()) return path From 3b48404cb68bef6624685591b8668661225c760c Mon Sep 17 00:00:00 2001 From: Marius van Niekerk Date: Sun, 26 Aug 2018 10:06:00 -0700 Subject: [PATCH 03/13] Add license to source distribution (#396) --- MANIFEST.in | 1 + 1 file changed, 1 insertion(+) diff --git a/MANIFEST.in b/MANIFEST.in index 7bb58fa0..97d7d2ae 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,4 @@ include README.rst include versioneer.py include pyls/_version.py +include LICENSE From 29b63bedfd54cca0b3568c55269b18116ea3105e Mon Sep 17 00:00:00 2001 From: JiahuiJiang Date: Tue, 28 Aug 2018 15:24:51 -0400 Subject: [PATCH 04/13] Make python language server exit when parent process dies (#410) --- pyls/_utils.py | 18 ++++++++ pyls/config/config.py | 7 ++- pyls/python_ls.py | 17 +++++++- test/fixtures.py | 2 +- test/plugins/test_pycodestyle_lint.py | 2 +- test/test_language_server.py | 62 +++++++++++++++++++-------- 6 files changed, 85 insertions(+), 23 deletions(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index 25cf889d..d4f12924 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -113,3 +113,21 @@ def clip_column(column, lines, line_number): # https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#position max_column = len(lines[line_number].rstrip('\r\n')) if len(lines) > line_number else 0 return min(column, max_column) + + +def is_process_alive(pid): + """ Check whether the process with the given pid is still alive. + + Args: + pid (int): process ID + + Returns: + bool: False if the process is not alive or don't have permission to check, True otherwise. + """ + try: + os.kill(pid, 0) + except OSError: + # no such process or process is already dead + return False + else: + return True diff --git a/pyls/config/config.py b/pyls/config/config.py index 9d0a2ccc..9e06f6ec 100644 --- a/pyls/config/config.py +++ b/pyls/config/config.py @@ -14,10 +14,11 @@ class Config(object): - def __init__(self, root_uri, init_opts): + def __init__(self, root_uri, init_opts, process_id): self._root_path = uris.to_fs_path(root_uri) self._root_uri = root_uri self._init_opts = init_opts + self._process_id = process_id self._settings = {} self._plugin_settings = {} @@ -77,6 +78,10 @@ def init_opts(self): def root_uri(self): return self._root_uri + @property + def process_id(self): + return self._process_id + def settings(self, document_path=None): """Settings are constructed from a few sources: diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 634a5507..0647332d 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,6 +1,7 @@ # Copyright 2017 Palantir Technologies, Inc. import logging import socketserver +import threading from jsonrpc.dispatchers import MethodDispatcher from jsonrpc.endpoint import Endpoint @@ -14,6 +15,7 @@ LINT_DEBOUNCE_S = 0.5 # 500 ms +PARENT_PROCESS_WATCH_INTERVAL = 10 # 10 s class _StreamHandlerWrapper(socketserver.StreamRequestHandler, object): @@ -149,10 +151,23 @@ def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializati rootUri = uris.from_fs_path(rootPath) if rootPath is not None else '' self.workspace = Workspace(rootUri, self._endpoint) - self.config = config.Config(rootUri, initializationOptions or {}) + self.config = config.Config(rootUri, initializationOptions or {}, processId) self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') + if processId is not None: + def watch_parent_process(pid): + # exist when the given pid is not alive + if not _utils.is_process_alive(pid): + log.info("parent process %s is not alive", pid) + self.m_exit() + log.debug("parent process %s is still alive", pid) + threading.Timer(PARENT_PROCESS_WATCH_INTERVAL, watch_parent_process(pid)).start() + + watching_thread = threading.Thread(target=watch_parent_process, args=[processId]) + watching_thread.daemon = True + watching_thread.start() + # Get our capabilities return {'capabilities': self.capabilities()} diff --git a/test/fixtures.py b/test/fixtures.py index d4b53d3c..4afc9295 100644 --- a/test/fixtures.py +++ b/test/fixtures.py @@ -44,7 +44,7 @@ def workspace(tmpdir): @pytest.fixture def config(workspace): # pylint: disable=redefined-outer-name """Return a config object.""" - return Config(workspace.root_uri, {}) + return Config(workspace.root_uri, {}, 0) @pytest.fixture diff --git a/test/plugins/test_pycodestyle_lint.py b/test/plugins/test_pycodestyle_lint.py index 583da797..698704fc 100644 --- a/test/plugins/test_pycodestyle_lint.py +++ b/test/plugins/test_pycodestyle_lint.py @@ -67,7 +67,7 @@ def test_pycodestyle_config(workspace): doc_uri = uris.from_fs_path(os.path.join(workspace.root_path, 'test.py')) workspace.put_document(doc_uri, DOC) doc = workspace.get_document(doc_uri) - config = Config(workspace.root_uri, {}) + config = Config(workspace.root_uri, {}, 1234) # Make sure we get a warning for 'indentation contains tabs' diags = pycodestyle_lint.pyls_lint(config, doc) diff --git a/test/test_language_server.py b/test/test_language_server.py index 7597feb6..65f5b357 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -14,42 +14,66 @@ def start_client(client): client.start() +class _ClientServer(object): + """ A class to setup a client/server pair """ + def __init__(self): + # Client to Server pipe + csr, csw = os.pipe() + # Server to client pipe + scr, scw = os.pipe() + + self.server_thread = Thread(target=start_io_lang_server, args=( + os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), PythonLanguageServer + )) + self.server_thread.daemon = True + self.server_thread.start() + + self.client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) + self.client_thread = Thread(target=start_client, args=[self.client]) + self.client_thread.daemon = True + self.client_thread.start() + + @pytest.fixture def client_server(): - """ A fixture to setup a client/server """ + """ A fixture that sets up a client/server pair and shuts down the server """ + client_server_pair = _ClientServer() - # Client to Server pipe - csr, csw = os.pipe() - # Server to client pipe - scr, scw = os.pipe() + yield client_server_pair.client + + shutdown_response = client_server_pair.client._endpoint.request('shutdown').result(timeout=CALL_TIMEOUT) + assert shutdown_response is None + client_server_pair.client._endpoint.notify('exit') - server_thread = Thread(target=start_io_lang_server, args=( - os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), PythonLanguageServer - )) - server_thread.daemon = True - server_thread.start() - client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) - client_thread = Thread(target=start_client, args=[client]) - client_thread.daemon = True - client_thread.start() +@pytest.fixture +def client_exited_server(): + """ A fixture that sets up a client/server pair and assert the server has already exited """ + client_server_pair = _ClientServer() - yield client + yield client_server_pair.client - shutdown_response = client._endpoint.request('shutdown').result(timeout=CALL_TIMEOUT) - assert shutdown_response is None - client._endpoint.notify('exit') + assert client_server_pair.server_thread.is_alive() is False def test_initialize(client_server): # pylint: disable=redefined-outer-name response = client_server._endpoint.request('initialize', { - 'processId': 1234, 'rootPath': os.path.dirname(__file__), 'initializationOptions': {} }).result(timeout=CALL_TIMEOUT) assert 'capabilities' in response +def test_exit_with_parent_process_died(client_exited_server): # pylint: disable=redefined-outer-name + # language server should have already exited before responding + with pytest.raises(Exception): + client_exited_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + + def test_missing_message(client_server): # pylint: disable=redefined-outer-name with pytest.raises(JsonRpcMethodNotFound): client_server._endpoint.request('unknown_method').result(timeout=CALL_TIMEOUT) From 9f48de1cc799323ad7adf6cf0ebcb1821324ca52 Mon Sep 17 00:00:00 2001 From: JiahuiJiang Date: Wed, 29 Aug 2018 15:59:58 -0400 Subject: [PATCH 05/13] Fix package.json (#412) --- vscode-client/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vscode-client/package.json b/vscode-client/package.json index dc00fecf..29d60061 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -218,7 +218,7 @@ "pyls.rope.extensionModules": { "type": "string", "default": null, - "description": "The name of the folder in which rope stores project configurations and data. Pass `null` for not using such a folder at all." + "description": "Builtin and c-extension modules that are allowed to be imported and inspected by rope." }, "pyls.rope.ropeFolder": { "type": "array", @@ -227,7 +227,7 @@ "type": "string" }, "uniqueItems": true, - "description": "Builtin and c-extension modules that are allowed to be imported and inspected by rope." + "description": "The name of the folder in which rope stores project configurations and data. Pass `null` for not using such a folder at all." } } } From 0239ebbf6b441f47243e56d90a38fe2ae7854bae Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Thu, 6 Sep 2018 14:28:47 +0100 Subject: [PATCH 06/13] Increase max workers to 64 (#415) --- pyls/python_ls.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 0647332d..238cffc2 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -16,6 +16,7 @@ LINT_DEBOUNCE_S = 0.5 # 500 ms PARENT_PROCESS_WATCH_INTERVAL = 10 # 10 s +MAX_WORKERS = 64 class _StreamHandlerWrapper(socketserver.StreamRequestHandler, object): @@ -73,7 +74,7 @@ def __init__(self, rx, tx): self._jsonrpc_stream_reader = JsonRpcStreamReader(rx) self._jsonrpc_stream_writer = JsonRpcStreamWriter(tx) - self._endpoint = Endpoint(self, self._jsonrpc_stream_writer.write) + self._endpoint = Endpoint(self, self._jsonrpc_stream_writer.write, max_workers=MAX_WORKERS) self._dispatchers = [] self._shutdown = False From db42da669587a9464d78d5d2ed3380f8130dbaed Mon Sep 17 00:00:00 2001 From: JiahuiJiang Date: Thu, 6 Sep 2018 14:08:11 -0400 Subject: [PATCH 07/13] Fix parent process watcher thread (#416) * Fix parent process watcher thread * timer --- pyls/python_ls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 238cffc2..b559de17 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -163,9 +163,9 @@ def watch_parent_process(pid): log.info("parent process %s is not alive", pid) self.m_exit() log.debug("parent process %s is still alive", pid) - threading.Timer(PARENT_PROCESS_WATCH_INTERVAL, watch_parent_process(pid)).start() + threading.Timer(PARENT_PROCESS_WATCH_INTERVAL, watch_parent_process, args=[pid]).start() - watching_thread = threading.Thread(target=watch_parent_process, args=[processId]) + watching_thread = threading.Thread(target=watch_parent_process, args=(processId,)) watching_thread.daemon = True watching_thread.start() From 2d5fc51caa360472000114d258d7a979fc049acc Mon Sep 17 00:00:00 2001 From: Alexey Evseev Date: Wed, 19 Sep 2018 13:27:46 -0700 Subject: [PATCH 08/13] Respect option `follow_imports` in `goto_assignments` call (#404) --- pyls/plugins/definition.py | 5 +++-- test/plugins/test_definitions.py | 12 ++++++------ vscode-client/package.json | 10 ++++++++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pyls/plugins/definition.py b/pyls/plugins/definition.py index fbcd5ffb..7df80a19 100644 --- a/pyls/plugins/definition.py +++ b/pyls/plugins/definition.py @@ -6,8 +6,9 @@ @hookimpl -def pyls_definitions(document, position): - definitions = document.jedi_script(position).goto_assignments() +def pyls_definitions(config, document, position): + params = {k: v for k, v in config.plugin_settings('jedi_definition').items() if v is not None} + definitions = document.jedi_script(position).goto_assignments(**params) definitions = [ d for d in definitions diff --git a/test/plugins/test_definitions.py b/test/plugins/test_definitions.py index c360ac44..97408894 100644 --- a/test/plugins/test_definitions.py +++ b/test/plugins/test_definitions.py @@ -19,7 +19,7 @@ def add_member(self, id, name): """ -def test_definitions(): +def test_definitions(config): # Over 'a' in print a cursor_pos = {'line': 3, 'character': 6} @@ -30,19 +30,19 @@ def test_definitions(): } doc = Document(DOC_URI, DOC) - assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(doc, cursor_pos) + assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(config, doc, cursor_pos) -def test_builtin_definition(): +def test_builtin_definition(config): # Over 'i' in dict cursor_pos = {'line': 8, 'character': 24} # No go-to def for builtins doc = Document(DOC_URI, DOC) - assert [] == pyls_definitions(doc, cursor_pos) + assert [] == pyls_definitions(config, doc, cursor_pos) -def test_assignment(): +def test_assignment(config): # Over 's' in self.members[id] cursor_pos = {'line': 11, 'character': 19} @@ -53,4 +53,4 @@ def test_assignment(): } doc = Document(DOC_URI, DOC) - assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(doc, cursor_pos) + assert [{'uri': DOC_URI, 'range': def_range}] == pyls_definitions(config, doc, cursor_pos) diff --git a/vscode-client/package.json b/vscode-client/package.json index 29d60061..b2fa4e96 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -40,6 +40,16 @@ "default": true, "description": "Enable or disable the plugin." }, + "pyls.plugins.jedi_definition.follow_imports": { + "type": "boolean", + "default": null, + "description": "The goto call will follow imports." + }, + "pyls.plugins.jedi_definition.follow_builtin_imports": { + "type": "boolean", + "default": null, + "description": "If follow_imports is True will decide if it follow builtin imports." + }, "pyls.plugins.jedi_hover.enabled": { "type": "boolean", "default": true, From 4a37087c2b74c8a061dcc95896107ff21913217a Mon Sep 17 00:00:00 2001 From: Ruslan Kiyanchuk Date: Thu, 11 Oct 2018 11:31:40 -0700 Subject: [PATCH 09/13] Explicitly state pydocstyle is disabled by default (#427) Information in README is misleading listing `pydocstyle` among providers that will be enabled if found, as according to #312 it is disabled by default. The README is updated to explicitly state that pydocstyle is disabled by default and minimal configuration to enable it is provided. --- README.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index b7ff0174..26ad2d2a 100644 --- a/README.rst +++ b/README.rst @@ -26,7 +26,7 @@ If the respective dependencies are found, the following optional providers will * Pyflakes_ linter to detect various errors * McCabe_ linter for complexity checking * pycodestyle_ linter for style checking -* pydocstyle_ linter for docstring style checking +* pydocstyle_ linter for docstring style checking (disabled by default) * autopep8_ for code formatting * YAPF_ for code formatting (preferred over autopep8) @@ -67,6 +67,11 @@ order to respect flake8 configuration instead. Overall configuration is computed first from user configuration (in home directory), overridden by configuration passed in by the language client, and then overriden by configuration discovered in the workspace. +To enable pydocstyle for linting docstrings add the following setting in your LSP configuration: +``` +"pyls.plugins.pydocstyle.enabled": true +``` + Language Server Features ------------------------ From 77d6e3293b61e336310e5bf7a5de79c38cb49b01 Mon Sep 17 00:00:00 2001 From: Will Vaughn Date: Wed, 21 Nov 2018 06:23:36 -0800 Subject: [PATCH 10/13] Update setup.py (#448) --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 3052a817..485ad090 100755 --- a/setup.py +++ b/setup.py @@ -52,7 +52,7 @@ 'pydocstyle>=2.0.0', 'pyflakes>=1.6.0', 'pylint', - 'rope>-0.10.5', + 'rope>=0.10.5', 'yapf', ], 'autopep8': ['autopep8'], From 0c77dafdbac912a73b58589973022d51c086ea45 Mon Sep 17 00:00:00 2001 From: JiahuiJiang Date: Fri, 23 Nov 2018 12:57:18 +0000 Subject: [PATCH 11/13] Make checking parent process aliveness behind a flag (#455) Closes #445 --- pyls/__main__.py | 8 +++++++- pyls/python_ls.py | 9 +++++---- test/test_language_server.py | 25 +++++++++++++++++++------ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/pyls/__main__.py b/pyls/__main__.py index c2175d46..59abf9fb 100644 --- a/pyls/__main__.py +++ b/pyls/__main__.py @@ -24,6 +24,12 @@ def add_arguments(parser): "--port", type=int, default=2087, help="Bind to this port" ) + parser.add_argument( + '--check-parent-process', action="store_true", + help="Check whether parent process is still alive using os.kill(ppid, 0) " + "and auto shut down language server process when parent process is not alive." + "Note that this may not work on a Windows machine." + ) log_group = parser.add_mutually_exclusive_group() log_group.add_argument( @@ -52,7 +58,7 @@ def main(): start_tcp_lang_server(args.host, args.port, PythonLanguageServer) else: stdin, stdout = _binary_stdio() - start_io_lang_server(stdin, stdout, PythonLanguageServer) + start_io_lang_server(stdin, stdout, args.check_parent_process, PythonLanguageServer) def _binary_stdio(): diff --git a/pyls/python_ls.py b/pyls/python_ls.py index b559de17..fffd53ef 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -53,11 +53,11 @@ def start_tcp_lang_server(bind_addr, port, handler_class): server.server_close() -def start_io_lang_server(rfile, wfile, handler_class): +def start_io_lang_server(rfile, wfile, check_parent_process, handler_class): if not issubclass(handler_class, PythonLanguageServer): raise ValueError('Handler class must be an instance of PythonLanguageServer') log.info('Starting %s IO language server', handler_class.__name__) - server = handler_class(rfile, wfile) + server = handler_class(rfile, wfile, check_parent_process) server.start() @@ -68,12 +68,13 @@ class PythonLanguageServer(MethodDispatcher): # pylint: disable=too-many-public-methods,redefined-builtin - def __init__(self, rx, tx): + def __init__(self, rx, tx, check_parent_process=False): self.workspace = None self.config = None self._jsonrpc_stream_reader = JsonRpcStreamReader(rx) self._jsonrpc_stream_writer = JsonRpcStreamWriter(tx) + self._check_parent_process = check_parent_process self._endpoint = Endpoint(self, self._jsonrpc_stream_writer.write, max_workers=MAX_WORKERS) self._dispatchers = [] self._shutdown = False @@ -156,7 +157,7 @@ def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializati self._dispatchers = self._hook('pyls_dispatchers') self._hook('pyls_initialize') - if processId is not None: + if self._check_parent_process and processId is not None: def watch_parent_process(pid): # exist when the given pid is not alive if not _utils.is_process_alive(pid): diff --git a/test/test_language_server.py b/test/test_language_server.py index 65f5b357..41b76152 100644 --- a/test/test_language_server.py +++ b/test/test_language_server.py @@ -16,19 +16,19 @@ def start_client(client): class _ClientServer(object): """ A class to setup a client/server pair """ - def __init__(self): + def __init__(self, check_parent_process=False): # Client to Server pipe csr, csw = os.pipe() # Server to client pipe scr, scw = os.pipe() self.server_thread = Thread(target=start_io_lang_server, args=( - os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), PythonLanguageServer + os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), check_parent_process, PythonLanguageServer )) self.server_thread.daemon = True self.server_thread.start() - self.client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb')) + self.client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb'), start_io_lang_server) self.client_thread = Thread(target=start_client, args=[self.client]) self.client_thread.daemon = True self.client_thread.start() @@ -36,7 +36,9 @@ def __init__(self): @pytest.fixture def client_server(): - """ A fixture that sets up a client/server pair and shuts down the server """ + """ A fixture that sets up a client/server pair and shuts down the server + This client/server pair does not support checking parent process aliveness + """ client_server_pair = _ClientServer() yield client_server_pair.client @@ -48,8 +50,10 @@ def client_server(): @pytest.fixture def client_exited_server(): - """ A fixture that sets up a client/server pair and assert the server has already exited """ - client_server_pair = _ClientServer() + """ A fixture that sets up a client/server pair that support checking parent process aliveness + and assert the server has already exited + """ + client_server_pair = _ClientServer(True) yield client_server_pair.client @@ -74,6 +78,15 @@ def test_exit_with_parent_process_died(client_exited_server): # pylint: disable }).result(timeout=CALL_TIMEOUT) +def test_not_exit_without_check_parent_process_flag(client_server): # pylint: disable=redefined-outer-name + response = client_server._endpoint.request('initialize', { + 'processId': 1234, + 'rootPath': os.path.dirname(__file__), + 'initializationOptions': {} + }).result(timeout=CALL_TIMEOUT) + assert 'capabilities' in response + + def test_missing_message(client_server): # pylint: disable=redefined-outer-name with pytest.raises(JsonRpcMethodNotFound): client_server._endpoint.request('unknown_method').result(timeout=CALL_TIMEOUT) From 807dee5ac675ac753b5f9ff8a79d52331b3a1093 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Mon, 3 Dec 2018 10:34:40 +0100 Subject: [PATCH 12/13] Fix goto_assignments config (#457) --- pyls/plugins/definition.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pyls/plugins/definition.py b/pyls/plugins/definition.py index 7df80a19..005e754c 100644 --- a/pyls/plugins/definition.py +++ b/pyls/plugins/definition.py @@ -7,8 +7,10 @@ @hookimpl def pyls_definitions(config, document, position): - params = {k: v for k, v in config.plugin_settings('jedi_definition').items() if v is not None} - definitions = document.jedi_script(position).goto_assignments(**params) + settings = config.plugin_settings('jedi_definition') + definitions = document.jedi_script(position).goto_assignments( + follow_imports=settings.get('follow_imports', False), + follow_builtin_imports=settings.get('follow_builtin_imports', False)) definitions = [ d for d in definitions From 4c7c0d7899cb098ad50ddd04702deb3b8e1e1a94 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Sun, 9 Dec 2018 23:21:33 +0100 Subject: [PATCH 13/13] Fix signature index error (#461) --- pyls/plugins/signature.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyls/plugins/signature.py b/pyls/plugins/signature.py index 1c1c1c0c..c2b835f5 100644 --- a/pyls/plugins/signature.py +++ b/pyls/plugins/signature.py @@ -21,7 +21,7 @@ def pyls_signature_help(document, position): s = signatures[0] sig = { - 'label': s.docstring().splitlines()[0], + 'label': (s.docstring().splitlines() or [''])[0], 'documentation': _utils.format_docstring(s.docstring(raw=True)) }