variable collisions validated

src/ytdl_sub/config/overrides.py

@@ -9,9 +9,9 @@ from ytdl_sub.entries.entry import Entry
 from ytdl_sub.entries.script.variable_definitions import VARIABLES
 from ytdl_sub.entries.variables.override_variables import SUBSCRIPTION_NAME
 from ytdl_sub.entries.variables.override_variables import OverrideVariables
-from ytdl_sub.script.functions import Functions
 from ytdl_sub.script.parser import parse
 from ytdl_sub.script.script import Script
+from ytdl_sub.utils.exceptions import ValidationException
 from ytdl_sub.utils.script import ScriptUtils
 from ytdl_sub.utils.scriptable import Scriptable
 from ytdl_sub.validators.string_formatter_validators import DictFormatterValidator
@@ -56,28 +56,48 @@ class Overrides(DictFormatterValidator, Scriptable):
         Scriptable.__init__(self)
 
         for key in self._keys:
-            if OverrideVariables.is_override_variable_name(key):
+            self.ensure_variable_name_valid(key)
+
+        self.unresolvable.add(VARIABLES.entry_metadata.variable_name)
+
+    def ensure_added_plugin_variable_valid(self, added_variable: str) -> bool:
+        """
+        Returns False if the variable exists as a non-override.
+
+        Raises
+        ------
+        ValidationException
+            If the variable is already added as an override variable.
+        """
+        try:
+            self.ensure_variable_name_valid(added_variable)
+        except ValidationException:
+            return False
+
+        if added_variable in self.keys:
             raise self._validation_exception(
-                    f"Override variable with name {key} cannot be used since it is a"
+                f"Override variable with name {added_variable} cannot be used since it is"
-                    " built-in ytdl-sub override variable name."
+                " added by a plugin."
             )
 
-            if key in self.script.variable_names:
+        return True
+
+    def ensure_variable_name_valid(self, name: str) -> None:
+        """
+        Ensures the variable name does not collide with any entry variables or built-in functions.
+        """
+        if OverrideVariables.is_entry_variable_name(name):
             raise self._validation_exception(
-                    f"Override variable with name {key} cannot be used since it is a"
+                f"Override variable with name {name} cannot be used since it is a"
                 " built-in ytdl-sub entry variable name."
             )
 
-            if key in self.script.function_names or (
+        if OverrideVariables.is_function_name(name):
-                key.startswith("%") and Functions.is_built_in(key[1:])
-            ):
             raise self._validation_exception(
-                    f"Override function definition with name {key} cannot be used since it is a"
+                f"Override function definition with name {name} cannot be used since it is"
-                    " built-in ytdl-sub function name."
+                " a built-in ytdl-sub function name."
             )
 
-        self.unresolvable.add(VARIABLES.entry_metadata.variable_name)
-
     def initial_variables(
         self, unresolved_variables: Optional[Dict[str, str]] = None
     ) -> Dict[str, str]:

src/ytdl_sub/config/validators/variable_validation.py

@@ -29,14 +29,17 @@ def _add_dummy_variables(variables: Iterable[str]) -> Dict[str, str]:
 
 def _get_added_and_modified_variables(
     plugins: PresetPlugins, downloader_options: MultiUrlValidator, resolved_variables: Set[str]
-) -> Tuple[Set[str], Set[str]]:
+) -> Iterable[Tuple[OptionsValidator, Set[str], Set[str]]]:
-    added_variables: Set[str] = set()
+    """
-    modified_variables: Set[str] = set()
+    Iterates and returns the plugin options, added variables, modified variables
-
+    """
     options: List[OptionsValidator] = plugins.plugin_options
     options.append(downloader_options)
 
     for plugin_options in options:
