Fix crashes during toml configuration parsing

Add test for current pyproject.toml issues

Add a 'bad-configuration-option-value' message for bad toml configuration

We do not catch all the problem in toml because we don't know what
is expected so we can't recommend. See #5259

We can detect bad top level option when reading the toml

Author: Pierre-Sassoulas

ChangeLog

@@ -146,6 +146,14 @@ Release date: TBA
 
   Closes #5261
 
+* Crashes when a list is encountered in a toml configuration do not happen anymore. A new
+  ``bad-configuration-option-value`` checker was added that will emit for these problems.
+  Some bad configurations might still silently not be taken into account which is tracked in
+  a follow-up issue.
+
+  Closes #4580
+  Follow-up in #5259
+
 * Added new checker ``useless-with-lock`` to find incorrect usage of with statement and threading module locks.
   Emitted when ``with threading.Lock():`` is used instead of ``with lock_instance:``.
 

pylint/config/option_manager_mixin.py

@@ -271,6 +271,14 @@ def read_config_file(self, config_file=None, verbose=None):
 
         use_config_file = config_file and os.path.exists(config_file)
         if use_config_file:
+            mod_name = config_file
+            self.current_name = mod_name
+            try:
+                self.set_current_module(mod_name)
+            except AttributeError:
+                # 'OptionsManagerMixIn' object has no attribute 'set_current_module'
+                # Ie: the class is not used as a mixin
+                pass
             parser = self.cfgfile_parser
             if config_file.endswith(".toml"):
                 self._parse_toml(config_file, parser)
@@ -292,9 +300,8 @@ def read_config_file(self, config_file=None, verbose=None):
             msg = "No config file found, using default configuration"
         print(msg, file=sys.stderr)
 
-    @staticmethod
     def _parse_toml(
-        config_file: Union[Path, str], parser: configparser.ConfigParser
+        self, config_file: Union[Path, str], parser: configparser.ConfigParser
     ) -> None:
         """Parse and handle errors of a toml configuration file."""
         with open(config_file, encoding="utf-8") as fp:
@@ -304,23 +311,42 @@ def _parse_toml(
         except KeyError:
             return
         for section, values in sections_values.items():
+            section_name = section.upper()
             # TOML has rich types, convert values to
             # strings as ConfigParser expects.
+            if not isinstance(values, dict):
+                # This class is a mixin the add_message come from the pylinter
+                self.add_message(  # type: ignore
+                    "bad-configuration-option-value", line=0, args=(section, values)
+                )
+                continue
             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  # type: ignore
+                else:
+                    values[option] = str(value)
+            for option, value in values.items():
+                try:
+                    parser.set(section_name, option, value=value)
+                except configparser.NoSectionError:
+                    parser.add_section(section_name)
+                    parser.set(section_name, option, value=value)
 
     def load_config_file(self):
         """Dispatch values previously read from a configuration file to each
         options provider)"""
         parser = self.cfgfile_parser
         for section in parser.sections():
-            for option, value in parser.items(section):
+            try:
+                items = parser.items(section)
+            except ValueError:
+                self.add_message(
+                    "bad-configuration-option-value", line=0, args=(section, "?")
+                )
+                continue
+            for option, value in items:
                 try:
                     self.global_set_option(option, value)
                 except (KeyError, optparse.OptionError):

pylint/lint/pylinter.py

@@ -158,6 +158,11 @@ def _load_reporter_by_class(reporter_class: str) -> type:
         "bad-plugin-value",
         "Used when a bad value is used in 'load-plugins'.",
     ),
+    "E0014": (
+        "Bad top level configuration section name encountered '%s' : '%s'",
+        "bad-configuration-option-value",
+        "Used when a top section value can't be parsed probably because it does not exists.",
+    ),
 }
 
 

tests/config/file_to_lint.py

@@ -0,0 +1 @@
+"""Perfect module with only documentation for test_config_pyproject_toml.py"""

tests/config/issue_4580/basic_example.toml

@@ -0,0 +1,4 @@
+[tool.pylint."messages control"]
+disable = "logging-not-lazy,logging-format-interpolation"
+jobs = "10"
+reports = "yes"

tests/config/issue_4580/correct_basic_name_group.toml

@@ -0,0 +1,2 @@
+[tool.pylint.basic]
+name-group = { "a"="b" }

tests/config/issue_4580/correct_import_preferred_module.toml

@@ -0,0 +1,2 @@
+[tool.pylint.imports]
+preferred-modules = { "a"="b" }

tests/config/issue_4580/empty_list.toml

@@ -0,0 +1,2 @@
+[tool.pylint]
+load-plugins = []

tests/config/issue_4580/invalid_data_for_basic.toml

@@ -0,0 +1,4 @@
+[tool.pylint]
+# Both disable and load-plugins are not top level section
+load-plugins = []
+disable = "logging-not-lazy,logging-format-interpolation"

tests/config/issue_4580/invalid_data_for_import.toml

@@ -0,0 +1,7 @@
+# Both disable and load-plugins are not top level section
+[tool.pylint."imports"]
+disable = [
+    "logging-not-lazy",
+    "logging-format-interpolation",
+]
+preferred-modules = { "a"="b" }

tests/config/issue_4580/rich_types.toml

@@ -0,0 +1,7 @@
+[tool.pylint."messages control"]
+disable = [
+    "logging-not-lazy",
+    "logging-format-interpolation",
+]
+jobs = 10
+reports = true

tests/config/issue_4580/top_level_disable.toml

@@ -0,0 +1,2 @@
+[tool.pylint]
+disable = "logging-not-lazy,logging-format-interpolation"

tests/config/issue_4746/loaded_plugin_does_not_exists.toml

@@ -0,0 +1,8 @@
+# The pylint_websockets plugin does not exist and therefore this toml is invalid
+[tool.poe.tasks]
+docs = {cmd = "sphinx-build docs build", help = "Build documentation"}
+
+[tool.pylint.MASTER]
+load-plugins = 'pylint_websockets'
+
+format = ["black", "isort"]

tests/config/test_config.py

@@ -124,54 +124,3 @@ def test_read_config_file() -> None:
     jobs, jobs_nr = options_manager_mix_in.cfgfile_parser.items(section)[1]
     assert jobs == "jobs"
     assert jobs_nr == "10"
-
-
-def test_toml_with_empty_list_for_plugins(tmp_path):
-    # This would check that there is no crash for when empty lists
-    # passed as plugins , refer #4580
-    config_file = tmp_path / "pyproject.toml"
-    config_file.write_text(
-        """
-[tool.pylint]
-disable = "logging-not-lazy,logging-format-interpolation"
-load-plugins = []
-"""
-    )
-    with pytest.raises(AttributeError):
-        check_configuration_file_reader(config_file)
-
-
-def test_toml_with_invalid_data_for_imports(tmp_path):
-    # This would test config with invalid data for imports section
-    # refer #4580
-    config_file = tmp_path / "pyproject.toml"
-    config_file.write_text(
-        """
-[tool.pylint."imports"]
-disable = [
-    "logging-not-lazy",
-    "logging-format-interpolation",
-]
-preferred-modules = { "a"="b" }
-"""
-    )
-    with pytest.raises(AttributeError):
-        check_configuration_file_reader(config_file)
-
-
-def test_toml_with_invalid_data_for_basic(tmp_path):
-    # This would test config with invalid data for basic section
-    # refer #4580
-    config_file = tmp_path / "pyproject.toml"
-    config_file.write_text(
-        """
-[tool.pylint."basic"]
-disable = [
-    "logging-not-lazy",
-    "logging-format-interpolation",
-]
-name-group = { "a"="b" }
-"""
-    )
-    with pytest.raises(AttributeError):
-        check_configuration_file_reader(config_file)

tests/config/test_config_pyproject_toml.py

@@ -0,0 +1,70 @@
+from pathlib import Path
+from typing import List
+from unittest.mock import patch
+
+import pytest
+
+from pylint import run_pylint
+
+HERE = Path(__file__).parent
+
+
+@pytest.mark.parametrize(
+    "pyproject_toml_path,expected_results",
+    [
+        [
+            "issue_4580",
+            {
+                "basic_example.toml": {"code": 0},
+                "correct_basic_name_group.toml": {"code": 0},
+                "correct_import_preferred_module.toml": {"code": 0},
+                "empty_list.toml": {
+                    "code": 2,
+                    "out": "encountered 'load-plugins' : '[]'",
+                },
+                "invalid_data_for_basic.toml": {
+                    "code": 2,
+                    "out": "encountered 'load-plugins' : '[]'",
+                },
+                "invalid_data_for_import.toml": {"code": 0},
+                "rich_types.toml": {"code": 0},
+                "top_level_disable.toml": {
+                    "code": 2,
+                    "out": "encountered 'disable' : 'logging-not-lazy,",
+                },
+            },
+        ],
+        [
+            "issue_4746",
+            {
+                "loaded_plugin_does_not_exists.toml": {
+                    "code": 2,
+                    "out": "'pylint_websockets' is impossible to load",
+                }
+            },
+        ],
+    ],
+)
+def test_config_pyproject_toml(pyproject_toml_path, expected_results, capsys):
+    pyproject_toml_paths: List[Path] = (HERE / pyproject_toml_path).iterdir()
+    for toml_path in pyproject_toml_paths:
+        expected_result = expected_results[toml_path.name]
+        file_to_lint = str(HERE / "file_to_lint.py")
+        with patch(
+            "sys.argv",
+            ["pylint", "--rcfile", str(toml_path), file_to_lint, "-r", "n"],
+        ):
+            try:
+                run_pylint()
+            except SystemExit as ex:
+                out, err = capsys.readouterr()
+                msg = f"Wrong result with configuration {toml_path}"
+                if "out" not in expected_result:
+                    assert not out, msg
+                else:
+                    assert expected_result["out"] in out, msg
+                if "err" not in expected_result:
+                    assert not err, msg
+                else:
+                    assert expected_result["err"] in err, msg
+                assert ex.code == expected_result.get("code"), msg