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

Pylint Crash on invalid TOML config #4580

Closed
ShadowLNC opened this issue Jun 15, 2021 · 8 comments · Fixed by #4720
Closed

Pylint Crash on invalid TOML config #4580

ShadowLNC opened this issue Jun 15, 2021 · 8 comments · Fixed by #4720
Assignees
Labels
Configuration Related to configuration Crash 💥 A bug that makes pylint crash Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors
Milestone

Comments

@ShadowLNC
Copy link

Steps to reproduce

It was not immediately clear to me that the main table should be [tool.pylint.master] and not [tool.pylint] (I searched in the docs but did not find an example).

Given any python file and a config file called repro.toml (I initially encountered this on my pyproject.toml, but for the sake of a simple example):

[tool.pylint]
load-plugins = []

Current behavior

pylint --rcfile=repro.toml repro.py       
Traceback (most recent call last):
  File "C:\Program Files\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "D:\...\venv\Scripts\pylint.exe\__main__.py", line 7, in <module>
  File "d:\...\venv\lib\site-packages\pylint\__init__.py", line 24, in run_pylint
    PylintRun(sys.argv[1:])
  File "d:\...\venv\lib\site-packages\pylint\lint\run.py", line 316, in __init__
    linter.read_config_file(verbose=self.verbose)
  File "d:\...\venv\lib\site-packages\pylint\config\option_manager_mixin.py", line 281, in read_config_file
    for option, value in values.items():
AttributeError: 'str' object has no attribute 'items'

Expected behavior

It should not crash.

pylint --version output

I just upgraded to prerelease versions as suggested in this template.

pylint 3.0.0a3
astroid 2.5.8
Python 3.10.0b1 (tags/v3.10.0b1:ba42175, May  3 2021, 20:22:30) [MSC v.1928 64 bit (AMD64)]

Additional crashes for invalid data

Additional crashes are possible for "mapping" configs where I thought a TOML table might be used rather than a list of strings with a built in separator as is used in .pylintrc (again, I could not find examples).

Given config:

[tool.pylint.imports]
preferred-modules = { "a"="b" }

Or config:

[tool.pylint.basic]
name-group = { "a"="b" }

Results in (same traceback for both configs):

pylint --rcfile=repro.toml repro.py
Traceback (most recent call last):
  File "C:\Program Files\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Program Files\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "D:\...\venv\Scripts\pylint.exe\__main__.py", line 7, in <module>
  File "d:\...\venv\lib\site-packages\pylint\__init__.py", line 24, in run_pylint
    PylintRun(sys.argv[1:])
  File "d:\...\venv\lib\site-packages\pylint\lint\run.py", line 333, in __init__
    linter.load_config_file()
  File "d:\...\venv\lib\site-packages\pylint\config\option_manager_mixin.py", line 313, in load_config_file
    for option, value in parser.items(section):
  File "C:\Program Files\Python310\lib\configparser.py", line 860, in items
    return [(option, value_getter(option)) for option in orig_keys]
  File "C:\Program Files\Python310\lib\configparser.py", line 860, in <listcomp>
    return [(option, value_getter(option)) for option in orig_keys]
  File "C:\Program Files\Python310\lib\configparser.py", line 856, in <lambda>
    value_getter = lambda option: self._interpolation.before_get(self,
  File "C:\Program Files\Python310\lib\configparser.py", line 395, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "C:\Program Files\Python310\lib\configparser.py", line 412, in _interpolate_some
    p = rest.find("%")
AttributeError: 'DynamicInlineTableDict' object has no attribute 'find'

Related issues: #4518 #4204

@Pierre-Sassoulas Pierre-Sassoulas added Good first issue Friendly and approachable by new contributors Crash 💥 A bug that makes pylint crash Help wanted 🙏 Outside help would be appreciated, good for new contributors Configuration Related to configuration labels Jun 15, 2021
@tanvimoharir
Copy link
Contributor

Hi,
I would like to work on this. (I'm new to this).
Thanks

@Pierre-Sassoulas
Copy link
Member

Hello, thank you for willing to contribute to this issue ! I assigned you to it, do not hesitate to ask questions here or in a merge request.

@tanvimoharir
Copy link
Contributor

Hello, thank you for willing to contribute to this issue ! I assigned you to it, do not hesitate to ask questions here or in a merge request.

Thanks, I will start going through this and ask questions whenever I need to :)

@tanvimoharir
Copy link
Contributor

@Pierre-Sassoulas I was able to reproduce this, however I did this on
pylint 2.9.0-dev1
astroid 2.5.8
should the fix be done on the prerelease version (clarifying because the issue was raised with 3.0.0a3)

@Pierre-Sassoulas
Copy link
Member

3.0.0a3 is only 2.8.2 with some breaking changes on top of it, the version on pylint master branch (soon to be main branch) is the one that will be included in the next version (2.9.0), then 3.0.0a4 will be based on it.

@tanvimoharir
Copy link
Contributor

@Pierre-Sassoulas Sorry its taking me some time to understand this.
I was looking into the first case

[tool.pylint] 
load-plugins = []

The error I get is slightly different than above, I get:
AttributeError: 'list' object has no attribute 'items'

I also tried with non-empty list and got the same error (AttributeError: 'list' object has no attribute 'items')

[tool.pylint] 
load-plugins = ["a", "b"]

Its mentioned above that this should not be crashing, but what should the expected behavior be for this case?

I was thinking about something like (its not tested yet)this(https://github.com/PyCQA/pylint/blob/99589b08de8c5a2c6cc61e13a37420a868c80599/pylint/config/option_manager_mixin.py#L281)

                      if isinstance(values,dict): 
                          for option, value in values.items():
                                if isinstance(value, bool):
                                    values[option] = "yes" if value else "no"
                                elif isinstance(value, (int, float)):
                                    values[option] = str(value)
                                elif isinstance(value, list):
                                    values[option] = ",".join(value)
                            parser._sections[section.upper()] = values
                     else:
                           parse._sections[section.upper()] = "".join(values)

@Pierre-Sassoulas
Copy link
Member

I think what the code expect in values is a dict so probably {"load-plugins": ["a", "b"]} and not ["a", "b"] ? Ideally the parsing should be the same for TOML or pylintrc, it's possible that the way it's done right now is not ideal. You can refactor to make it cleaner if you have to.

@tanvimoharir
Copy link
Contributor

tanvimoharir commented Jul 19, 2021

@Pierre-Sassoulas section_values reads {"load-plugins": ["a", "b"]} but then we break it down further into option, value where it breaks. I went ahead with a check as commented above. (I have raised a draft PR for now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configuration Related to configuration Crash 💥 A bug that makes pylint crash Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants