Pylint fix for invalid TOML config (#4720)

* Fix crashes during toml configuration parsing

Add test for current 'pyproject.toml' issues. Add a 'bad-configuration-section'
message for bad toml configuration

We can detect bad top level option when reading the toml but we do not catch
all the problem in toml because we don't know what is expected so we can't
recommend. See #5259

Co-authored-by: Daniël van Noord <[email protected]>

Author: tanvimoharir

CONTRIBUTORS.txt

@@ -529,6 +529,9 @@ contributors:
 
 * Marcin Kurczewski (rr-): contributor
 
+* Tanvi Moharir: contributor
+  - Fix for invalid toml config
+
 * Eisuke Kawashima (e-kwsm): contributor
 
 * Daniel van Noord (DanielNoord): contributor

ChangeLog

@@ -158,6 +158,17 @@ Release date: TBA
 
   Closes #5261
 
+* Crashes when a list is encountered in a toml configuration do not happen anymore.
+
+  Closes #4580
+
+* A new ``bad-configuration-section`` checker was added that will emit for misplaced option
+  in pylint's top level namespace for toml configuration. Top-level dictionaries or option defined
+  in the wrong section will still silently not be taken into account, which is tracked in a
+  follow-up issue.
+
+  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:``.
 

doc/whatsnew/2.12.rst

@@ -56,6 +56,13 @@ New checkers
 
   Closes #5208
 
+* A new ``bad-configuration-section`` checker was added that will emit for misplaced option
+  in pylint's top level namespace for toml configuration. Top-level dictionaries or option defined
+  in the wrong section will still silently not be taken into account, which is tracked in a
+  follow-up issue.
+
+  Follow-up in #5259
+
 Removed checkers
 ================
 
@@ -162,4 +169,8 @@ Other Changes
 
   Closes #5216
 
+* Crashes when a list is encountered in a toml configuration do not happen anymore.
+
+  Closes #4580
+
 * Make yn validator case insensitive, to allow for ``True`` and ``False`` in config files.

pylint/config/option_manager_mixin.py

@@ -293,9 +293,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:
@@ -305,16 +304,28 @@ 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: add_message comes from the `PyLinter` class
+                self.add_message(  # type: ignore
+                    "bad-configuration-section", 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

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": (
+        "Out-of-place setting encountered in top level configuration-section '%s' : '%s'",
+        "bad-configuration-section",
+        "Used when we detect a setting in the top level of a toml configuration that shouldn't be there.",
+    ),
 }
 
 

tests/config/functional/toml/issue_4580/correct_basic_name_group.toml

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

tests/config/functional/toml/issue_4580/correct_import_preferred_module.toml

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

tests/config/functional/toml/issue_4580/empty_list.out

@@ -0,0 +1,2 @@
+************* Module {abspath}
+{relpath}:1:0: E0014: Out-of-place setting encountered in top level configuration-section 'load-plugins' : '[]' (bad-configuration-section)

tests/config/functional/toml/issue_4580/empty_list.toml

@@ -0,0 +1,3 @@
+[tool.pylint]
+# load-plugins does not belong in top level section
+load-plugins = []

tests/config/functional/toml/issue_4580/invalid_data_for_basic.out

@@ -0,0 +1,3 @@
+************* Module {abspath}
+{relpath}:1:0: E0014: Out-of-place setting encountered in top level configuration-section 'load-plugins' : '[]' (bad-configuration-section)
+{relpath}:1:0: E0014: Out-of-place setting encountered in top level configuration-section 'disable' : 'logging-not-lazy,logging-format-interpolation' (bad-configuration-section)

tests/config/functional/toml/issue_4580/invalid_data_for_basic.toml

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

tests/config/functional/toml/issue_4580/invalid_data_for_import.result.json

@@ -0,0 +1,5 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  }
+}

tests/config/functional/toml/issue_4580/invalid_data_for_import.toml

@@ -0,0 +1,8 @@
+# Both disable and load-plugins do not belong in the imports section
+# TODO This should be fixed in #5259
+[tool.pylint."imports"]
+disable = [
+    "logging-not-lazy",
+    "logging-format-interpolation",
+]
+preferred-modules = { "a"="b" }

tests/config/functional/toml/issue_4580/rich_types.result.json

@@ -0,0 +1,7 @@
+{
+  "functional_append": {
+    "disable": [["logging-not-lazy"], ["logging-format-interpolation"]]
+  },
+  "jobs": 10,
+  "reports": true
+}

tests/config/functional/toml/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/functional/toml/issue_4580/top_level_disable.out

@@ -0,0 +1,2 @@
+************* Module {abspath}
+{relpath}:1:0: E0014: Out-of-place setting encountered in top level configuration-section 'disable' : 'logging-not-lazy,logging-format-interpolation' (bad-configuration-section)

tests/config/functional/toml/issue_4580/top_level_disable.toml

@@ -0,0 +1,3 @@
+[tool.pylint]
+# disable does not belong in top level section
+disable = "logging-not-lazy,logging-format-interpolation"