better messages, tests

src/ytdl_sub/script/parser.py

@@ -1,4 +1,5 @@
 from contextlib import contextmanager
+from enum import Enum
 from typing import Dict
 from typing import List
 from typing import Optional
@@ -23,13 +24,32 @@ from ytdl_sub.validators.string_formatter_validators import is_valid_source_vari
 
 # pylint: disable=invalid-name
 
+
+class ArgumentParser(Enum):
+    SCRIPT = "script"
+    FUNCTION = "function"
+    ARRAY = "array"
+    MAP_KEY = "map key"
+    MAP_VALUE = "map value"
+
+
 UNREACHABLE = UnreachableSyntaxException(
     "If you see this error, you have discovered a bug in the script parser!\n"
     "Please upload your config/subscription file(s) to and make a GitHub issue at "
     "https://github.com/jmbannon/ytdl-sub/issues"
 )
 
-UNEXPECTED_COMMA_ARGUMENT = InvalidSyntaxException("Unexpected comma when parsing arguments")
+BRACKET_NOT_CLOSED = InvalidSyntaxException("Bracket not properly closed")
+
+
+def UNEXPECTED_CHAR_ARGUMENT(parser: ArgumentParser):
+    return InvalidSyntaxException(f"Unexpected character when parsing {parser.value} arguments")
+
+
+def UNEXPECTED_COMMA_ARGUMENT(parser: ArgumentParser):
+    return InvalidSyntaxException(f"Unexpected comma when parsing {parser.value} arguments")
+
+
 MAP_KEY_WITH_NO_VALUE = InvalidSyntaxException("Map has a key with no value")
 MAP_KEY_MULTIPLE_VALUES = InvalidSyntaxException(
     "Map key has multiple values when there should only be one"
@@ -40,6 +60,10 @@ MAP_KEY_NOT_HASHABLE = InvalidSyntaxException(
 )
 
 
+def _is_variable_start(char: str) -> bool:
+    return char.isalpha() and char.islower()
+
+
 class _Parser:
     def __init__(self, text: str):
         self._text = text
@@ -67,6 +91,9 @@ class _Parser:
         self._error_highlight_pos = self._pos
 
     def _read(self, increment_pos: bool = True, length: int = 1) -> Optional[str]:
+        if self._pos >= len(self._text):
+            return None
+
         try:
             ch = self._text[self._pos : (self._pos + length)]
         except IndexError:
@@ -101,7 +128,7 @@ class _Parser:
         assert is_valid_source_variable_name(var_name, raise_exception=False)
         return Variable(var_name)
 
-    def _parse_function_argument(self) -> FunctionArgument:
+    def _parse_custom_function_argument(self) -> FunctionArgument:
         """
         Begin parsing function args after the first ``$``, i.e. ``$1``
         """
@@ -159,7 +186,7 @@ class _Parser:
 
         raise StringFormattingException("String not closed")
 
-    def _parse_function_arg(self) -> ArgumentType:
+    def _parse_function_arg(self, argument_parser: ArgumentParser) -> ArgumentType:
         if self._read(increment_pos=False) == "%":
             self._pos += 1
             return self._parse_function()
@@ -181,38 +208,39 @@ class _Parser:
             return self._parse_map()
         if self._read(increment_pos=False) == "$":
             self._pos += 1
-            return self._parse_function_argument()
+            return self._parse_custom_function_argument()
-        if self._read(increment_pos=False).isascii() and self._read(increment_pos=False).islower():
+        if _is_variable_start(self._read(increment_pos=False)):
             return self._parse_variable()
-        raise StringFormattingException(
-            "Invalid function argument, should be either a function, int, float, "
-            "string, boolean, or variable without brackets"
-        )
 
-    def _parse_args(self, breaking_chars: str = ")") -> List[ArgumentType]:
+        self._set_highlight_position()
+        raise UNEXPECTED_CHAR_ARGUMENT(parser=argument_parser)
+
+    def _parse_args(
+        self, argument_parser: ArgumentParser, breaking_chars: str = ")"
+    ) -> List[ArgumentType]:
         """
         Begin parsing function args after the first ``(``, i.e. ``function_name(``
         """
-        argument_index = 0
         comma_count = 0
-
         arguments: List[ArgumentType] = []
         while ch := self._read(increment_pos=False):
             if ch in breaking_chars:
+                # i.e. ["arg", ] which is invalid
+                if arguments and len(arguments) == comma_count:
+                    raise UNEXPECTED_COMMA_ARGUMENT(argument_parser)
                 break
 
             if ch.isspace():
                 self._pos += 1
             elif ch == ",":
+                self._set_highlight_position()
                 comma_count += 1
-                if argument_index != comma_count:
+                if len(arguments) != comma_count:
-                    self._set_highlight_position()
+                    raise UNEXPECTED_COMMA_ARGUMENT(argument_parser)
-                    raise UNEXPECTED_COMMA_ARGUMENT
 
                 self._pos += 1
             else:
-                argument_index += 1
+                arguments.append(self._parse_function_arg(argument_parser=argument_parser))
-                arguments.append(self._parse_function_arg())
 
         return arguments
 
@@ -230,7 +258,7 @@ class _Parser:
             if ch != "(":
                 function_name += ch
             else:
-                function_args = self._parse_args()
+                function_args = self._parse_args(argument_parser=ArgumentParser.FUNCTION)
 
         raise StringFormattingException("Invalid function")
 
@@ -245,7 +273,9 @@ class _Parser:
                 self._pos += 1
                 return UnresolvedArray(value=function_args)
             else:
-                function_args = self._parse_args(breaking_chars="]")
+                function_args = self._parse_args(
+                    argument_parser=ArgumentParser.ARRAY, breaking_chars="]"
+                )
 
         raise UNREACHABLE
 
@@ -267,17 +297,19 @@ class _Parser:
                 return UnresolvedMap(value=output)
             elif ch == ",":
                 if in_comma:
-                    raise UNEXPECTED_COMMA_ARGUMENT
+                    raise UNEXPECTED_COMMA_ARGUMENT(ArgumentParser.MAP_KEY)
                 if key is not None:
                     raise MAP_KEY_WITH_NO_VALUE
                 if not output:
-                    raise UNEXPECTED_COMMA_ARGUMENT
+                    raise UNEXPECTED_COMMA_ARGUMENT(ArgumentParser.MAP_KEY)
                 in_comma = True
                 self._pos += 1
             elif key is None:
                 self._set_highlight_position()
                 in_comma = False
-                key_args = self._parse_args(breaking_chars=":}")
+                key_args = self._parse_args(
+                    argument_parser=ArgumentParser.MAP_KEY, breaking_chars=":}"
+                )
 
                 if len(key_args) == 0 and self._read(increment_pos=False) == "}":
                     continue  # will return the map next iteration
@@ -289,7 +321,9 @@ class _Parser:
             elif key is not None and ch == ":":
                 self._set_highlight_position()
                 self._pos += 1
-                value_args = self._parse_args(breaking_chars=",}")
+                value_args = self._parse_args(
+                    argument_parser=ArgumentParser.MAP_VALUE, breaking_chars=",}"
+                )
                 if len(value_args) == 0:
                     raise MAP_KEY_WITH_NO_VALUE
                 if isinstance(key, NonHashable):
@@ -303,13 +337,19 @@ class _Parser:
                 raise UNREACHABLE
 
     def _parse(self) -> SyntaxTree:
+        bracket_counter_pos_stack: List[int] = []
         bracket_counter = 0
         literal_str = ""
         while ch := self._read():
             if ch == "}":
+                if bracket_counter == 0:
+                    raise BRACKET_NOT_CLOSED
+
+                del bracket_counter_pos_stack[-1]
                 bracket_counter -= 1
                 continue
             if ch == "{":
+                bracket_counter_pos_stack.append(self._pos - 1)  # pos incremented when read
                 bracket_counter += 1
                 if literal_str:
                     self._ast.append(String(value=literal_str))
@@ -335,8 +375,10 @@ class _Parser:
                 elif ch1 == "{":
                     self._pos += 1
                     self._ast.append(self._parse_map())
-                else:
+                elif _is_variable_start(ch1):
                     self._ast.append(self._parse_variable())
+                else:
+                    raise UNEXPECTED_CHAR_ARGUMENT(parser=ArgumentParser.SCRIPT)
             elif bracket_counter == 0:
                 # Only accumulate literal str if not in brackets
                 literal_str += ch
@@ -345,7 +387,8 @@ class _Parser:
                 assert ch.isspace()
 
         if bracket_counter != 0:
-            raise StringFormattingException("Bracket count mismatch")
+            self._error_highlight_pos = bracket_counter_pos_stack[-1]
+            raise BRACKET_NOT_CLOSED
 
         if literal_str:
             self._ast.append(String(value=literal_str))

tests/unit/script/test_parser.py

@@ -1,8 +1,12 @@
+import re
 from typing import Optional
 from typing import Union
 
 import pytest
 
+from ytdl_sub.script.parser import BRACKET_NOT_CLOSED
+from ytdl_sub.script.parser import UNEXPECTED_CHAR_ARGUMENT
+from ytdl_sub.script.parser import ArgumentParser
 from ytdl_sub.script.parser import parse
 from ytdl_sub.script.types.function import BuiltInFunction
 from ytdl_sub.script.types.resolvable import Boolean
@@ -11,6 +15,7 @@ from ytdl_sub.script.types.resolvable import Integer
 from ytdl_sub.script.types.resolvable import String
 from ytdl_sub.script.types.syntax_tree import SyntaxTree
 from ytdl_sub.script.types.variable import Variable
+from ytdl_sub.script.utils.exceptions import InvalidSyntaxException
 from ytdl_sub.utils.exceptions import StringFormattingException
 
 
@@ -180,9 +185,12 @@ class TestParserBracketFailures:
             parse("{")
 
     def test_bracket_close(self):
-        with pytest.raises(StringFormattingException):
+        with pytest.raises(InvalidSyntaxException, match=re.escape(str(BRACKET_NOT_CLOSED))):
             parse("}")
 
     def test_bracket_in_function(self):
-        with pytest.raises(StringFormattingException):
+        with pytest.raises(
+            InvalidSyntaxException,
+            match=re.escape(str(UNEXPECTED_CHAR_ARGUMENT(ArgumentParser.MAP_KEY))),
+        ):
             parse("hello {%capitalize({as_arg)}")

tests/unit/script/types/test_array.py

@@ -2,7 +2,10 @@ import re
 
 import pytest
 
+from ytdl_sub.script.parser import BRACKET_NOT_CLOSED
+from ytdl_sub.script.parser import UNEXPECTED_CHAR_ARGUMENT
 from ytdl_sub.script.parser import UNEXPECTED_COMMA_ARGUMENT
+from ytdl_sub.script.parser import ArgumentParser
 from ytdl_sub.script.script import Script
 from ytdl_sub.script.types.array import ResolvedArray
 from ytdl_sub.script.types.resolvable import Float
@@ -64,9 +67,43 @@ class TestArray:
         ],
     )
     def test_unexpected_comma(self, array: str):
-        with pytest.raises(InvalidSyntaxException, match=re.escape(str(UNEXPECTED_COMMA_ARGUMENT))):
+        with pytest.raises(
+            InvalidSyntaxException,
+            match=re.escape(str(UNEXPECTED_COMMA_ARGUMENT(ArgumentParser.ARRAY))),
+        ):
             Script({"array": array}).resolve()
 
+    @pytest.mark.parametrize(
+        "array",
+        [
+            "{[}",
+            "{[      }",
+            "{[\n}",
+            "{['key'}",
+            "{[ 'key'   }",
+        ],
+    )
+    def test_array_not_closed(self, array: str):
+        with pytest.raises(
+            InvalidSyntaxException,
+            match=re.escape(str(UNEXPECTED_CHAR_ARGUMENT(ArgumentParser.ARRAY))),
+        ):
+            assert Script({"array": array}).resolve()
+
+    @pytest.mark.parametrize(
+        "array",
+        [
+            "{]}" "{      ]}",
+            "{\n]}",
+        ],
+    )
+    def test_array_not_opened(self, array: str):
+        with pytest.raises(
+            InvalidSyntaxException,
+            match=re.escape(str(UNEXPECTED_CHAR_ARGUMENT(ArgumentParser.SCRIPT))),
+        ):
+            assert Script({"array": array}).resolve()
+
     def test_custom_function(self):
         assert Script(
             {

tests/unit/script/types/test_map.py

@@ -2,11 +2,13 @@ import re
 
 import pytest
 
+from ytdl_sub.script.parser import BRACKET_NOT_CLOSED
 from ytdl_sub.script.parser import MAP_KEY_MULTIPLE_VALUES
 from ytdl_sub.script.parser import MAP_KEY_NOT_HASHABLE
 from ytdl_sub.script.parser import MAP_KEY_WITH_NO_VALUE
 from ytdl_sub.script.parser import MAP_MISSING_KEY
 from ytdl_sub.script.parser import UNEXPECTED_COMMA_ARGUMENT
+from ytdl_sub.script.parser import ArgumentParser
 from ytdl_sub.script.script import Script
 from ytdl_sub.script.types.map import ResolvedMap
 from ytdl_sub.script.types.resolvable import Float
@@ -74,6 +76,21 @@ class TestMap:
     def test_empty_map(self, empty_map: str):
         assert Script({"map": empty_map}).resolve() == {"map": ResolvedMap({})}
 
+    @pytest.mark.parametrize(
+        "map",
+        [
+            "{{}",
+            "{{      }",
+            "{{\n}",
+            "{{'key': 'value'}",
+            "{{ 'key' : 'value' }",
+            "{{      }",
+        ],
+    )
+    def test_map_not_closed(self, map: str):
+        with pytest.raises(InvalidSyntaxException, match=re.escape(str(BRACKET_NOT_CLOSED))):
+            Script({"map": map}).resolve()
+
     @pytest.mark.parametrize(
         "value",
         [
@@ -99,7 +116,10 @@ class TestMap:
         ],
     )
     def test_map_unexpected_comma(self, value: str):
-        with pytest.raises(InvalidSyntaxException, match=re.escape(str(UNEXPECTED_COMMA_ARGUMENT))):
+        with pytest.raises(
+            InvalidSyntaxException,
+            match=re.escape(str(UNEXPECTED_COMMA_ARGUMENT(ArgumentParser.MAP_KEY))),
+        ):
             Script({"map": value}).resolve()
 
     @pytest.mark.parametrize(