Fixed an issue when no error is raised if referred parameter (interpolation) is missing in a project configuration file // Resolve #3279

HISTORY.rst

@@ -43,7 +43,7 @@ PlatformIO Core 4.0
 * Fixed an issue when Project Inspector crashes when flash use > 100% (`issue #3368 <https://github.com/platformio/platformio-core/issues/3368>`_)
 * Fixed a "UnicodeDecodeError" when listing built-in libraries on macOS with Python 2.7 (`issue #3370 <https://github.com/platformio/platformio-core/issues/3370>`_)
 * Fixed an issue with improperly handled compiler flags with space symbols in VSCode template (`issue #3364 <https://github.com/platformio/platformio-core/issues/3364>`_)
-
+* Fixed an issue when no error is raised if referred parameter (interpolation) is missing in a project configuration file (`issue #3279 <https://github.com/platformio/platformio-core/issues/3279>`_)
 
 4.1.0 (2019-11-07)
 ~~~~~~~~~~~~~~~~~~

platformio/project/config.py

@@ -21,7 +21,7 @@ from hashlib import sha1
 import click
 
 from platformio import fs
-from platformio.compat import PY2, WINDOWS, hashlib_encode_data
+from platformio.compat import PY2, WINDOWS, hashlib_encode_data, string_types
 from platformio.project import exception
 from platformio.project.options import ProjectOptions
 
@@ -43,6 +43,9 @@ CONFIG_HEADER = """
 """
 
 
+MISSING = object()
+
+
 class ProjectConfigBase(object):
 
     INLINE_COMMENT_RE = re.compile(r"\s+;.*$")
@@ -242,46 +245,25 @@ class ProjectConfigBase(object):
             value = "\n" + value
         self._parser.set(section, option, value)
 
-    def getraw(self, section, option):
+    def getraw(  # pylint: disable=too-many-branches
+        self, section, option, default=MISSING
+    ):
         if not self.expand_interpolations:
             return self._parser.get(section, option)
 
-        value = None
+        value = MISSING
-        found = False
         for sec, opt in self.walk_options(section):
             if opt == option:
                 value = self._parser.get(sec, option)
-                found = True
                 break
 
-        if not found:
-            value = self._parser.get(section, option)
-
-        if "${" not in value or "}" not in value:
-            return value
-        return self.VARTPL_RE.sub(self._re_interpolation_handler, value)
-
-    def _re_interpolation_handler(self, match):
-        section, option = match.group(1), match.group(2)
-        if section == "sysenv":
-            return os.getenv(option)
-        return self.getraw(section, option)
-
-    def get(self, section, option, default=None):  # pylint: disable=too-many-branches
-        value = None
-        try:
-            value = self.getraw(section, option)
-        except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
-            pass  # handle value from system environment
-        except ConfigParser.Error as e:
-            raise exception.InvalidProjectConfError(self.path, str(e))
-
         option_meta = ProjectOptions.get("%s.%s" % (section.split(":", 1)[0], option))
         if not option_meta:
-            return value or default
+            if value == MISSING:
-
+                value = (
-        if option_meta.multiple:
+                    default if default != MISSING else self._parser.get(section, option)
-            value = self.parse_multi_values(value)
+                )
+            return self._expand_interpolations(value)
 
         if option_meta.sysenvvar:
             envvar_value = os.getenv(option_meta.sysenvvar)
@@ -291,17 +273,45 @@ class ProjectConfigBase(object):
                     if envvar_value:
                         break
             if envvar_value and option_meta.multiple:
-                value = value or []
+                value += ("" if value == MISSING else "\n") + envvar_value
-                value.extend(self.parse_multi_values(envvar_value))
+            elif envvar_value and value == MISSING:
-            elif envvar_value and not value:
                 value = envvar_value
 
-        # option is not specified by user
+        if value == MISSING:
-        if value is None or (
+            value = option_meta.default or default
-            option_meta.multiple and value == [] and option_meta.default
+        if value == MISSING:
-        ):
+            return None
-            return default if default is not None else option_meta.default
 
+        return self._expand_interpolations(value)
+
+    def _expand_interpolations(self, value):
+        if (
+            not value
+            or not isinstance(value, string_types)
+            or not all(["${" in value, "}" in value])
+        ):
+            return value
+        return self.VARTPL_RE.sub(self._re_interpolation_handler, value)
+
+    def _re_interpolation_handler(self, match):
+        section, option = match.group(1), match.group(2)
+        if section == "sysenv":
+            return os.getenv(option)
+        return self.getraw(section, option)
+
+    def get(self, section, option, default=MISSING):
+        value = None
+        try:
+            value = self.getraw(section, option, default)
+        except ConfigParser.Error as e:
+            raise exception.InvalidProjectConfError(self.path, str(e))
+
+        option_meta = ProjectOptions.get("%s.%s" % (section.split(":", 1)[0], option))
+        if not option_meta:
+            return value
+
+        if option_meta.multiple:
+            value = self.parse_multi_values(value or [])
         try:
             return self.cast_to(value, option_meta.type)
         except click.BadParameter as e:

tests/test_projectconf.py

@@ -17,7 +17,7 @@ import os
 import pytest
 
 from platformio.project.config import ConfigParser, ProjectConfig