+        added_variables: Set[str] = set()
+        modified_variables: Set[str] = set()
+
         for plugin_added_variables in plugin_options.added_variables(
             resolved_variables=resolved_variables,
             unresolved_variables=set(),
@@ -44,9 +47,9 @@ def _get_added_and_modified_variables(
             added_variables |= set(plugin_added_variables)
 
         for plugin_modified_variables in plugin_options.modified_variables().values():
-            modified_variables |= plugin_modified_variables
+            modified_variables = plugin_modified_variables
 
-    return added_variables, modified_variables
+        yield plugin_options, added_variables, modified_variables
 
 
 def _override_variables(overrides: Overrides) -> Set[str]:
@@ -82,24 +85,31 @@ class VariableValidation:
         # Set resolved variables as all entry + override variables
         # at this point to generate every possible added/modified variable
         self.resolved_variables = entry_variables | override_variables
-        added_variables, modified_variables = _get_added_and_modified_variables(
+
+        for (
+            plugin_options,
+            added_variables,
+            modified_variables,
+        ) in _get_added_and_modified_variables(
             plugins=self.plugins,
             downloader_options=self.downloader_options,
             resolved_variables=self.resolved_variables,
-        )
+        ):
 
             for added_variable in added_variables:
-            if added_variable in overrides.keys:
+                if not overrides.ensure_added_plugin_variable_valid(added_variable=added_variable):
                     # pylint: disable=protected-access
-                raise overrides._validation_exception(
+                    raise plugin_options._validation_exception(
-                    f"Override variable with name {added_variable} cannot be used since it is a"
+                        f"Cannot use the variable name {added_variable} because it exists as a"
-                    " built-in ytdl-sub variable added by a plugin."
+                        " built-in ytdl-sub variable name."
                     )
                     # pylint: enable=protected-access
 
-        # Set unresolved as variables that are added but do not exist as entry/override variables
+            # Set unresolved as variables that are added but do not exist as
+            # entry/override variables since they are created at run-time
+            self.unresolved_variables |= added_variables | modified_variables
+
         # Then update resolved variables to reflect that
-        self.unresolved_variables = added_variables | modified_variables
         self.resolved_variables -= self.unresolved_variables
 
         # Initialize overrides with unresolved variables + modified variables to throw an error.

src/ytdl_sub/entries/variables/override_variables.py

@@ -1,4 +1,6 @@
-import re
+from ytdl_sub.entries.script.function_scripts import CUSTOM_FUNCTION_SCRIPTS
+from ytdl_sub.entries.script.variable_scripts import VARIABLE_SCRIPTS
+from ytdl_sub.script.functions import Functions
 
 SUBSCRIPTION_NAME = "subscription_name"
 SUBSCRIPTION_VALUE = "subscription_value"
@@ -61,24 +63,21 @@ class OverrideVariables:
         return f"subscription_value_{index + 1}"
 
     @classmethod
-    def is_override_variable_name(cls, variable_name: str):
+    def is_entry_variable_name(cls, name: str) -> bool:
         """
         Returns
         -------
-        True of a variable name collides with an override variable name. False otherwise.
+        True if the name is an entry variable name. False otherwise.
         """
-        if variable_name in (
+        return name in VARIABLE_SCRIPTS
-            SUBSCRIPTION_NAME,
-            SUBSCRIPTION_VALUE,
-            SUBSCRIPTION_MAP,
-            SUBSCRIPTION_ARRAY,
-        ):
-            return True
-
-        if re.fullmatch(r"^subscription_value_(\d+)$", variable_name):
-            return True
-
-        if re.fullmatch(r"^subscription_indent_(\d+)$", variable_name):
-            return True
 
+    @classmethod
+    def is_function_name(cls, name: str) -> bool:
+        """
+        Returns
+        -------
+        True if the name is a function name (either built-in or script). False otherwise.
+        """
+        if name.startswith("%"):
+            return name in CUSTOM_FUNCTION_SCRIPTS or Functions.is_built_in(name[1:])
         return False

tests/unit/config/test_preset.py

@@ -208,29 +208,6 @@ class TestPreset:
             },
         )
 
-    @pytest.mark.parametrize(
-        "override_variable_name", ["subscription_name", "subscription_value_3", "subscription_map"]
-    )
-    def test_preset_error_override_variable_collides_with_override(
-        self, config_file, output_options, youtube_video, override_variable_name: str
-    ):
-        with pytest.raises(
-            ValidationException,
-            match=re.escape(
-                f"Override variable with name {override_variable_name} cannot be used since"
-                " it is a built-in ytdl-sub override variable name."
-            ),
-        ):
-            _ = Preset(
-                config=config_file,
-                name="test",
-                value={
-                    "download": youtube_video,
-                    "output_options": {"output_directory": "dir", "file_name": "{dne_var}"},
-                    "overrides": {override_variable_name: "fail"},
-                },
-            )
-
     @pytest.mark.parametrize(
         "entry_variable_name",
         [
@@ -267,7 +244,7 @@ class TestPreset:
             ValidationException,
             match=re.escape(
                 f"Override variable with name subtitles_ext cannot be used since"
-                " it is a built-in ytdl-sub variable added by a plugin."
+                " it is added by a plugin."
             ),
         ):
             _ = Preset(
@@ -311,3 +288,48 @@ class TestPreset:
                     "overrides": {function_name: "fail"},
                 },
             )
+
+    def test_preset_error_override_added_variable_collides_with_built_in(
+        self, config_file, output_options
+    ):
+        with pytest.raises(
+            ValidationException,
+            match=re.escape(
+                "Cannot use the variable name title because it exists as a "
+                "built-in ytdl-sub variable name."
+            ),
+        ):
+            _ = Preset(
+                config=config_file,
+                name="test",
+                value={
+                    "download": {
+                        "url": "youtube.com/watch?v=123abc",
+                        "variables": {"title": "nope"},
+                    },
+                    "output_options": {"output_directory": "dir", "file_name": "acjk"},
+                },
+            )
+
+    def test_preset_error_override_added_variable_collides_with_override(
+        self, config_file, output_options
+    ):
+        with pytest.raises(
+            ValidationException,
+            match=re.escape(
+                "Override variable with name the_bad_one cannot be used since "
+                "it is added by a plugin."
+            ),
+        ):
+            _ = Preset(
+                config=config_file,
+                name="test",
+                value={
+                    "download": {
+                        "url": "youtube.com/watch?v=123abc",
+                        "variables": {"the_bad_one": "should error"},
+                    },
+                    "output_options": {"output_directory": "dir", "file_name": "acjk"},
+                    "overrides": {"the_bad_one": "ack"},
+                },
+            )