From b3ed9881194a857ca5d53fccf3629bf1847e12ff Mon Sep 17 00:00:00 2001 From: Joel Goguen Date: Wed, 17 May 2023 00:33:08 -0400 Subject: [PATCH] Allow substitutions to be valid names (#4726) --- esphome/components/substitutions/__init__.py | 15 +------ esphome/config_validation.py | 22 ++++++++++ esphome/const.py | 3 ++ tests/unit_tests/test_config_validation.py | 43 +++++++++++++++++++- 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index b65410cbed..ef368015b1 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -1,19 +1,14 @@ import logging -import re import esphome.config_validation as cv from esphome import core -from esphome.const import CONF_SUBSTITUTIONS +from esphome.const import CONF_SUBSTITUTIONS, VALID_SUBSTITUTIONS_CHARACTERS from esphome.yaml_util import ESPHomeDataBase, make_data_base from esphome.config_helpers import merge_config CODEOWNERS = ["@esphome/core"] _LOGGER = logging.getLogger(__name__) -VALID_SUBSTITUTIONS_CHARACTERS = ( - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_" -) - def validate_substitution_key(value): value = cv.string(value) @@ -42,12 +37,6 @@ async def to_code(config): pass -# pylint: disable=consider-using-f-string -VARIABLE_PROG = re.compile( - "\\$([{0}]+|\\{{[{0}]*\\}})".format(VALID_SUBSTITUTIONS_CHARACTERS) -) - - def _expand_substitutions(substitutions, value, path, ignore_missing): if "$" not in value: return value @@ -56,7 +45,7 @@ def _expand_substitutions(substitutions, value, path, ignore_missing): i = 0 while True: - m = VARIABLE_PROG.search(value, i) + m = cv.VARIABLE_PROG.search(value, i) if not m: # Nothing more to match. Done break diff --git a/esphome/config_validation.py b/esphome/config_validation.py index 2482e5471c..0a6b2dfbb0 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -53,6 +53,7 @@ from esphome.const import ( KEY_TARGET_PLATFORM, TYPE_GIT, TYPE_LOCAL, + VALID_SUBSTITUTIONS_CHARACTERS, ) from esphome.core import ( CORE, @@ -79,6 +80,11 @@ from esphome.yaml_util import make_data_base _LOGGER = logging.getLogger(__name__) +# pylint: disable=consider-using-f-string +VARIABLE_PROG = re.compile( + "\\$([{0}]+|\\{{[{0}]*\\}})".format(VALID_SUBSTITUTIONS_CHARACTERS) +) + # pylint: disable=invalid-name Schema = _Schema @@ -265,6 +271,14 @@ def alphanumeric(value): def valid_name(value): value = string_strict(value) + + if CORE.vscode: + # If the value is a substitution, it can't be validated until the substitution + # is actually made. + sub_match = VARIABLE_PROG.search(value) + if sub_match: + return value + for c in value: if c not in ALLOWED_NAME_CHARS: raise Invalid( @@ -447,6 +461,14 @@ def validate_id_name(value): raise Invalid( "Dashes are not supported in IDs, please use underscores instead." ) + + if CORE.vscode: + # If the value is a substitution, it can't be validated until the substitution + # is actually made + sub_match = VARIABLE_PROG.match(value) + if sub_match: + return value + valid_chars = f"{ascii_letters + digits}_" for char in value: if char not in valid_chars: diff --git a/esphome/const.py b/esphome/const.py index f784efe820..cbc8f428f5 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -3,6 +3,9 @@ __version__ = "2023.6.0-dev" ALLOWED_NAME_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789-_" +VALID_SUBSTITUTIONS_CHARACTERS = ( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_" +) PLATFORM_ESP32 = "esp32" PLATFORM_ESP8266 = "esp8266" diff --git a/tests/unit_tests/test_config_validation.py b/tests/unit_tests/test_config_validation.py index 79d00adc29..34f70be2fb 100644 --- a/tests/unit_tests/test_config_validation.py +++ b/tests/unit_tests/test_config_validation.py @@ -6,7 +6,7 @@ from hypothesis.strategies import one_of, text, integers, builds from esphome import config_validation from esphome.config_validation import Invalid -from esphome.core import Lambda, HexInt +from esphome.core import CORE, Lambda, HexInt def test_check_not_templatable__invalid(): @@ -40,6 +40,47 @@ def test_valid_name__invalid(value): config_validation.valid_name(value) +@pytest.mark.parametrize("value", ("${name}", "${NAME}", "$NAME", "${name}_name")) +def test_valid_name__substitution_valid(value): + CORE.vscode = True + actual = config_validation.valid_name(value) + assert actual == value + + CORE.vscode = False + with pytest.raises(Invalid): + actual = config_validation.valid_name(value) + + +@pytest.mark.parametrize("value", ("{NAME}", "${A NAME}")) +def test_valid_name__substitution_like_invalid(value): + with pytest.raises(Invalid): + config_validation.valid_name(value) + + +@pytest.mark.parametrize("value", ("myid", "anID", "SOME_ID_test", "MYID_99")) +def test_validate_id_name__valid(value): + actual = config_validation.validate_id_name(value) + + assert actual == value + + +@pytest.mark.parametrize("value", ("id of mine", "id-4", "{name_id}", "id::name")) +def test_validate_id_name__invalid(value): + with pytest.raises(Invalid): + config_validation.validate_id_name(value) + + +@pytest.mark.parametrize("value", ("${id}", "${ID}", "${ID}_test_1", "$MYID")) +def test_validate_id_name__substitution_valid(value): + CORE.vscode = True + actual = config_validation.validate_id_name(value) + assert actual == value + + CORE.vscode = False + with pytest.raises(Invalid): + config_validation.validate_id_name(value) + + @given(one_of(integers(), text())) def test_string__valid(value): actual = config_validation.string(value)