Respect client capabilities snippet support (#529)

Fixes #515 
Fixes #526 

Also only generates snippets for positional args

.pylintrc

@@ -23,4 +23,6 @@ reports = no
 
 [TYPECHECK]
 
-generated-members = pyls_*
+generated-members =
+    pyls_*
+    cache_clear

pyls/config/config.py

@@ -18,11 +18,12 @@
 
 class Config(object):
 
-    def __init__(self, root_uri, init_opts, process_id):
+    def __init__(self, root_uri, init_opts, process_id, capabilities):
         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._capabilities = capabilities
 
         self._settings = {}
         self._plugin_settings = {}
@@ -86,6 +87,10 @@ def root_uri(self):
     def process_id(self):
         return self._process_id
 
+    @property
+    def capabilities(self):
+        return self._capabilities
+
     @lru_cache(maxsize=32)
     def settings(self, document_path=None):
         """Settings are constructed from a few sources:

pyls/config/source.py

@@ -25,7 +25,8 @@ def project_config(self, document_path):
         """Return project-level (i.e. workspace directory) configuration."""
         raise NotImplementedError()
 
-    def read_config_from_files(self, files):
+    @staticmethod
+    def read_config_from_files(files):
         config = configparser.RawConfigParser()
         for filename in files:
             if os.path.exists(filename) and not os.path.isdir(filename):

pyls/plugins/jedi_completion.py

@@ -45,10 +45,13 @@ def pyls_completions(config, document, position):
     if not definitions:
         return None
 
+    completion_capabilities = config.capabilities.get('textDocument', {}).get('completion', {})
+    snippet_support = completion_capabilities.get('completionItem', {}).get('snippetSupport')
+
     settings = config.plugin_settings('jedi_completion', document_path=document.path)
-    include_params = settings.get('include_params', True)
+    should_include_params = settings.get('include_params')
 
-    return [_format_completion(d, include_params) for d in definitions] or None
+    return [_format_completion(d, snippet_support and should_include_params) for d in definitions] or None
 
 
 def _format_completion(d, include_params=True):
@@ -62,15 +65,18 @@ def _format_completion(d, include_params=True):
     }
 
     if include_params and hasattr(d, 'params') and d.params:
+        positional_args = [param for param in d.params if '=' not in param.description]
+
         # For completions with params, we can generate a snippet instead
         completion['insertTextFormat'] = lsp.InsertTextFormat.Snippet
         snippet = d.name + '('
-        for i, param in enumerate(d.params):
+        for i, param in enumerate(positional_args):
             snippet += '${%s:%s}' % (i + 1, param.name)
-            if i < len(d.params) - 1:
+            if i < len(positional_args) - 1:
                 snippet += ', '
         snippet += ')$0'
         completion['insertText'] = snippet
+
     return completion
 
 

pyls/python_ls.py

@@ -157,7 +157,8 @@ 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 {}, processId)
+        self.config = config.Config(rootUri, initializationOptions or {},
+                                    processId, _kwargs.get('capabilities', {}))
         self._dispatchers = self._hook('pyls_dispatchers')
         self._hook('pyls_initialize')
 
@@ -299,17 +300,17 @@ def m_workspace__did_change_configuration(self, settings=None):
         for doc_uri in self.workspace.documents:
             self.lint(doc_uri, is_saved=False)
 
-    def m_workspace__did_change_watched_files(self, changes=[], **_kwargs):
+    def m_workspace__did_change_watched_files(self, changes=None, **_kwargs):
         changed_py_files = set()
         config_changed = False
-        for d in changes:
+        for d in (changes or []):
             if d['uri'].endswith(PYTHON_FILE_EXTENSIONS):
                 changed_py_files.add(d['uri'])
             elif d['uri'].endswith(CONFIG_FILEs):
                 config_changed = True
 
         if config_changed:
-            self.settings.cache_clear()
+            self.config.settings.cache_clear()
         elif not changed_py_files:
             # Only externally changed python files and lint configs may result in changed diagnostics.
             return

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, {}, 0)
+    return Config(workspace.root_uri, {}, 0, {})
 
 
 @pytest.fixture

test/plugins/test_completion.py

@@ -95,11 +95,15 @@ def test_jedi_method_completion(config):
     com_position = {'line': 20, 'character': 19}
     doc = Document(DOC_URI, DOC)
 
+    config.capabilities['textDocument'] = {'completion': {'completionItem': {'snippetSupport': True}}}
+    config.update({'plugins': {'jedi_completion': {'include_params': True}}})
+
     completions = pyls_jedi_completions(config, doc, com_position)
     everyone_method = [completion for completion in completions if completion['label'] == 'everyone(a, b, c, d)'][0]
 
+    # Ensure we only generate snippets for positional args
     assert everyone_method['insertTextFormat'] == lsp.InsertTextFormat.Snippet
-    assert everyone_method['insertText'] == 'everyone(${1:a}, ${2:b}, ${3:c}, ${4:d})$0'
+    assert everyone_method['insertText'] == 'everyone(${1:a}, ${2:b})$0'
 
     # Disable param snippets
     config.update({'plugins': {'jedi_completion': {'include_params': False}}})

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, {}, 1234)
+    config = Config(workspace.root_uri, {}, 1234, {})
 
     # Make sure we get a warning for 'indentation contains tabs'
     diags = pycodestyle_lint.pyls_lint(config, doc)