Skip to content

Commit

Permalink
Read pycodestyle and flake8 configurations per workspace (#827)
Browse files Browse the repository at this point in the history
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
  • Loading branch information
andfoy and ccordoba12 authored Jun 30, 2020
1 parent a72704f commit 4646633
Show file tree
Hide file tree
Showing 13 changed files with 141 additions and 102 deletions.
18 changes: 10 additions & 8 deletions pyls/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,29 +106,31 @@ def settings(self, document_path=None):
settings = {}
sources = self._settings.get('configurationSources', DEFAULT_CONFIG_SOURCES)

# Plugin configuration
settings = _utils.merge_dicts(settings, self._plugin_settings)

# LSP configuration
settings = _utils.merge_dicts(settings, self._settings)

# User configuration
for source_name in reversed(sources):
source = self._config_sources.get(source_name)
if not source:
continue
source_conf = source.user_config()
log.debug("Got user config from %s: %s", source.__class__.__name__, source_conf)
settings = _utils.merge_dicts(settings, source_conf)
log.debug("With user configuration: %s", settings)

settings = _utils.merge_dicts(settings, self._plugin_settings)
log.debug("With plugin configuration: %s", settings)

settings = _utils.merge_dicts(settings, self._settings)
log.debug("With lsp configuration: %s", settings)

# Project configuration
for source_name in reversed(sources):
source = self._config_sources.get(source_name)
if not source:
continue
source_conf = source.project_config(document_path or self._root_path)
log.debug("Got project config from %s: %s", source.__class__.__name__, source_conf)
settings = _utils.merge_dicts(settings, source_conf)
log.debug("With project configuration: %s", settings)

log.debug("With configuration: %s", settings)

return settings

Expand Down
3 changes: 2 additions & 1 deletion pyls/plugins/flake8_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def pyls_settings():


@hookimpl
def pyls_lint(config, document):
def pyls_lint(workspace, document):
config = workspace._config
settings = config.plugin_settings('flake8')
log.debug("Got flake8 settings: %s", settings)

Expand Down
3 changes: 2 additions & 1 deletion pyls/plugins/pycodestyle_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@


@hookimpl
def pyls_lint(config, document):
def pyls_lint(workspace, document):
config = workspace._config
settings = config.plugin_settings('pycodestyle')
log.debug("Got pycodestyle settings: %s", settings)

Expand Down
16 changes: 12 additions & 4 deletions pyls/python_ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def m_workspace__did_change_configuration(self, settings=None):
self.config.update((settings or {}).get('pyls', {}))
for workspace_uri in self.workspaces:
workspace = self.workspaces[workspace_uri]
workspace.update_config(self.config)
workspace.update_config(settings)
for doc_uri in workspace.documents:
self.lint(doc_uri, is_saved=False)

Expand All @@ -376,23 +376,31 @@ def m_workspace__did_change_workspace_folders(self, event=None, **_kwargs): # p
for added_info in added:
if 'uri' in added_info:
added_uri = added_info['uri']
self.workspaces[added_uri] = Workspace(added_uri, self._endpoint, self.config)
workspace_config = config.Config(
added_uri, self.config._init_opts,
self.config._process_id, self.config._capabilities)
self.workspaces[added_uri] = Workspace(
added_uri, self._endpoint, workspace_config)

root_workspace_removed = any(removed_info['uri'] == self.root_uri for removed_info in removed)
workspace_added = len(added) > 0 and 'uri' in added[0]
if root_workspace_removed and workspace_added:
added_uri = added[0]['uri']
self.root_uri = added_uri
self.workspace = self.workspaces[added_uri]
new_root_workspace = self.workspaces[added_uri]
self.config = new_root_workspace._config
self.workspace = new_root_workspace
elif root_workspace_removed:
# NOTE: Removing the root workspace can only happen when the server
# is closed, thus the else condition of this if can never happen.
if self.workspaces:
log.debug('Root workspace deleted!')
available_workspaces = sorted(self.workspaces)
first_workspace = available_workspaces[0]
new_root_workspace = self.workspaces[first_workspace]
self.root_uri = first_workspace
self.workspace = self.workspaces[first_workspace]
self.config = new_root_workspace._config
self.workspace = new_root_workspace

# Migrate documents that are on the root workspace and have a better
# match now
Expand Down
20 changes: 11 additions & 9 deletions pyls/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ def update_document(self, doc_uri, change, version=None):
self._docs[doc_uri].apply_change(change)
self._docs[doc_uri].version = version

def update_config(self, config):
self._config = config
def update_config(self, settings):
self._config.update((settings or {}).get('pyls', {}))
for doc_uri in self.documents:
self.get_document(doc_uri).update_config(config)
self.get_document(doc_uri).update_config(settings)

def apply_edit(self, edit):
return self._endpoint.request(self.M_APPLY_EDIT, {'edit': edit})
Expand All @@ -106,23 +106,25 @@ def source_roots(self, document_path):
def _create_document(self, doc_uri, source=None, version=None):
path = uris.to_fs_path(doc_uri)
return Document(
doc_uri, self, source=source, version=version,
doc_uri,
self,
source=source,
version=version,
extra_sys_path=self.source_roots(path),
rope_project_builder=self._rope_project_builder,
config=self._config,
)


class Document(object):

def __init__(self, uri, workspace, source=None, version=None, local=True, extra_sys_path=None,
rope_project_builder=None, config=None):
rope_project_builder=None):
self.uri = uri
self.version = version
self.path = uris.to_fs_path(uri)
self.filename = os.path.basename(self.path)

