Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for pylint config files #673

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pyls/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
log = logging.getLogger(__name__)

# Sources of config, first source overrides next source
DEFAULT_CONFIG_SOURCES = ['pycodestyle']
DEFAULT_CONFIG_SOURCES = ['pycodestyle', 'pylint']


class Config(object):
Expand All @@ -39,6 +39,11 @@ def __init__(self, root_uri, init_opts, process_id, capabilities):
self._config_sources['pycodestyle'] = PyCodeStyleConfig(self._root_path)
except ImportError:
pass
try:
from .pylint_conf import PylintConfig
self._config_sources['pylint'] = PylintConfig(self._root_path)
except ImportError:
pass

self._pm = pluggy.PluginManager(PYLS)
self._pm.trace.root.setwriter(log.debug)
Expand Down
53 changes: 53 additions & 0 deletions pyls/config/pylint_conf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Copyright 2019 Palantir Technologies, Inc.
import logging
import os
from pyls._utils import find_parents
from .source import ConfigSource, _get_opt, _set_opt

log = logging.getLogger(__name__)

PROJECT_CONFIGS = ['.pylintrc', 'pylintrc']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are only using the first 2 options?

https://docs.pylint.org/en/1.6.0/run.html

pylintrc in the current working directory
.pylintrc in the current working directory
If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a pylintrc file. This allows you to specify coding standards on a module-by-module basis. Of course, a directory is judged to be a Python module if it contains an __init__.py file.
The file named by environment variable PYLINTRC
if you have a home directory which isn’t /root:
.pylintrc in your home directory
.config/pylintrc in your home directory
/etc/pylintrc


CONFIG_KEYS = { # 'option': 'section key'
'disable': 'MESSAGES CONTROL',
'ignore': 'MASTER',
'max-line-length': 'FORMAT',
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@goanpeca @youben11 From all of pylint's configuration options, we only extract three fields and when the document is passed to pylint, pylintrc is ignored.

  • Why is there a need to extract configuration at all?
  • Why are we not calling pylint in a way that it itself looks for its configuration?


OPTIONS = [
('disable', 'plugins.pylint.disable', list),
('ignore', 'plugins.pylint.ignore', list),
('max-line-length', 'plugins.pylint.maxLineLength', int),
]


class PylintConfig(ConfigSource):
"""Parse pylint configurations."""

def user_config(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add a docstring with some explanation. Also maybe saying what the returned object is.

config_file = self._user_config_file()
config = self.read_config_from_files([config_file])
return self.parse_config_multi_keys(config, CONFIG_KEYS, OPTIONS)

def _user_config_file(self):
if self.is_windows:
Copy link
Contributor

@goanpeca goanpeca Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to make this special case for windows?

I was expecting this os.path.expanduser('~/.pylintrc') to work on all osses

return os.path.expanduser('~\\.pylintrc')
return os.path.expanduser('~/.pylintrc')

def project_config(self, document_path):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add a docstring with some explanation. Also maybe saying what the returned object is.

files = find_parents(self.root_path, document_path, PROJECT_CONFIGS)
config = self.read_config_from_files(files)
return self.parse_config_multi_keys(config, CONFIG_KEYS, OPTIONS)

@staticmethod
def parse_config_multi_keys(config, keys, options):
"""Parse the config with the given options.
This method use multiple keys depending on the value we want to get.
"""
conf = {}
for source, destination, opt_type in options:
key = keys[source]
opt_value = _get_opt(config, key, source, opt_type)
if opt_value is not None:
_set_opt(conf, destination, opt_value)
return conf
24 changes: 22 additions & 2 deletions pyls/plugins/pylint_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@

log = logging.getLogger(__name__)

ARGS = { # 'argument_name': 'name_under_plugin_conf'
'disable': 'disable',
'ignore': 'ignore',
'max-line-length': 'maxLineLength',
}


class PylintLinter(object):
last_diags = collections.defaultdict(list)
Expand Down Expand Up @@ -140,10 +146,24 @@ def lint(cls, document, is_saved, flags=''):


def _build_pylint_flags(settings):
"""Build arguments for calling pylint."""
"""Build arguments for calling pylint.
If args is found then it's the arguments used, otherwise,
we build arguments from the plugin config.
"""
pylint_args = settings.get('args')
if pylint_args is None:
return ''
# Build args from plugin config
pylint_args = list()
for arg_name in ARGS:
arg_val = settings.get(ARGS[arg_name])
arg = None
if isinstance(arg_val, list):
arg = '--{}={}'.format(arg_name, ','.join(arg_val))
elif isinstance(arg_val, int):
arg = '--{}={}'.format(arg_name, arg_val)
if arg:
pylint_args.append(arg)

return ' '.join(pylint_args)


Expand Down
2 changes: 1 addition & 1 deletion vscode-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
},
"pyls.configurationSources": {
"type": "array",
"default": ["pycodestyle"],
"default": ["pycodestyle", "pylint"],
"description": "List of configuration sources to use.",
"items": {
"type": "string",
Expand Down