From 25e277305877239ca0464e5980403790da24116b Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 01/10] Introduce config source for "pyls" --- pyls/config/config.py | 5 +++- pyls/config/pyls_conf.py | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 pyls/config/pyls_conf.py diff --git a/pyls/config/config.py b/pyls/config/config.py index 65696d81..f035ae1d 100644 --- a/pyls/config/config.py +++ b/pyls/config/config.py @@ -40,6 +40,9 @@ def __init__(self, root_uri, init_opts, process_id, capabilities): except ImportError: pass + from .pyls_conf import PylsConfig + self._config_sources['pyls'] = PylsConfig(self._root_path) + self._pm = pluggy.PluginManager(PYLS) self._pm.trace.root.setwriter(log.debug) self._pm.enable_tracing() @@ -104,7 +107,7 @@ def settings(self, document_path=None): settings.cache_clear() when the config is updated """ settings = {} - sources = self._settings.get('configurationSources', DEFAULT_CONFIG_SOURCES) + sources = self._settings.get('configurationSources', DEFAULT_CONFIG_SOURCES) + ['pyls'] for source_name in reversed(sources): source = self._config_sources.get(source_name) diff --git a/pyls/config/pyls_conf.py b/pyls/config/pyls_conf.py new file mode 100644 index 00000000..941a37d0 --- /dev/null +++ b/pyls/config/pyls_conf.py @@ -0,0 +1,63 @@ +import logging +import os +from pyls._utils import find_parents, merge_dicts +from .source import ConfigSource + +log = logging.getLogger(__name__) + +CONFIG_KEY = 'pyls' +PROJECT_CONFIGS = ['setup.cfg', 'tox.ini'] + +OPTIONS = [ +] + + +class PylsConfig(ConfigSource): + """Parse pyls configuration""" + + def __init__(self, root_path): + super(PylsConfig, self).__init__(root_path) + + # If workspace URI has trailing '/', root_path does, too. + # (e.g. "lsp" on Emacs sends such URI). To avoid normpath at + # each normalization, os.path.normpath()-ed root_path is + # cached here. + self._norm_root_path = os.path.normpath(self.root_path) + + def user_config(self): + # pyls specific configuration mainly focuses on per-project + # configuration + return {} + + def project_config(self, document_path): + settings = {} + seen = set() + + # To read config files in root_path even if any config file is + # found by find_parents() in the directory other than + # root_path, root_path is listed below as one of targets. + # + # On the other hand, "seen" is used to manage name of already + # evaluated files, in order to avoid multiple evaluation of + # config files in root_path. + for target in self.root_path, document_path: + # os.path.normpath is needed to treat that + # "root_path/./foobar" and "root_path/foobar" are + # identical. + sources = map(os.path.normpath, find_parents(self.root_path, target, PROJECT_CONFIGS)) + files = [] + for source in sources: + if source not in seen: + files.append(source) + seen.add(source) + if not files: + continue # there is no file to be read in + + parsed = self.parse_config(self.read_config_from_files(files), + CONFIG_KEY, OPTIONS) + if not parsed: + continue # no pyls specific configuration + + settings = merge_dicts(settings, parsed) + + return settings From 1d3141d5a4517352c015eeb6b7ff11e9eb9be7a7 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 02/10] Add utility to access configuration value by path At getting the value in nested settings dict, newly added get_config_by_path() is more efficient and readable than "get with empty dict" chain like below, which is sometimes used in current implementation. settings.get("foo", {}).get("bar", {}).get("baz") --- pyls/_utils.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pyls/_utils.py b/pyls/_utils.py index 919bf1c5..67229e23 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -132,6 +132,25 @@ def _merge_dicts_(a, b): return dict(_merge_dicts_(dict_a, dict_b)) +def get_config_by_path(settings, path, default_value=None): + """Get the value in settings dict at the given path. + + If path is not resolvable in specified settings, this returns + default_value. + """ + paths = path.split('.') + while len(paths) > 1: + settings = settings.get(paths.pop(0)) + if not isinstance(settings, dict): + # Here, at least one more looking up should be available, + # but the last retrieved was non-dict. Therefore, path is + # not resolvable in specified settings. + return default_value + + # Here, paths should have only one value + return settings.get(paths[0], default_value) + + def format_docstring(contents): """Python doc strings come in a number of formats, but LSP wants markdown. From b28c544ec56eaa30756e0adc0a10a6c082537b94 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 03/10] Introduce pyls.source_roots configuration Before this commit, 'setup.py' or 'pyproject.toml' should be created to specify source root(s) other than the workspace root, even if such file is not actually needed. In order to explicitly specify source root(s) in the workspace, this commit introduces pyls.source_roots configuration, and makes workspace.source_roots() return its value if configured. Path in pyls.source_roots is ignored, if it refers outside of the workspace. This configuration is also useful in the case that the workspace consists of multiple (independent) services and a library shared by them, for example. In such case, N + 1 source roots are listed in pyls.source_roots configuration. BTW, this commit puts some utilities into _util.py for reuse in the future. Especially, os.path.commonprefix() for checking inside-ness in find_parents() below should be replaced with is_inside_of(), because the former returns unintentional value in some cases. if not os.path.commonprefix((root, path)): log.warning("Path %s not in %s", path, root) return [] For example: - commonprefix(('/foo', '/bar')) returns not false value but '/' - commonprefix(('/foo', '/foobar')) returns not false value but '/foo' --- pyls/_utils.py | 42 +++++++++++++ pyls/config/pyls_conf.py | 27 +++++++- pyls/workspace.py | 5 ++ test/test_workspace.py | 125 +++++++++++++++++++++++++++++++++++++ vscode-client/package.json | 9 +++ 5 files changed, 206 insertions(+), 2 deletions(-) diff --git a/pyls/_utils.py b/pyls/_utils.py index 67229e23..db2c7ee6 100644 --- a/pyls/_utils.py +++ b/pyls/_utils.py @@ -83,6 +83,48 @@ def find_parents(root, path, names): return [] +def is_inside_of(path, root, strictly=True): + """Return whether path is inside of root or not + + It is assumed that both path and root are absolute. + + If strictly=False, os.path.normcase and os.path.normpath are not + applied on path and root for efficiency. This is useful if + ablsolute "path" is made from relative one and "root" (and already + normpath-ed), for example. + """ + if strictly: + path = os.path.normcase(os.path.normpath(path)) + root = os.path.normcase(os.path.normpath(root)) + + return path == root or path.startswith(root + os.path.sep) + + +def normalize_paths(paths, basedir, inside_only): + """Normalize each elements in paths + + Relative elements in paths are treated as relative to basedir. + + This function yields "(path, validity)" tuple as normalization + result for each elements in paths. If inside_only is specified and path is + not so, validity is False. Otherwise, path is already normalized + as absolute path, and validity is True. + """ + for path in paths: + full_path = os.path.normpath(os.path.join(basedir, path)) + if (not inside_only or + # If "inside_only" is specified, path must (1) not be + # absolute (= be relative to basedir), (2) not have + # drive letter (on Windows), and (3) be descendant of + # the root (= "inside_only"). + (not os.path.isabs(path) and + not os.path.splitdrive(path)[0] and + is_inside_of(full_path, inside_only, strictly=False))): + yield full_path, True + else: + yield path, False + + def match_uri_to_workspace(uri, workspaces): if uri is None: return None diff --git a/pyls/config/pyls_conf.py b/pyls/config/pyls_conf.py index 941a37d0..d6be3131 100644 --- a/pyls/config/pyls_conf.py +++ b/pyls/config/pyls_conf.py @@ -1,7 +1,7 @@ import logging import os -from pyls._utils import find_parents, merge_dicts -from .source import ConfigSource +from pyls._utils import find_parents, get_config_by_path, merge_dicts, normalize_paths +from .source import ConfigSource, _set_opt log = logging.getLogger(__name__) @@ -9,6 +9,12 @@ PROJECT_CONFIGS = ['setup.cfg', 'tox.ini'] OPTIONS = [ + ('source_roots', 'source_roots', list), +] + +# list of (config_path, inside_only) tuples for path normalization +NORMALIZED_CONFIGS = [ + ('source_roots', True), ] @@ -58,6 +64,23 @@ def project_config(self, document_path): if not parsed: continue # no pyls specific configuration + self.normalize(parsed, os.path.dirname(files[0])) + settings = merge_dicts(settings, parsed) return settings + + def normalize(self, config, basedir): + for config_path, inside_only in NORMALIZED_CONFIGS: + paths = get_config_by_path(config, config_path) + if not paths: + continue # not specified (or empty) + + normalized = [] + for path, valid in normalize_paths(paths, basedir, inside_only and self._norm_root_path): + if valid: + normalized.append(path) + else: + log.warning("Ignoring path '%s' for pyls.%s", path, config_path) + + _set_opt(config, config_path, normalized) diff --git a/pyls/workspace.py b/pyls/workspace.py index a58b76a2..a39a468b 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -97,6 +97,11 @@ def show_message(self, message, msg_type=lsp.MessageType.Info): def source_roots(self, document_path): """Return the source roots for the given document.""" + source_roots = self._config and self._config.settings(document_path=document_path).get('source_roots') + if source_roots: + # Use specified source_roots without traversing upper directories + return source_roots + files = _utils.find_parents(self._root_path, document_path, ['setup.py', 'pyproject.toml']) or [] return list(set((os.path.dirname(project_file) for project_file in files))) or [self._root_path] diff --git a/test/test_workspace.py b/test/test_workspace.py index 9b5b7b06..88ebc716 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -5,13 +5,16 @@ import pytest from pyls import uris +from pyls.python_ls import PythonLanguageServer PY2 = sys.version_info.major == 2 if PY2: import pathlib2 as pathlib + from StringIO import StringIO else: import pathlib + from io import StringIO DOC_URI = uris.from_fs_path(__file__) @@ -119,3 +122,125 @@ def test_multiple_workspaces(tmpdir, pyls): pyls.m_workspace__did_change_workspace_folders( added=[], removed=[added_workspaces[0]]) assert workspace1_uri not in pyls.workspaces + + +def _make_paths_dir_relative(paths, dirname): + """Make paths relative to dirname + + This method assumes below for simplicity: + + - if a path in paths is relative, it is relative to "root" + - dirname must be relative to "root" + - empty dirname means "root" itself + - neither "." nor ".." is allowed for dirname + - dirname must use '/' as delimiter (because "metafile" uses it) + """ + to_root = os.path.join(*(['..'] * len(dirname.split('/')))) if dirname else '' + return list(p if os.path.isabs(p) else os.path.join(to_root, p) for p in paths) + + +@pytest.mark.parametrize('metafile', [ + 'setup.cfg', + 'tox.ini', + 'service/foo/setup.cfg', + 'service/foo/tox.ini', +]) +def test_source_roots_config(tmpdir, metafile): + """Examine that source_roots config is intentionaly read in. + + This test also examines below for entries in source_roots: + + * absolute path is ignored + * relative path is: + - ignored, if it does not refer inside of the workspace + - otherwise, it is treated as relative to config file location, and + - normalized into absolute one + """ + root_path = str(tmpdir) + + invalid_roots = ['/invalid/root', '../baz'] + source_roots = ['service/foo', 'service/bar'] + invalid_roots + doc_root = source_roots[0] + + if metafile: + dirname = os.path.dirname(metafile) + if dirname: + os.makedirs(os.path.join(root_path, dirname)) + + # configured by metafile at pyls startup + with open(os.path.join(root_path, metafile), 'w+') as f: + f.write('[pyls]\nsource_roots=\n %s\n' % + ',\n '.join(_make_paths_dir_relative(source_roots, dirname))) + + pyls = PythonLanguageServer(StringIO, StringIO) + pyls.m_initialize( + processId=1, + rootUri=uris.from_fs_path(root_path), + initializationOptions={} + ) + + # put new document under ROOT/service/foo + test_uri = uris.from_fs_path(os.path.join(root_path, doc_root, 'hello/test.py')) + pyls.workspace.put_document(test_uri, 'assert true') + test_doc = pyls.workspace.get_document(test_uri) + + # apply os.path.normcase() on paths below, because case-sensitive + # comparison on Windows causes unintentional failure for case + # instability around drive letter + + sys_path = [os.path.normcase(p) for p in test_doc.sys_path()] + for raw_path in source_roots: + full_path = os.path.normcase(os.path.join(root_path, raw_path)) + if raw_path in invalid_roots: + assert os.path.normpath(full_path) not in sys_path + assert full_path not in sys_path # check for safety + else: + assert os.path.normpath(full_path) in sys_path + + +@pytest.mark.parametrize('metafile', ['setup.cfg', 'tox.ini']) +def test_pyls_config_readin(tmpdir, metafile): + """Examine that pyls config in the workspace root is always read in. + + This test creates two config files. One is created in the + workspace root, and another is created in ascendant of the target + document. Only the former has source_roots config. + + Then, this test examines that the former is always read in, + regardless of existence of the latter, by checking source_roots + config. + """ + root_path = str(tmpdir) + + source_roots = ['service/foo', 'service/bar'] + + with open(os.path.join(root_path, metafile), 'w+') as f: + f.write('[pyls]\nsource_roots=\n %s\n' % + ',\n '.join(source_roots)) + + doc_root = source_roots[0] + + os.makedirs(os.path.join(root_path, doc_root)) + with open(os.path.join(root_path, doc_root, metafile), 'w+') as f: + f.write('\n') + + pyls = PythonLanguageServer(StringIO, StringIO) + pyls.m_initialize( + processId=1, + rootUri=uris.from_fs_path(root_path), + initializationOptions={} + ) + + # put new document under root/service/foo + test_uri = uris.from_fs_path(os.path.join(root_path, doc_root, 'hello/test.py')) + pyls.workspace.put_document(test_uri, 'assert True') + test_doc = pyls.workspace.get_document(test_uri) + + # apply os.path.normcase() on paths below, because case-sensitive + # comparison on Windows causes unintentional failure for case + # instability around drive letter + + sys_path = [os.path.normcase(p) for p in test_doc.sys_path()] + for raw_path in source_roots: + full_path = os.path.normcase(os.path.join(root_path, raw_path)) + assert os.path.normpath(full_path) in sys_path diff --git a/vscode-client/package.json b/vscode-client/package.json index eb4faeea..42863b08 100644 --- a/vscode-client/package.json +++ b/vscode-client/package.json @@ -35,6 +35,15 @@ }, "uniqueItems": true }, + "pyls.source_roots": { + "type": "array", + "default": [], + "description": "List of source roots in the working space.", + "items": { + "type": "string" + }, + "uniqueItems": true + }, "pyls.plugins.jedi.extra_paths": { "type": "array", "default": [], From bd23fcfd646b5154cc0df861759c67915ab41197 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 04/10] Intorduce ConfigSource.lsp_config to hook at updating --- pyls/config/config.py | 7 +++++++ pyls/config/source.py | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/pyls/config/config.py b/pyls/config/config.py index f035ae1d..fe6072ba 100644 --- a/pyls/config/config.py +++ b/pyls/config/config.py @@ -144,6 +144,13 @@ def plugin_settings(self, plugin, document_path=None): def update(self, settings): """Recursively merge the given settings into the current settings.""" + sources = settings.get('configurationSources', DEFAULT_CONFIG_SOURCES) + ['pyls'] + for source_name in reversed(sources): + source = self._config_sources.get(source_name) + if not source: + continue + source.lsp_config(settings) + self.settings.cache_clear() self._settings = settings log.info("Updated settings to %s", self._settings) diff --git a/pyls/config/source.py b/pyls/config/source.py index c3442e01..46715deb 100644 --- a/pyls/config/source.py +++ b/pyls/config/source.py @@ -25,6 +25,16 @@ def project_config(self, document_path): """Return project-level (i.e. workspace directory) configuration.""" raise NotImplementedError() + def lsp_config(self, config): + """Check configuration at updating. + + Typical usecase is updating via LSP didChangeConfiguration + method. + + Change config directly, if any changing like normalization is + needed. It is nested dictionay. + """ + @staticmethod def read_config_from_files(files): config = configparser.RawConfigParser() From 6dd31491d8eee3cce1dc230d89665eaf49d572d9 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 05/10] Normalize ply.source_roots at LSP didChangeConfiguration method This is useful to specify source roots in relative path, if pyls process might run in other than the root of the target workspace(s). --- pyls/config/pyls_conf.py | 3 +++ test/test_workspace.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/pyls/config/pyls_conf.py b/pyls/config/pyls_conf.py index d6be3131..d6a92a6d 100644 --- a/pyls/config/pyls_conf.py +++ b/pyls/config/pyls_conf.py @@ -70,6 +70,9 @@ def project_config(self, document_path): return settings + def lsp_config(self, config): + self.normalize(config, self._norm_root_path) + def normalize(self, config, basedir): for config_path, inside_only in NORMALIZED_CONFIGS: paths = get_config_by_path(config, config_path) diff --git a/test/test_workspace.py b/test/test_workspace.py index 88ebc716..5da7e47c 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -144,6 +144,7 @@ def _make_paths_dir_relative(paths, dirname): 'tox.ini', 'service/foo/setup.cfg', 'service/foo/tox.ini', + None, ]) def test_source_roots_config(tmpdir, metafile): """Examine that source_roots config is intentionaly read in. @@ -179,6 +180,12 @@ def test_source_roots_config(tmpdir, metafile): initializationOptions={} ) + if not metafile: + # configured by client via LSP after pyls startup + pyls.m_workspace__did_change_configuration({ + 'pyls': {'source_roots': source_roots}, + }) + # put new document under ROOT/service/foo test_uri = uris.from_fs_path(os.path.join(root_path, doc_root, 'hello/test.py')) pyls.workspace.put_document(test_uri, 'assert true') From 8094a096774472707c68c242ba26520ddac2d810 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 06/10] Move plugins.jedi.extra_paths handling into Document.sys_path() Before this commit, plugins.jedi.extra_paths configuration is used only at execution of jedi.Script in Document.jedi_script(). Therefore, at spawning a process for other plugins (e.g pylint), such extra paths are ignored. It might causes unintentional failure of finding out libraries installed into those locations, even though Jedi can find out. After this commit, just using Document.sys_path() instead of sys.path is enough path configuration for any plugins. --- pyls/workspace.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pyls/workspace.py b/pyls/workspace.py index a39a468b..1c958cf9 100644 --- a/pyls/workspace.py +++ b/pyls/workspace.py @@ -230,15 +230,13 @@ def jedi_names(self, all_scopes=False, definitions=True, references=False): ) def jedi_script(self, position=None): - extra_paths = [] environment_path = None if self._config: jedi_settings = self._config.plugin_settings('jedi', document_path=self.path) environment_path = jedi_settings.get('environment') - extra_paths = jedi_settings.get('extra_paths') or [] - sys_path = self.sys_path(environment_path) + extra_paths + sys_path = self.sys_path(environment_path) environment = self.get_enviroment(environment_path) if environment_path else None kwargs = { @@ -272,4 +270,11 @@ def sys_path(self, environment_path=None): path = list(self._extra_sys_path) environment = self.get_enviroment(environment_path=environment_path) path.extend(environment.get_sys_path()) + + if self._config: + jedi_settings = self._config.plugin_settings('jedi', document_path=self.path) + extra_paths = jedi_settings.get('extra_paths') + if extra_paths: + path.extend(extra_paths) + return path From ff1a6c2bb297dc284c8486ffbab21c482431866f Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 07/10] Read plugins.jedi.extra_paths configuration also from files Before this commit, plugins.jedi.extra_paths can be configured only via Language Server Protocol didChangeConfiguration method. This is inconvenient, if: - it is difficult to issue LSP didChangeConfiguration method with per-workspace configuration - such configuration should be persisted inside workspace This commit reads plugins.jedi.extra_paths configuration also from project-level configuration "setup.cfg" and "tox.ini". --- pyls/config/pyls_conf.py | 4 +++ test/test_workspace.py | 59 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/pyls/config/pyls_conf.py b/pyls/config/pyls_conf.py index d6a92a6d..5436daf4 100644 --- a/pyls/config/pyls_conf.py +++ b/pyls/config/pyls_conf.py @@ -10,11 +10,15 @@ OPTIONS = [ ('source_roots', 'source_roots', list), + + # inter-plugins configurations + ('plugins.jedi.extra_paths', 'plugins.jedi.extra_paths', list), ] # list of (config_path, inside_only) tuples for path normalization NORMALIZED_CONFIGS = [ ('source_roots', True), + ('plugins.jedi.extra_paths', False), ] diff --git a/test/test_workspace.py b/test/test_workspace.py index 5da7e47c..d43c625d 100644 --- a/test/test_workspace.py +++ b/test/test_workspace.py @@ -251,3 +251,62 @@ def test_pyls_config_readin(tmpdir, metafile): for raw_path in source_roots: full_path = os.path.normcase(os.path.join(root_path, raw_path)) assert os.path.normpath(full_path) in sys_path + + +@pytest.mark.parametrize('metafile', [ + 'setup.cfg', + 'tox.ini', + 'service/foo/setup.cfg', + 'service/foo/tox.ini', + None, +]) +def test_jedi_extra_paths_config(tmpdir, metafile): + """Examine that plugins.jedi.extra_paths config is intentionaly read in. + + This test also examines below for entries in plugins.jedi.extra_paths: + + * relative path is: + - treated as relative to config file location, and + - normalized into absolute one + """ + root_path = str(tmpdir) + + extra_paths = ['extra/foo', 'extra/bar', '/absolute/root', '../baz'] + doc_root = 'service/foo' + + if metafile: + dirname = os.path.dirname(metafile) + if dirname: + os.makedirs(os.path.join(root_path, dirname)) + + # configured by metafile at pyls startup + with open(os.path.join(root_path, metafile), 'w+') as f: + f.write('[pyls]\nplugins.jedi.extra_paths=\n %s\n' % + ',\n '.join(_make_paths_dir_relative(extra_paths, dirname))) + + pyls = PythonLanguageServer(StringIO, StringIO) + pyls.m_initialize( + processId=1, + rootUri=uris.from_fs_path(root_path), + initializationOptions={} + ) + + if not metafile: + # configured by client via LSP after pyls startup + pyls.m_workspace__did_change_configuration({ + 'pyls': {'plugins': {'jedi': {'extra_paths': extra_paths}}}, + }) + + # put new document under ROOT/service/foo + test_uri = uris.from_fs_path(os.path.join(root_path, doc_root, 'hello/test.py')) + pyls.workspace.put_document(test_uri, 'assert true') + test_doc = pyls.workspace.get_document(test_uri) + + # apply os.path.normcase() on paths below, because case-sensitive + # comparison on Windows causes unintentional failure for case + # instability around drive letter + + sys_path = [os.path.normcase(p) for p in test_doc.sys_path()] + for raw_path in extra_paths: + full_path = os.path.normcase(os.path.join(root_path, raw_path)) + assert os.path.normpath(full_path) in sys_path From 5aee574a3c5eee3ce7335a453a6a434c76dfe9c7 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 08/10] Extract py_run of pylint.epylint as spawn_pylint This is the preparation for spawning pylint process with pyls specific customization in subsequent change. Almost all of spawn_pylint is cited from pylint/epylint.py in 6d818e3b84b853fe06f4199eb8408fe6dfe033be (the latest version on master branch at 2019-12-04) of pylint. BTW, this commit imports StringIO manually for compatibility between Python 2.x and 3.x instead of using "six" like recent pylint 1.9.x (supporting python 2.x), because requiring "six" only for StringIO seems over-kill. --- pyls/plugins/pylint_lint.py | 61 ++++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/pyls/plugins/pylint_lint.py b/pyls/plugins/pylint_lint.py index 9a412637..852d2f62 100644 --- a/pyls/plugins/pylint_lint.py +++ b/pyls/plugins/pylint_lint.py @@ -2,9 +2,11 @@ """Linter plugin for pylint.""" import collections import logging +import os +import shlex +import subprocess import sys -from pylint.epylint import py_run from pyls import hookimpl, lsp try: @@ -12,9 +14,55 @@ except Exception: # pylint: disable=broad-except import json +if sys.version_info.major == 2: + from StringIO import StringIO +else: + from io import StringIO + log = logging.getLogger(__name__) +def spawn_pylint(document, flags): + """Spawn pylint process as same as pylint.epylint.py_run + """ + path = document.path + if sys.platform.startswith('win'): + # Now, shlex.split is not applied on path, and Windows path + # (including '\\') is safe enough. But turn backslashes into + # forward slashes in order to keep previous behavior. + path = path.replace('\\', '/') + + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join(sys.path) + + # Detect if we use Python as executable or not, else default to `python` + executable = sys.executable if "python" in sys.executable else "python" + + # Create command line to call pylint + epylint_part = [executable, "-c", "from pylint import epylint;epylint.Run()"] + options = shlex.split(flags, posix=not sys.platform.startswith("win")) + + pylint_call = [path, '-f', 'json'] + options + log.debug("Calling pylint with %r", pylint_call) + + cli = epylint_part + pylint_call + + stdout = stderr = subprocess.PIPE + + # Call pylint in a subprocess + process = subprocess.Popen( + cli, + shell=False, + stdout=stdout, + stderr=stderr, + env=env, + universal_newlines=True, + ) + proc_stdout, proc_stderr = process.communicate() + # Return standard output and error + return StringIO(proc_stdout), StringIO(proc_stderr) + + class PylintLinter(object): last_diags = collections.defaultdict(list) @@ -56,16 +104,7 @@ def lint(cls, document, is_saved, flags=''): # save. 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('\\', '/') - - pylint_call = '{} -f json {}'.format(path, flags) - log.debug("Calling pylint with '%s'", pylint_call) - json_out, err = py_run(pylint_call, return_std=True) + json_out, err = spawn_pylint(document, flags) # Get strings json_out = json_out.getvalue() From 5a4b7802b311ccf79b9ea6a604971228fb37487b Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 09/10] Make pylint respect libraries installed into extra paths For example, jedi respects VIRTUAL_ENV environment variable at finding out libraries. Therefore, (virtualenv) runtime for pyls/jedi can be separated from one for the target workspace. On the other hand, pylint does not respect VIRTUAL_ENV, and might cause unintentional "import-error" (E0401) for libraries installed in such virtualenv, even though jedi can recognize them. In order to make pylint respect libraries installed into extra paths, this commit uses Document.sys_path() instead of sys.path of current pyls process, at spawning pylint. At this commit, Document.sys_path() should respect source roots in the workspace, VIRTUAL_ENV, PYTHONPATH, and plugins.jedi.extra_paths configuration. --- pyls/plugins/pylint_lint.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pyls/plugins/pylint_lint.py b/pyls/plugins/pylint_lint.py index 852d2f62..13836ac2 100644 --- a/pyls/plugins/pylint_lint.py +++ b/pyls/plugins/pylint_lint.py @@ -16,9 +16,19 @@ if sys.version_info.major == 2: from StringIO import StringIO + + # subprocess.Popen() on Windows expects that env contains only + # "str" keys/values (= "bytes" for Python2.x, "unicode" for + # Python3.x), even though pyls treats all path values as "unicode" + # regardless of Python version + def stringify(u): + return u.encode('utf-8') else: from io import StringIO + def stringify(u): + return u + log = logging.getLogger(__name__) @@ -33,7 +43,7 @@ def spawn_pylint(document, flags): path = path.replace('\\', '/') env = dict(os.environ) - env["PYTHONPATH"] = os.pathsep.join(sys.path) + env["PYTHONPATH"] = stringify(os.pathsep.join(document.sys_path())) # Detect if we use Python as executable or not, else default to `python` executable = sys.executable if "python" in sys.executable else "python" From 3715b28a609d95ac13b4928da7e2e7761f8097a3 Mon Sep 17 00:00:00 2001 From: FUJIWARA Katsunori Date: Fri, 17 Jan 2020 19:48:51 +0900 Subject: [PATCH 10/10] Add description about configuration in README.rst This commit describes about not only features added by previous commits but also some underlying specifications. --- README.rst | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/README.rst b/README.rst index 0648a8ff..4e949ae4 100644 --- a/README.rst +++ b/README.rst @@ -56,6 +56,98 @@ issue if you require assistance writing a plugin. Configuration ------------- +Location of Configuration File +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +There are three configuration levels below. + +* user-level configuration (e.g. ~/.config/CONFIGFILE) +* lsp-level configuration (via LSP didChangeConfiguration method) +* project-level configuration + +The latter level has priority over the former. + +As project-level configuration, configurations are read in from files +in the root of the workspace, by default. What files are read in is +described after. + +At evaluation of python source file ``foo/bar/baz/example.py`` for +example, if there is any configuration file in the ascendant directory +(i.e. ``foo``, ``foo/bar`` or ``foo/bar/baz``), it is read in before +evaluation. If multiple ascendant directories contain configuration +files, files only in the nearest ascendant directory are read in. + +In some cases, automatically discovered files are exclusive with files +in the root of the workspace. + +Syntax in Configuration File +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Configuration file should be written in "INI file" syntax. + +Value specified in configuration file should be one of types below. + +* bool +* int +* string +* list + +"List" value is string entries joined with comma. Both leading and +trailing white spaces of each entries in a "list" value are trimmed. + +Source Roots +~~~~~~~~~~~~ + +"Source roots" is determined in the order below. + +1. if ``pyls.source_roots`` (described after) is specified, its value + is used as "source roots" +2. if any of setup.py or pyproject.toml is found in the ascendant + directory of python source file at evaluation, that directory is + treated as "source roots" +3. otherwise, the root of the workspace is treated as "source roots" + +"Source roots" is used as a part of sys path at evaluation of python +source files. + +Python Language Server Specific Configuration +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +lsp-level and project-level configuration are supported for Python +Language Server specific configuration. + +For project-level configuration, setup.cfg and tox.ini are read in. +Configuration files discovered automatically at evaluation of python +source file are **not** exclusive with configuration files in the root +of the workspace. Files in both locations are read in, and a +configuration in the former files has priority over one in the latter. + +Python Language Server specific configurations are show below. + +* ``pyls.source_roots`` (list) to specify source roots +* ``pyls.plugins.jedi.extra_paths`` (list) to specify extra sys paths + +Relative path in these configurations is treated as relative to the +directory, in which configuration file exists. For configuration via +LSP didChangeConfiguration method, the root of the workspace is used +as base directory. + +Path in ``pyls.source_roots`` is ignored, if it refers outside of the +workspace. + +To make these configurations persisted into setup.cfg or tox.ini, +describe them under ``[pyls]`` section like below. + +.. code-block:: ini + + [pyls] + source_roots = services/foo, services/bar + plugins.jedi.extra_paths = ../extra_libs + + +Configuration at Source Code Evaluation +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + Configuration is loaded from zero or more configuration sources. Currently implemented are: * pycodestyle: discovered in ~/.config/pycodestyle, setup.cfg, tox.ini and pycodestyle.cfg. @@ -67,6 +159,10 @@ 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. +Configuration files discovered in the workspace automatically at +evaluation of python source file are exclusive with configuration +files in the root of the workspace. + To enable pydocstyle for linting docstrings add the following setting in your LSP configuration: ``` "pyls.plugins.pydocstyle.enabled": true