self._config = config
self._config = workspace._config
self._workspace = workspace
self._local = local
self._source = source
Expand All @@ -147,8 +149,8 @@ def source(self):
return f.read()
return self._source

def update_config(self, config):
self._config = config
def update_config(self, settings):
self._config.update((settings or {}).get('pyls', {}))

def apply_change(self, change):
"""Apply a change to the document."""
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def pyls(tmpdir):
@pytest.fixture
def workspace(tmpdir):
"""Return a workspace."""
return Workspace(uris.from_fs_path(str(tmpdir)), Mock())
ws = Workspace(uris.from_fs_path(str(tmpdir)), Mock())
ws._config = Config(ws.root_uri, {}, 0, {})
return ws


@pytest.fixture
Expand Down
27 changes: 13 additions & 14 deletions test/plugins/test_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import os
import sys

from test.test_utils import MockWorkspace
import pytest

from pyls import uris, lsp
Expand Down Expand Up @@ -278,7 +277,7 @@ def test_multistatement_snippet(config, workspace):
assert completions[0]['insertText'] == 'date(${1:year}, ${2:month}, ${3:day})$0'


def test_jedi_completion_extra_paths(config, tmpdir, workspace):
def test_jedi_completion_extra_paths(tmpdir, workspace):
# Create a tempfile with some content and pass to extra_paths
temp_doc_content = '''
def spam():
Expand All @@ -296,42 +295,42 @@ def spam():

# After 'foo.s' without extra paths
com_position = {'line': 1, 'character': 5}
completions = pyls_jedi_completions(config, doc, com_position)
completions = pyls_jedi_completions(doc._config, doc, com_position)
assert completions is None

# Update config extra paths
config.update({'plugins': {'jedi': {'extra_paths': extra_paths}}})
doc.update_config(config)
settings = {'pyls': {'plugins': {'jedi': {'extra_paths': extra_paths}}}}
doc.update_config(settings)

# After 'foo.s' with extra paths
com_position = {'line': 1, 'character': 5}
completions = pyls_jedi_completions(config, doc, com_position)
completions = pyls_jedi_completions(doc._config, doc, com_position)
assert completions[0]['label'] == 'spam()'


@pytest.mark.skipif(PY2 or not LINUX or not CI, reason="tested on linux and python 3 only")
def test_jedi_completion_environment(config):
def test_jedi_completion_environment(workspace):
# Content of doc to test completion
doc_content = '''import logh
'''
doc = Document(DOC_URI, MockWorkspace(), doc_content)
doc = Document(DOC_URI, workspace, doc_content)

# After 'import logh' with default environment
com_position = {'line': 0, 'character': 11}

assert os.path.isdir('/tmp/pyenv/')

config.update({'plugins': {'jedi': {'environment': None}}})
doc.update_config(config)
completions = pyls_jedi_completions(config, doc, com_position)
settings = {'pyls': {'plugins': {'jedi': {'environment': None}}}}
doc.update_config(settings)
completions = pyls_jedi_completions(doc._config, doc, com_position)
assert completions is None

# Update config extra environment
env_path = '/tmp/pyenv/bin/python'
config.update({'plugins': {'jedi': {'environment': env_path}}})
doc.update_config(config)
settings = {'pyls': {'plugins': {'jedi': {'environment': env_path}}}}
doc.update_config(settings)

# After 'import logh' with new environment
completions = pyls_jedi_completions(config, doc, com_position)
completions = pyls_jedi_completions(doc._config, doc, com_position)
assert completions[0]['label'] == 'loghub'
assert 'changelog generator' in completions[0]['documentation'].lower()
23 changes: 11 additions & 12 deletions test/plugins/test_flake8_lint.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright 2019 Palantir Technologies, Inc.
import tempfile
import os
from test.test_utils import MockWorkspace
from mock import patch
from pyls import lsp, uris
from pyls.plugins import flake8_lint
Expand All @@ -19,29 +18,29 @@ def using_const():
"""


def temp_document(doc_text):
def temp_document(doc_text, workspace):
temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False)
name = temp_file.name
temp_file.write(doc_text)
temp_file.close()
doc = Document(uris.from_fs_path(name), MockWorkspace())
doc = Document(uris.from_fs_path(name), workspace)

return name, doc


def test_flake8_no_checked_file(config, workspace):
def test_flake8_no_checked_file(workspace):
# A bad uri or a non-saved file may cause the flake8 linter to do nothing.
# In this situtation, the linter will return an empty list.

doc = Document('', workspace, DOC)
diags = flake8_lint.pyls_lint(config, doc)
diags = flake8_lint.pyls_lint(workspace, doc)
assert 'Error' in diags[0]['message']


def test_flake8_lint(config):
def test_flake8_lint(workspace):
try:
name, doc = temp_document(DOC)
diags = flake8_lint.pyls_lint(config, doc)
name, doc = temp_document(DOC, workspace)
diags = flake8_lint.pyls_lint(workspace, doc)
msg = 'local variable \'a\' is assigned to but never used'
unused_var = [d for d in diags if d['message'] == msg][0]

Expand All @@ -55,14 +54,14 @@ def test_flake8_lint(config):
os.remove(name)


def test_flake8_config_param(config):
def test_flake8_config_param(workspace):
with patch('pyls.plugins.flake8_lint.Popen') as popen_mock:
mock_instance = popen_mock.return_value
mock_instance.communicate.return_value = [bytes(), bytes()]
flake8_conf = '/tmp/some.cfg'
config.update({'plugins': {'flake8': {'config': flake8_conf}}})
_name, doc = temp_document(DOC)
flake8_lint.pyls_lint(config, doc)
workspace._config.update({'plugins': {'flake8': {'config': flake8_conf}}})
_name, doc = temp_document(DOC, workspace)
flake8_lint.pyls_lint(workspace, doc)
call_args = popen_mock.call_args.args[0]
assert 'flake8' in call_args
assert '--config={}'.format(flake8_conf) in call_args
16 changes: 7 additions & 9 deletions test/plugins/test_pycodestyle_lint.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright 2017 Palantir Technologies, Inc.
import os
from pyls import lsp, uris
from pyls.config.config import Config
from pyls.workspace import Document
from pyls.plugins import pycodestyle_lint

Expand All @@ -20,9 +19,9 @@ def hello( ):
"""


def test_pycodestyle(config, workspace):
def test_pycodestyle(workspace):
doc = Document(DOC_URI, workspace, DOC)
diags = pycodestyle_lint.pyls_lint(config, doc)
diags = pycodestyle_lint.pyls_lint(workspace, doc)

assert all([d['source'] == 'pycodestyle' for d in diags])

Expand Down Expand Up @@ -78,10 +77,9 @@ 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, {})

# Make sure we get a warning for 'indentation contains tabs'
diags = pycodestyle_lint.pyls_lint(config, doc)
diags = pycodestyle_lint.pyls_lint(workspace, doc)
assert [d for d in diags if d['code'] == 'W191']

content = {
Expand All @@ -93,20 +91,20 @@ def test_pycodestyle_config(workspace):
# Now we'll add config file to ignore it
with open(os.path.join(workspace.root_path, conf_file), 'w+') as f:
f.write(content)
config.settings.cache_clear()
workspace._config.settings.cache_clear()

# And make sure we don't get any warnings
diags = pycodestyle_lint.pyls_lint(config, doc)
diags = pycodestyle_lint.pyls_lint(workspace, doc)
assert len([d for d in diags if d['code'] == 'W191']) == (0 if working else 1)
assert len([d for d in diags if d['code'] == 'E201']) == (0 if working else 1)
assert [d for d in diags if d['code'] == 'W391']

os.unlink(os.path.join(workspace.root_path, conf_file))

# Make sure we can ignore via the PYLS config as well
config.update({'plugins': {'pycodestyle': {'ignore': ['W191', 'E201']}}})
workspace._config.update({'plugins': {'pycodestyle': {'ignore': ['W191', 'E201']}}})
# And make sure we only get one warning
diags = pycodestyle_lint.pyls_lint(config, doc)
diags = pycodestyle_lint.pyls_lint(workspace, doc)
assert not [d for d in diags if d['code'] == 'W191']
assert not [d for d in diags if d['code'] == 'E201']
assert [d for d in diags if d['code'] == 'W391']
Loading

0 comments on commit 4646633

Please sign in to comment.