From a1672bebb73b130f0de918f3dc0d33879d84c064 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Thu, 28 Dec 2017 15:47:41 +0100 Subject: [PATCH 1/4] Completion: Remove pluggy racing and disable Rope by default This allows the user to choose between Rope and Jedi for completions. It also remove the pluggy racing since Jedi's cache isn't thread safe. Closes #189 --- pyls/_utils.py | 27 --------------------------- pyls/hookspecs.py | 7 ++++++- pyls/plugins/jedi_completion.py | 2 +- pyls/plugins/rope_completion.py | 2 +- pyls/python_ls.py | 22 ++++++++-------------- test/plugins/test_completion.py | 4 ++-- vscode-client/package.json | 15 ++++++++------- 7 files changed, 26 insertions(+), 53 deletions(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index 81414312..fb353c3c 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -89,33 +89,6 @@ def _merge_dicts_(a, b): return dict(_merge_dicts_(dict_a, dict_b)) -def race_hooks(hook_caller, pool, **kwargs): - """Given a pluggy hook spec, execute impls in parallel returning the first non-None result. - - Note this does not support a lot of pluggy functionality, e.g. hook wrappers. - """ - impls = hook_caller._nonwrappers + hook_caller._wrappers - log.debug("Racing hook impls for hook %s: %s", hook_caller, impls) - - if not impls: - return None - - def _apply(impl): - try: - return impl, impl.function(**kwargs) - except Exception: - log.exception("Failed to run hook %s", impl.plugin_name) - raise - - # imap unordered gives us an iterator over the items in the order they finish. - # We have to be careful to set chunksize to 1 to ensure hooks each get their own thread. - # Unfortunately, there's no way to interrupt these threads, so we just have to leave them be. - for impl, result in pool.imap_unordered(_apply, impls, chunksize=1): - if result is not None: - log.debug("Hook from plugin %s returned: %s", impl.plugin_name, result) - return result - - def format_docstring(contents): """Python doc strings come in a number of formats, but LSP wants markdown. diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index 9daa889d..b941c32d 100644 --- a/pyls/hookspecs.py +++ b/pyls/hookspecs.py @@ -23,7 +23,12 @@ def pyls_commands(config, workspace): @hookspec -def pyls_completions(config, workspace, document, position): +def pyls_jedi_completions(config, workspace, document, position): + pass + + +@hookspec +def pyls_rope_completions(config, workspace, document, position): pass diff --git a/pyls/plugins/jedi_completion.py b/pyls/plugins/jedi_completion.py index e70cb5d4..ae770fb7 100644 --- a/pyls/plugins/jedi_completion.py +++ b/pyls/plugins/jedi_completion.py @@ -7,7 +7,7 @@ @hookimpl -def pyls_completions(document, position): +def pyls_jedi_completions(document, position): log.debug('Launching Jedi') definitions = document.jedi_script(position).completions() definitions = [{ diff --git a/pyls/plugins/rope_completion.py b/pyls/plugins/rope_completion.py index e3b17dcf..c9159f43 100644 --- a/pyls/plugins/rope_completion.py +++ b/pyls/plugins/rope_completion.py @@ -9,7 +9,7 @@ @hookimpl -def pyls_completions(document, position): +def pyls_rope_completions(document, position): log.debug('Launching Rope') # Rope is a bit rubbish at completing module imports, so we'll return None diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 8bceb6e5..937b643d 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -1,6 +1,5 @@ # Copyright 2017 Palantir Technologies, Inc. import logging -from multiprocessing import dummy as multiprocessing from . import lsp, _utils from .config import config from .language_server import LanguageServer @@ -8,7 +7,6 @@ log = logging.getLogger(__name__) -PLUGGY_RACE_POOL_SIZE = 5 LINT_DEBOUNCE_S = 0.5 # 500 ms @@ -18,14 +16,10 @@ class PythonLanguageServer(LanguageServer): workspace = None config = None - _pool = multiprocessing.Pool(PLUGGY_RACE_POOL_SIZE) - - def _hook_caller(self, hook_name): - return self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) - def _hook(self, hook_name, doc_uri=None, **kwargs): doc = self.workspace.get_document(doc_uri) if doc_uri else None - return self._hook_caller(hook_name)(config=self.config, workspace=self.workspace, document=doc, **kwargs) + hook = self.config.plugin_manager.subset_hook_caller(hook_name, self.config.disabled_plugins) + return hook(config=self.config, workspace=self.workspace, document=doc, **kwargs) def capabilities(self): return { @@ -65,14 +59,14 @@ def code_lens(self, doc_uri): return flatten(self._hook('pyls_code_lens', doc_uri)) def completions(self, doc_uri, position): - completions = _utils.race_hooks( - self._hook_caller('pyls_completions'), self._pool, - document=self.workspace.get_document(doc_uri) if doc_uri else None, - position=position - ) + if self.config.plugin_settings('completion').get('enabled', True) is False: + return {'isIncomplete': False, 'items': []} + + provider = self.config.plugin_settings('completion').get('provider', 'jedi') + completions = self._hook('pyls_{}_completions'.format(provider), doc_uri, position=position) return { 'isIncomplete': False, - 'items': completions or [] + 'items': flatten(completions) } def definitions(self, doc_uri, position): diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index 7d9b280c..bbc97d5a 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -4,8 +4,8 @@ from pyls import uris from pyls.workspace import Document, get_preferred_submodules -from pyls.plugins.jedi_completion import pyls_completions as pyls_jedi_completions -from pyls.plugins.rope_completion import pyls_completions as pyls_rope_completions +from pyls.plugins.jedi_completion import pyls_jedi_completions +from pyls.plugins.rope_completion import pyls_rope_completions LOCATION = os.path.realpath( os.path.join(os.getcwd(), os.path.dirname(__file__)) diff --git a/vscode-client/package.json b/vscode-client/package.json index a36da57e..587fe760 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -30,10 +30,16 @@ }, "uniqueItems": true }, - "pyls.plugins.jedi_completion.enabled": { + "pyls.plugins.completion.enabled": { "type": "boolean", "default": true, - "description": "Enable or disable the plugin." + "description": "Enable or disable completions." + }, + "pyls.plugins.completion.provider": { + "type": "string", + "default": "jedi", + "enum": ["jedi", "rope"], + "description": "Select a completion provider." }, "pyls.plugins.jedi_definition.enabled": { "type": "boolean", @@ -136,11 +142,6 @@ "default": true, "description": "Enable or disable the plugin." }, - "pyls.plugins.rope_completion.enabled": { - "type": "boolean", - "default": true, - "description": "Enable or disable the plugin." - }, "pyls.plugins.yapf.enabled": { "type": "boolean", "default": true, From f86c66e8f8b992c561f049930153ec0e1b0bfa7a Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Tue, 2 Jan 2018 20:11:22 +0100 Subject: [PATCH 2/4] Only use single completion hook --- pyls/hookspecs.py | 7 +------ pyls/plugins/completion/__init__.py | 18 ++++++++++++++++++ .../{ => completion}/jedi_completion.py | 3 +-- .../{ => completion}/rope_completion.py | 3 +-- pyls/python_ls.py | 6 +----- setup.py | 3 +-- test/plugins/test_completion.py | 4 ++-- 7 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 pyls/plugins/completion/__init__.py rename pyls/plugins/{ => completion}/jedi_completion.py (98%) rename pyls/plugins/{ => completion}/rope_completion.py (98%) diff --git a/pyls/hookspecs.py b/pyls/hookspecs.py index b941c32d..9daa889d 100644 --- a/pyls/hookspecs.py +++ b/pyls/hookspecs.py @@ -23,12 +23,7 @@ def pyls_commands(config, workspace): @hookspec -def pyls_jedi_completions(config, workspace, document, position): - pass - - -@hookspec -def pyls_rope_completions(config, workspace, document, position): +def pyls_completions(config, workspace, document, position): pass diff --git a/pyls/plugins/completion/__init__.py b/pyls/plugins/completion/__init__.py new file mode 100644 index 00000000..b1fce092 --- /dev/null +++ b/pyls/plugins/completion/__init__.py @@ -0,0 +1,18 @@ +from pyls import hookimpl +from .jedi_completion import pyls_jedi_completions +from .rope_completion import pyls_rope_completions + + +@hookimpl +def pyls_settings(): + return {'plugins': {'completion': {'provider': 'jedi'}}} + + +@hookimpl +def pyls_completions(config, document, position): + provider = config.plugin_settings('completion').get('provider', 'jedi') + if provider == 'jedi': + return pyls_jedi_completions(document, position) + elif provider == 'rope': + return pyls_rope_completions(document, position) + return [] diff --git a/pyls/plugins/jedi_completion.py b/pyls/plugins/completion/jedi_completion.py similarity index 98% rename from pyls/plugins/jedi_completion.py rename to pyls/plugins/completion/jedi_completion.py index ae770fb7..882b18fa 100644 --- a/pyls/plugins/jedi_completion.py +++ b/pyls/plugins/completion/jedi_completion.py @@ -1,12 +1,11 @@ # Copyright 2017 Palantir Technologies, Inc. import logging from pyls.lsp import CompletionItemKind -from pyls import hookimpl, _utils +from pyls import _utils log = logging.getLogger(__name__) -@hookimpl def pyls_jedi_completions(document, position): log.debug('Launching Jedi') definitions = document.jedi_script(position).completions() diff --git a/pyls/plugins/rope_completion.py b/pyls/plugins/completion/rope_completion.py similarity index 98% rename from pyls/plugins/rope_completion.py rename to pyls/plugins/completion/rope_completion.py index c9159f43..b9d234c4 100644 --- a/pyls/plugins/rope_completion.py +++ b/pyls/plugins/completion/rope_completion.py @@ -2,13 +2,12 @@ import logging from rope.contrib.codeassist import code_assist, sorted_proposals -from pyls import hookimpl, lsp +from pyls import lsp log = logging.getLogger(__name__) -@hookimpl def pyls_rope_completions(document, position): log.debug('Launching Rope') diff --git a/pyls/python_ls.py b/pyls/python_ls.py index 937b643d..45f26a23 100644 --- a/pyls/python_ls.py +++ b/pyls/python_ls.py @@ -59,11 +59,7 @@ def code_lens(self, doc_uri): return flatten(self._hook('pyls_code_lens', doc_uri)) def completions(self, doc_uri, position): - if self.config.plugin_settings('completion').get('enabled', True) is False: - return {'isIncomplete': False, 'items': []} - - provider = self.config.plugin_settings('completion').get('provider', 'jedi') - completions = self._hook('pyls_{}_completions'.format(provider), doc_uri, position=position) + completions = self._hook('pyls_completions', doc_uri, position=position) return { 'isIncomplete': False, 'items': flatten(completions) diff --git a/setup.py b/setup.py index 665e3bcc..55a821a6 100755 --- a/setup.py +++ b/setup.py @@ -61,8 +61,7 @@ 'pyls = pyls.__main__:main', ], 'pyls': [ - 'rope_completion = pyls.plugins.rope_completion', - 'jedi_completion = pyls.plugins.jedi_completion', + 'completion = pyls.plugins.completion', 'jedi_definition = pyls.plugins.definition', 'jedi_hover = pyls.plugins.hover', 'jedi_references = pyls.plugins.references', diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index bbc97d5a..020df482 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -4,8 +4,8 @@ from pyls import uris from pyls.workspace import Document, get_preferred_submodules -from pyls.plugins.jedi_completion import pyls_jedi_completions -from pyls.plugins.rope_completion import pyls_rope_completions +from pyls.plugins.completion.jedi_completion import pyls_jedi_completions +from pyls.plugins.completion.rope_completion import pyls_rope_completions LOCATION = os.path.realpath( os.path.join(os.getcwd(), os.path.dirname(__file__)) From 64a580c15caa6a297cafb9c96680b97439f5e893 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 10 Jan 2018 11:52:18 +0000 Subject: [PATCH 3/4] Remove pluggy and disable rope completions by default --- pyls/plugins/completion/__init__.py | 18 ---- pyls/plugins/completion/jedi_completion.py | 87 ------------------- pyls/plugins/jedi_completion.py | 84 ++++++++++++++++++ .../{completion => }/rope_completion.py | 13 ++- setup.py | 3 +- test/plugins/test_completion.py | 4 +- 6 files changed, 97 insertions(+), 112 deletions(-) delete mode 100644 pyls/plugins/completion/__init__.py delete mode 100644 pyls/plugins/completion/jedi_completion.py create mode 100644 pyls/plugins/jedi_completion.py rename pyls/plugins/{completion => }/rope_completion.py (93%) diff --git a/pyls/plugins/completion/__init__.py b/pyls/plugins/completion/__init__.py deleted file mode 100644 index b1fce092..00000000 --- a/pyls/plugins/completion/__init__.py +++ /dev/null @@ -1,18 +0,0 @@ -from pyls import hookimpl -from .jedi_completion import pyls_jedi_completions -from .rope_completion import pyls_rope_completions - - -@hookimpl -def pyls_settings(): - return {'plugins': {'completion': {'provider': 'jedi'}}} - - -@hookimpl -def pyls_completions(config, document, position): - provider = config.plugin_settings('completion').get('provider', 'jedi') - if provider == 'jedi': - return pyls_jedi_completions(document, position) - elif provider == 'rope': - return pyls_rope_completions(document, position) - return [] diff --git a/pyls/plugins/completion/jedi_completion.py b/pyls/plugins/completion/jedi_completion.py deleted file mode 100644 index 882b18fa..00000000 --- a/pyls/plugins/completion/jedi_completion.py +++ /dev/null @@ -1,87 +0,0 @@ -# Copyright 2017 Palantir Technologies, Inc. -import logging -from pyls.lsp import CompletionItemKind -from pyls import _utils - -log = logging.getLogger(__name__) - - -def pyls_jedi_completions(document, position): - log.debug('Launching Jedi') - definitions = document.jedi_script(position).completions() - definitions = [{ - 'label': _label(d), - 'kind': _kind(d), - 'detail': _detail(d), - 'documentation': _utils.format_docstring(d.docstring()), - 'sortText': _sort_text(d), - 'insertText': d.name - } for d in definitions] - log.debug('Jedi finished') - return definitions - - -def _label(definition): - if definition.type in ('function', 'method'): - params = ", ".join(param.name for param in definition.params) - return "{}({})".format(definition.name, params) - - return definition.name - - -def _detail(definition): - return "builtin" if definition.in_builtin_module() else definition.parent().full_name or "" - - -def _sort_text(definition): - """ Ensure builtins appear at the bottom. - Description is of format : . - """ - if definition.in_builtin_module(): - # It's a builtin, put it last - return 'z' + definition.name - - if definition.name.startswith("_"): - # It's a 'hidden' func, put it next last - return 'y' + definition.name - - # Else put it at the front - return 'a' + definition.name - - -def _kind(d): - """ Return the VSCode type """ - MAP = { - 'none': CompletionItemKind.Value, - 'type': CompletionItemKind.Class, - 'tuple': CompletionItemKind.Class, - 'dict': CompletionItemKind.Class, - 'dictionary': CompletionItemKind.Class, - 'function': CompletionItemKind.Function, - 'lambda': CompletionItemKind.Function, - 'generator': CompletionItemKind.Function, - 'class': CompletionItemKind.Class, - 'instance': CompletionItemKind.Reference, - 'method': CompletionItemKind.Method, - 'builtin': CompletionItemKind.Class, - 'builtinfunction': CompletionItemKind.Function, - 'module': CompletionItemKind.Module, - 'file': CompletionItemKind.File, - 'xrange': CompletionItemKind.Class, - 'slice': CompletionItemKind.Class, - 'traceback': CompletionItemKind.Class, - 'frame': CompletionItemKind.Class, - 'buffer': CompletionItemKind.Class, - 'dictproxy': CompletionItemKind.Class, - 'funcdef': CompletionItemKind.Function, - 'property': CompletionItemKind.Property, - 'import': CompletionItemKind.Module, - 'keyword': CompletionItemKind.Keyword, - 'constant': CompletionItemKind.Variable, - 'variable': CompletionItemKind.Variable, - 'value': CompletionItemKind.Value, - 'param': CompletionItemKind.Variable, - 'statement': CompletionItemKind.Keyword, - } - - return MAP.get(d.type) diff --git a/pyls/plugins/jedi_completion.py b/pyls/plugins/jedi_completion.py new file mode 100644 index 00000000..5d4ac6c1 --- /dev/null +++ b/pyls/plugins/jedi_completion.py @@ -0,0 +1,84 @@ +# Copyright 2017 Palantir Technologies, Inc. +import logging +from pyls import hookimpl, lsp, _utils + +log = logging.getLogger(__name__) + + +@hookimpl +def pyls_completions(document, position): + definitions = document.jedi_script(position).completions() + return [{ + 'label': _label(d), + 'kind': _kind(d), + 'detail': _detail(d), + 'documentation': _utils.format_docstring(d.docstring()), + 'sortText': _sort_text(d), + 'insertText': d.name + } for d in definitions] or None + + +def _label(definition): + if definition.type in ('function', 'method'): + params = ", ".join(param.name for param in definition.params) + return "{}({})".format(definition.name, params) + + return definition.name + + +def _detail(definition): + return "builtin" if definition.in_builtin_module() else definition.parent().full_name or "" + + +def _sort_text(definition): + """ Ensure builtins appear at the bottom. + Description is of format : . + """ + if definition.in_builtin_module(): + # It's a builtin, put it last + return 'z' + definition.name + + if definition.name.startswith("_"): + # It's a 'hidden' func, put it next last + return 'y' + definition.name + + # Else put it at the front + return 'a' + definition.name + + +def _kind(d): + """ Return the VSCode type """ + MAP = { + 'none': lsp.CompletionItemKind.Value, + 'type': lsp.CompletionItemKind.Class, + 'tuple': lsp.CompletionItemKind.Class, + 'dict': lsp.CompletionItemKind.Class, + 'dictionary': lsp.CompletionItemKind.Class, + 'function': lsp.CompletionItemKind.Function, + 'lambda': lsp.CompletionItemKind.Function, + 'generator': lsp.CompletionItemKind.Function, + 'class': lsp.CompletionItemKind.Class, + 'instance': lsp.CompletionItemKind.Reference, + 'method': lsp.CompletionItemKind.Method, + 'builtin': lsp.CompletionItemKind.Class, + 'builtinfunction': lsp.CompletionItemKind.Function, + 'module': lsp.CompletionItemKind.Module, + 'file': lsp.CompletionItemKind.File, + 'xrange': lsp.CompletionItemKind.Class, + 'slice': lsp.CompletionItemKind.Class, + 'traceback': lsp.CompletionItemKind.Class, + 'frame': lsp.CompletionItemKind.Class, + 'buffer': lsp.CompletionItemKind.Class, + 'dictproxy': lsp.CompletionItemKind.Class, + 'funcdef': lsp.CompletionItemKind.Function, + 'property': lsp.CompletionItemKind.Property, + 'import': lsp.CompletionItemKind.Module, + 'keyword': lsp.CompletionItemKind.Keyword, + 'constant': lsp.CompletionItemKind.Variable, + 'variable': lsp.CompletionItemKind.Variable, + 'value': lsp.CompletionItemKind.Value, + 'param': lsp.CompletionItemKind.Variable, + 'statement': lsp.CompletionItemKind.Keyword, + } + + return MAP.get(d.type) diff --git a/pyls/plugins/completion/rope_completion.py b/pyls/plugins/rope_completion.py similarity index 93% rename from pyls/plugins/completion/rope_completion.py rename to pyls/plugins/rope_completion.py index b9d234c4..de3312f4 100644 --- a/pyls/plugins/completion/rope_completion.py +++ b/pyls/plugins/rope_completion.py @@ -2,15 +2,20 @@ import logging from rope.contrib.codeassist import code_assist, sorted_proposals -from pyls import lsp +from pyls import hookimpl, lsp log = logging.getLogger(__name__) -def pyls_rope_completions(document, position): - log.debug('Launching Rope') +@hookimpl +def pyls_settings(): + # Default rope_completion to disabled + return {'plugins': {'rope_completion': {'enabled': False}}} + +@hookimpl +def pyls_completions(document, position): # Rope is a bit rubbish at completing module imports, so we'll return None word = document.word_at_position({ # The -1 should really be trying to look at the previous word, but that might be quite expensive @@ -39,7 +44,7 @@ def pyls_rope_completions(document, position): 'documentation': doc or "", 'sortText': _sort_text(d)}) definitions = new_definitions - log.debug('Rope finished') + return definitions or None diff --git a/setup.py b/setup.py index 55a821a6..194d3d3f 100755 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ 'pyls = pyls.__main__:main', ], 'pyls': [ - 'completion = pyls.plugins.completion', + 'jedi_completion = pyls.plugins.jedi_completion', 'jedi_definition = pyls.plugins.definition', 'jedi_hover = pyls.plugins.hover', 'jedi_references = pyls.plugins.references', @@ -71,6 +71,7 @@ 'pycodestyle = pyls.plugins.pycodestyle_lint', 'pydocstyle = pyls.plugins.pydocstyle_lint', 'pyflakes = pyls.plugins.pyflakes_lint', + 'rope_completion = pyls.plugins.rope_completion', 'rope_rename = pyls.plugins.rope_rename', 'yapf = pyls.plugins.format', ] diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index 020df482..7d9b280c 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -4,8 +4,8 @@ from pyls import uris from pyls.workspace import Document, get_preferred_submodules -from pyls.plugins.completion.jedi_completion import pyls_jedi_completions -from pyls.plugins.completion.rope_completion import pyls_rope_completions +from pyls.plugins.jedi_completion import pyls_completions as pyls_jedi_completions +from pyls.plugins.rope_completion import pyls_completions as pyls_rope_completions LOCATION = os.path.realpath( os.path.join(os.getcwd(), os.path.dirname(__file__)) From 9b2c7b6d62080e43d88d8e98946835f1dafd2751 Mon Sep 17 00:00:00 2001 From: Nicholas Gates Date: Wed, 10 Jan 2018 11:57:45 +0000 Subject: [PATCH 4/4] Remove pluggy --- vscode-client/package.json | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/vscode-client/package.json b/vscode-client/package.json index 587fe760..a36da57e 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -30,16 +30,10 @@ }, "uniqueItems": true }, - "pyls.plugins.completion.enabled": { + "pyls.plugins.jedi_completion.enabled": { "type": "boolean", "default": true, - "description": "Enable or disable completions." - }, - "pyls.plugins.completion.provider": { - "type": "string", - "default": "jedi", - "enum": ["jedi", "rope"], - "description": "Select a completion provider." + "description": "Enable or disable the plugin." }, "pyls.plugins.jedi_definition.enabled": { "type": "boolean", @@ -142,6 +136,11 @@ "default": true, "description": "Enable or disable the plugin." }, + "pyls.plugins.rope_completion.enabled": { + "type": "boolean", + "default": true, + "description": "Enable or disable the plugin." + }, "pyls.plugins.yapf.enabled": { "type": "boolean", "default": true,