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

Crash when pyproject.toml disable = [ "array", "of", "items" ] #3538

Closed
bruceadams opened this issue Apr 28, 2020 · 1 comment · Fixed by #3616
Closed

Crash when pyproject.toml disable = [ "array", "of", "items" ] #3538

bruceadams opened this issue Apr 28, 2020 · 1 comment · Fixed by #3616
Labels
Bug 🪲 Configuration Related to configuration

Comments

@bruceadams
Copy link

Steps to reproduce

  1. Specify an array of messages to disable in pyproject.toml
  2. Run pylint

Current behavior

Crash parsing pyproject.toml:

$ pylint sample
Traceback (most recent call last):
  File "/Users/bruce/Library/Caches/pypoetry/virtualenvs/ifb-partner-api-AO4b-1bH-py3.8/bin/pylint", line 10, in <module>
    sys.exit(run_pylint())
  File "/Users/bruce/Library/Caches/pypoetry/virtualenvs/ifb-partner-api-AO4b-1bH-py3.8/lib/python3.8/site-packages/pylint/__init__.py", line 22, in run_pylint
    PylintRun(sys.argv[1:])
  File "/Users/bruce/Library/Caches/pypoetry/virtualenvs/ifb-partner-api-AO4b-1bH-py3.8/lib/python3.8/site-packages/pylint/lint/run.py", line 300, in __init__
    linter.load_config_file()
  File "/Users/bruce/Library/Caches/pypoetry/virtualenvs/ifb-partner-api-AO4b-1bH-py3.8/lib/python3.8/site-packages/pylint/config.py", line 785, in load_config_file
    for option, value in parser.items(section):
  File "/usr/local/Cellar/python@3.8/3.8.2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/configparser.py", line 859, in items
    return [(option, value_getter(option)) for option in orig_keys]
  File "/usr/local/Cellar/python@3.8/3.8.2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/configparser.py", line 859, in <listcomp>
    return [(option, value_getter(option)) for option in orig_keys]
  File "/usr/local/Cellar/python@3.8/3.8.2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/configparser.py", line 855, in <lambda>
    value_getter = lambda option: self._interpolation.before_get(self,
  File "/usr/local/Cellar/python@3.8/3.8.2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/configparser.py", line 395, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/usr/local/Cellar/python@3.8/3.8.2/Frameworks/Python.framework/Versions/3.8/lib/python3.8/configparser.py", line 412, in _interpolate_some
    p = rest.find("%")
AttributeError: 'list' object has no attribute 'find'
[tool.poetry]
name = "fake"
version = "0.1.0"
description = "dummy"
authors = ["Bruce Adams <bruce.adams@acm.org>"]

[tool.poetry.dependencies]
python = "^3.8"
connexion = { extras = ["swagger-ui"], version = "^2.7.0" }
okta_jwt = "^1.3.5"

[tool.poetry.dev-dependencies]
black = "^19.10b0"
fire = "^0.3.1"
mypy = "^0.770"
pyjwt = { extras = ["crypto"], version = "^1.7.1" }
pylint = "^2.5.0"
pytest = "^5.4.1"
pytest-cov = "^2.8.1"

[tool.pylint."MESSAGES CONTROL"]
# bad-continuation - pylint sometimes complains about black's formatting; we go with black
# logging-format-interpolation, logging-fstring-interpolation, logging-not-lazy
# - pylint wants deferred formatting for log messages (to use less CPU if that
#   log level is disabled). We prefer the legibility of f-strings: f"a is {a}"
disable = [
    "bad-continuation",
    "logging-format-interpolation",
    "logging-fstring-interpolation",
    "logging-not-lazy"
]

[build-system]
requires = ["poetry>=1.0"]
build-backend = "poetry.masonry.api"

Expected behavior

What I hope for is that the pyproject.toml above behaves the same as when the disable line looks like this.

disable = "bad-continuation,logging-format-interpolation,logging-fstring-interpolation,logging-not-lazy"

pylint --version output

$ pylint --version
pylint 2.5.0
astroid 2.4.0
Python 3.8.2 (default, Mar 11 2020, 00:29:50)
[Clang 11.0.0 (clang-1100.0.33.17)]

Additional information

I tried adding a test for the behavior I want into https://github.com/PyCQA/pylint/blob/master/tests/test_config.py I was very surprised to have that test pass!

I can add that test in a PR, if you'd like. Here it is, just for reference:

import unittest.mock

import pylint.lint


def test_can_read_toml(tmp_path):
    config_file = tmp_path / "pyproject.toml"
    config_file.write_text(
        "[tool.pylint.'messages control']\n"
        "disable='logging-format-interpolation,logging-fstring-interpolation'\n"
        "enable='missing-module-docstring'\n"
        "jobs=10\n"
    )
    linter = pylint.lint.PyLinter()
    linter.global_set_option = unittest.mock.MagicMock()
    linter.read_config_file(str(config_file))

    assert linter.global_set_option.called_with(
        "disable", "logging-format-interpolation"
    )
    assert linter.global_set_option.called_with(
        "disable", "logging-fstring-interpolation"
    )
    assert linter.global_set_option.called_with("enable", "missing-module-docstring")
    assert linter.global_set_option.called_with("jobs", 10)


def test_can_read_toml_array(tmp_path):
    config_file = tmp_path / "pyproject.toml"
    config_file.write_text(
        '[tool.pylint."messages control"]\n'
        'disable=[\n"logging-format-interpolation",\n"logging-fstring-interpolation"\n]\n'
        'enable="missing-module-docstring"\n'
        "jobs=10\n"
    )
    linter = pylint.lint.PyLinter()
    linter.global_set_option = unittest.mock.MagicMock()
    linter.read_config_file(str(config_file))

    assert linter.global_set_option.called_with(
        "disable", "logging-format-interpolation"
    )
    assert linter.global_set_option.called_with(
        "disable", "logging-fstring-interpolation"
    )
    assert linter.global_set_option.called_with("enable", "missing-module-docstring")
    assert linter.global_set_option.called_with("jobs", 10)


def test_can_read_setup_cfg(tmp_path):
    config_file = tmp_path / "setup.cfg"
    config_file.write_text(
        "[pylint.messages control]\n"
        "disable=all\n"
        "enable=missing-module-docstring\n"
        "jobs=10\n"
    )
    linter = pylint.lint.PyLinter()
    linter.global_set_option = unittest.mock.MagicMock()
    linter.read_config_file(str(config_file))

    assert linter.global_set_option.called_with("disable", "all")
    assert linter.global_set_option.called_with("enable", "missing-module-docstring")
    assert linter.global_set_option.called_with("jobs", 10)
@dbaty
Copy link
Contributor

dbaty commented May 12, 2020

Hi @bruceadams, I tried to tackle this: see pull request #3616. Thanks for providing the test above, I might have overlooked the existing test file. I'm sure that saved me some time, thank you. :)

By the way, your test (and the existing tests it was based on) did not fail because they used MagicMock, which says "yes" to anything you throw at it. You could have said linter.global_set_option.called_with("whatever", "really", "anything") and it would still pass. That's a bit too much magic. :)

PCManticore pushed a commit that referenced this issue May 16, 2020
Fixes #3538

Before that, we had to use strings in a TOML configuration file, like
this:

    enable = "use-symbolic-message-instead,useless-suppression"
    jobs = "10"
    suggestion-mode = "no"

TOML supports rich types like list, integer and boolean. They make for
a more readable and less error-prone file. We can now express the same
configuration like this:

    enable = [
        "use-symbolic-message-instead",
        "useless-suppression",
    ]
    jobs = 10
    suggestion-mode = false
PCManticore pushed a commit that referenced this issue May 16, 2020
Fixes #3538

Before that, we had to use strings in a TOML configuration file, like
this:

    enable = "use-symbolic-message-instead,useless-suppression"
    jobs = "10"
    suggestion-mode = "no"

TOML supports rich types like list, integer and boolean. They make for
a more readable and less error-prone file. We can now express the same
configuration like this:

    enable = [
        "use-symbolic-message-instead",
        "useless-suppression",
    ]
    jobs = 10
    suggestion-mode = false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Configuration Related to configuration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants