fix: in toml config, only apply environment substitution to coverage …

…settings. #1481

CHANGES.rst

@@ -30,11 +30,17 @@ Unreleased
   - A ``[paths]`` setting like ``*/foo`` will now match ``foo/bar.py`` so that
     relative file paths can be combined more easily.
 
-- Fix internal logic that prevented coverage.py from running on implementations
-  other than CPython or PyPy (`issue 1474`_).
+- Fixed environment variable expansion in pyproject.toml files.  It was overly
+  broad, causing errors outside of coverage.py settings, as described in `issue
+  1481`_.  This is now fixed, but in rare cases will require changing your
+  pyproject.toml to quote non-string values using environment substitution.
+
+- Fixed internal logic that prevented coverage.py from running on
+  implementations other than CPython or PyPy (`issue 1474`_).
 
 .. _issue 991: https://github.com/nedbat/coveragepy/issues/991
 .. _issue 1474: https://github.com/nedbat/coveragepy/issues/1474
+.. _issue 1481: https://github.com/nedbat/coveragepy/issues/1481
 
 
 .. _changes_6-5-0:

coverage/tomlconfig.py

@@ -52,7 +52,6 @@ def read(self, filenames):
         except OSError:
             return []
         if tomllib is not None:
-            toml_text = substitute_variables(toml_text, os.environ)
             try:
                 self.data = tomllib.loads(toml_text)
             except tomllib.TOMLDecodeError as err:
@@ -101,9 +100,21 @@ def _get(self, section, option):
         if data is None:
             raise configparser.NoSectionError(section)
         try:
-            return name, data[option]
+            value = data[option]
         except KeyError as exc:
             raise configparser.NoOptionError(option, name) from exc
+        return name, value
+
+    def _get_single(self, section, option):
+        """Get a single-valued option.
+
+        Performs environment substitution if the value is a string. Other types
+        will be converted later as needed.
+        """
+        name, value = self._get(section, option)
+        if isinstance(value, str):
+            value = substitute_variables(value, os.environ)
+        return name, value
 
     def has_option(self, section, option):
         _, data = self._get_section(section)
@@ -126,29 +137,45 @@ def get_section(self, section):
         return data
 
     def get(self, section, option):
-        _, value = self._get(section, option)
+        _, value = self._get_single(section, option)
         return value
 
-    def _check_type(self, section, option, value, type_, type_desc):
-        if not isinstance(value, type_):
-            raise ValueError(
-                'Option {!r} in section {!r} is not {}: {!r}'
-                    .format(option, section, type_desc, value)
-            )
+    def _check_type(self, section, option, value, type_, converter, type_desc):
+        """Check that `value` has the type we want, converting if needed.
+
+        Returns the resulting value of the desired type.
+        """
+        if isinstance(value, type_):
+            return value
+        if isinstance(value, str) and converter is not None:
+            try:
+                return converter(value)
+            except Exception as e:
+                raise ValueError(
+                    f"Option [{section}]{option} couldn't convert to {type_desc}: {value!r}"
+                ) from e
+        raise ValueError(
+            f"Option [{section}]{option} is not {type_desc}: {value!r}"
+        )
 
     def getboolean(self, section, option):
-        name, value = self._get(section, option)
-        self._check_type(name, option, value, bool, "a boolean")
-        return value
+        name, value = self._get_single(section, option)
+        bool_strings = {"true": True, "false": False}
+        return self._check_type(name, option, value, bool, bool_strings.__getitem__, "a boolean")
 
-    def getlist(self, section, option):
+    def _get_list(self, section, option):
+        """Get a list of strings, substituting environment variables in the elements."""
         name, values = self._get(section, option)
-        self._check_type(name, option, values, list, "a list")
+        values = self._check_type(name, option, values, list, None, "a list")
+        values = [substitute_variables(value, os.environ) for value in values]
+        return name, values
+
+    def getlist(self, section, option):
+        _, values = self._get_list(section, option)
         return values
 
     def getregexlist(self, section, option):
-        name, values = self._get(section, option)
-        self._check_type(name, option, values, list, "a list")
+        name, values = self._get_list(section, option)
         for value in values:
             value = value.strip()
             try:
@@ -158,13 +185,11 @@ def getregexlist(self, section, option):
         return values
 
     def getint(self, section, option):
-        name, value = self._get(section, option)
-        self._check_type(name, option, value, int, "an integer")
-        return value
+        name, value = self._get_single(section, option)
+        return self._check_type(name, option, value, int, int, "an integer")
 
     def getfloat(self, section, option):
-        name, value = self._get(section, option)
+        name, value = self._get_single(section, option)
         if isinstance(value, int):
             value = float(value)
