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 diff --git a/pyls/_utils.py b/pyls/_utils.py index 919bf1c5..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 @@ -132,6 +174,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. diff --git a/pyls/config/config.py b/pyls/config/config.py index 65696d81..fe6072ba 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) @@ -141,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/pyls_conf.py b/pyls/config/pyls_conf.py new file mode 100644 index 00000000..5436daf4 --- /dev/null +++ b/pyls/config/pyls_conf.py @@ -0,0 +1,93 @@ +import logging +import os +from pyls._utils import find_parents, get_config_by_path, merge_dicts, normalize_paths +from .source import ConfigSource, _set_opt + +log = logging.getLogger(__name__) + +CONFIG_KEY = 'pyls' +PROJECT_CONFIGS = ['setup.cfg', 'tox.ini'] + +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), +] + + +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 + + self.normalize(parsed, os.path.dirname(files[0])) + + settings = merge_dicts(settings, parsed) + + 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) + 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/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() diff --git a/pyls/plugins/pylint_lint.py b/pyls/plugins/pylint_lint.py index 9a412637..13836ac2 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,65 @@ except Exception: # pylint: disable=broad-except import json +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__) +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"] = 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" + + # 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 +114,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() diff --git a/pyls/workspace.py b/pyls/workspace.py index a58b76a2..1c958cf9 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] @@ -225,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 = { @@ -267,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 diff --git a/test/test_workspace.py b/test/test_workspace.py index 9b5b7b06..d43c625d 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,191 @@ 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', + None, +]) +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={} + ) + + 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') + 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 + + +@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 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": [],