-from platformio.project.exception import UnknownEnvNamesError
+from platformio.project.exception import InvalidProjectConfError, UnknownEnvNamesError
 
 BASE_CONFIG = """
 [platformio]
@@ -34,6 +34,7 @@ lib_deps =
     Lib1 ; inline comment in multi-line value
     Lib2
 lib_ignore = ${custom.lib_ignore}
+custom_builtin_option = ${env.build_type}
 
 [strict_ldf]
 lib_ldf_mode = chain+
@@ -54,7 +55,7 @@ lib_ignore = LibIgnoreCustom
 
 [env:base]
 build_flags = ${custom.debug_flags} ${custom.extra_flags}
-lib_compat_mode = ${strict_ldf.strict}
+lib_compat_mode = ${strict_ldf.lib_compat_mode}
 targets =
 
 [env:test_extends]
@@ -100,13 +101,10 @@ def config(tmpdir_factory):
 
 def test_empty_config():
     config = ProjectConfig("/non/existing/platformio.ini")
-
     # unknown section
-    with pytest.raises(ConfigParser.NoSectionError):
+    with pytest.raises(InvalidProjectConfError):
-        config.getraw("unknown_section", "unknown_option")
+        config.get("unknown_section", "unknown_option")
-
     assert config.sections() == []
-    assert config.get("section", "option") is None
     assert config.get("section", "option", 13) == 13
 
 
@@ -159,6 +157,7 @@ def test_options(config):
         "custom_monitor_speed",
         "lib_deps",
         "lib_ignore",
+        "custom_builtin_option",
     ]
     assert config.options(env="test_extends") == [
         "extends",
@@ -169,6 +168,7 @@ def test_options(config):
         "custom_monitor_speed",
         "lib_deps",
         "lib_ignore",
+        "custom_builtin_option",
     ]
 
 
@@ -180,7 +180,7 @@ def test_has_option(config):
 
 
 def test_sysenv_options(config):
-    assert config.get("custom", "extra_flags") is None
+    assert config.getraw("custom", "extra_flags") == ""
     assert config.get("env:base", "build_flags") == ["-D DEBUG=1"]
     assert config.get("env:base", "upload_port") is None
     assert config.get("env:extra_2", "upload_port") == "/dev/extra_2/port"
@@ -205,6 +205,7 @@ def test_sysenv_options(config):
         "custom_monitor_speed",
         "lib_deps",
         "lib_ignore",
+        "custom_builtin_option",
         "upload_port",
     ]
 
@@ -227,6 +228,10 @@ def test_getraw_value(config):
     with pytest.raises(ConfigParser.NoOptionError):
         config.getraw("platformio", "monitor_speed")
 
+    # default
+    assert config.getraw("unknown", "option", "default") == "default"
+    assert config.getraw("env:base", "custom_builtin_option") == "release"
+
     # known
     assert config.getraw("env:base", "targets") == ""
     assert config.getraw("env:extra_1", "lib_deps") == "574"
@@ -258,17 +263,18 @@ def test_items(config):
     assert config.items("custom") == [
         ("debug_flags", "-D DEBUG=1"),
         ("lib_flags", "-lc -lm"),
-        ("extra_flags", None),
+        ("extra_flags", ""),
         ("lib_ignore", "LibIgnoreCustom"),
     ]
     assert config.items(env="base") == [
         ("build_flags", ["-D DEBUG=1"]),
-        ("lib_compat_mode", "soft"),
+        ("lib_compat_mode", "strict"),
         ("targets", []),
         ("monitor_speed", 9600),
         ("custom_monitor_speed", "115200"),
         ("lib_deps", ["Lib1", "Lib2"]),
         ("lib_ignore", ["LibIgnoreCustom"]),
+        ("custom_builtin_option", "release"),
     ]
     assert config.items(env="extra_1") == [
         (
@@ -279,6 +285,7 @@ def test_items(config):
         ("monitor_speed", 9600),
         ("custom_monitor_speed", "115200"),
         ("lib_ignore", ["LibIgnoreCustom"]),
+        ("custom_builtin_option", "release"),
     ]
     assert config.items(env="extra_2") == [
         ("build_flags", ["-Og"]),
@@ -287,6 +294,7 @@ def test_items(config):
         ("monitor_speed", 9600),
         ("custom_monitor_speed", "115200"),
         ("lib_deps", ["Lib1", "Lib2"]),
+        ("custom_builtin_option", "release"),
     ]
     assert config.items(env="test_extends") == [
         ("extends", ["strict_settings"]),
@@ -297,6 +305,7 @@ def test_items(config):
         ("custom_monitor_speed", "115200"),
         ("lib_deps", ["Lib1", "Lib2"]),
         ("lib_ignore", ["LibIgnoreCustom"]),
+        ("custom_builtin_option", "release"),
     ]
 
 
@@ -393,6 +402,7 @@ def test_dump(tmpdir_factory):
                 ("custom_monitor_speed", "115200"),
                 ("lib_deps", ["Lib1", "Lib2"]),
                 ("lib_ignore", ["${custom.lib_ignore}"]),
+                ("custom_builtin_option", "${env.build_type}"),
             ],
         ),
         ("strict_ldf", [("lib_ldf_mode", "chain+"), ("lib_compat_mode", "strict")]),
@@ -414,7 +424,7 @@ def test_dump(tmpdir_factory):
             "env:base",
             [
                 ("build_flags", ["${custom.debug_flags} ${custom.extra_flags}"]),
-                ("lib_compat_mode", "${strict_ldf.strict}"),
+                ("lib_compat_mode", "${strict_ldf.lib_compat_mode}"),
                 ("targets", []),
             ],
         ),