-        self._check_type(name, option, value, float, "a float")
-        return value
+        return self._check_type(name, option, value, float, float, "a float")

doc/config.rst

@@ -31,10 +31,14 @@ Coverage.py will read settings from other usual configuration files if no other
 configuration file is used.  It will automatically read from "setup.cfg" or
 "tox.ini" if they exist.  In this case, the section names have "coverage:"
 prefixed, so the ``[run]`` options described below will be found in the
-``[coverage:run]`` section of the file. If coverage.py is installed with the
-``toml`` extra (``pip install coverage[toml]``), it will automatically read
-from "pyproject.toml". Configuration must be within the ``[tool.coverage]``
-section, for example, ``[tool.coverage.run]``.
+``[coverage:run]`` section of the file.
+
+Coverage.py will read from "pyproject.toml" if TOML support is available,
+either because you are running on Python 3.11 or later, or because you
+installed with the ``toml`` extra (``pip install coverage[toml]``).
+Configuration must be within the ``[tool.coverage]`` section, for example,
+``[tool.coverage.run]``.  Environment variable expansion in values is
+available, but only within quoted strings, even for non-string values.
 
 
 Syntax

tests/test_config.py

@@ -3,7 +3,6 @@
 
 """Test the config file handling for coverage.py"""
 
-import math
 import sys
 from collections import OrderedDict
 
@@ -89,7 +88,7 @@ def test_toml_config_file(self):
         assert cov.config.plugins == ["plugins.a_plugin"]
         assert cov.config.precision == 3
         assert cov.config.html_title == "tabblo & «ταБЬℓσ»"
-        assert math.isclose(cov.config.fail_under, 90.5)
+        assert cov.config.fail_under == 90.5
         assert cov.config.get_plugin_options("plugins.a_plugin") == {"hello": "world"}
 
         # Test that our class doesn't reject integers when loading floats
@@ -99,7 +98,7 @@ def test_toml_config_file(self):
             fail_under = 90
             """)
         cov = coverage.Coverage(config_file="pyproject.toml")
-        assert math.isclose(cov.config.fail_under, 90)
+        assert cov.config.fail_under == 90
         assert isinstance(cov.config.fail_under, float)
 
     def test_ignored_config_file(self):
@@ -200,7 +199,7 @@ def test_parse_errors(self, bad_config, msg):
          r"multiple repeat"),
         ('[tool.coverage.run]\nconcurrency="foo"', "not a list"),
         ("[tool.coverage.report]\nprecision=1.23", "not an integer"),
-        ('[tool.coverage.report]\nfail_under="s"', "not a float"),
+        ('[tool.coverage.report]\nfail_under="s"', "couldn't convert to a float"),
     ])
     def test_toml_parse_errors(self, bad_config, msg):
         # Im-parsable values raise ConfigError, with details.
@@ -230,14 +229,15 @@ def test_environment_vars_in_config(self):
         assert cov.config.branch is True
         assert cov.config.exclude_list == ["the_$one", "anotherZZZ", "xZZZy", "xy", "huh${X}what"]
 
-    @pytest.mark.xfail(reason="updated to demonstrate bug #1481")
     def test_environment_vars_in_toml_config(self):
         # Config files can have $envvars in them.
         self.make_file("pyproject.toml", """\
             [tool.coverage.run]
             data_file = "$DATA_FILE.fooey"
-            branch = $BRANCH
+            branch = "$BRANCH"
             [tool.coverage.report]
+            precision = "$DIGITS"
+            fail_under = "$FAIL_UNDER"
             exclude_lines = [
                 "the_$$one",
                 "another${THING}",
@@ -246,15 +246,20 @@ def test_environment_vars_in_toml_config(self):
                 "huh$${X}what",
             ]
             [othersection]
+            # This reproduces the failure from https://github.com/nedbat/coveragepy/issues/1481
+            # When OTHER has a backslash that isn't a valid escape, like \\z (see below).
             something = "if [ $OTHER ]; then printf '%s\\n' 'Hi'; fi"
             """)
         self.set_environ("BRANCH", "true")
+        self.set_environ("DIGITS", "3")
+        self.set_environ("FAIL_UNDER", "90.5")
         self.set_environ("DATA_FILE", "hello-world")
         self.set_environ("THING", "ZZZ")
         self.set_environ("OTHER", "hi\\zebra")
         cov = coverage.Coverage()
-        assert cov.config.data_file == "hello-world.fooey"
         assert cov.config.branch is True
+        assert cov.config.precision == 3
+        assert cov.config.data_file == "hello-world.fooey"
         assert cov.config.exclude_list == ["the_$one", "anotherZZZ", "xZZZy", "xy", "huh${X}what"]
 
     def test_tilde_in_config(self):