Add tests for action parsing. (#2112)

* Add tests for action parsing.

[skip ci]

* Fix action parsing issues.

Related issues: #2088.

* Simplify action parsing.

Turns out that we can just wrap the string that we want to parse as the arguments in '({string_here},)', as per @willmcgugan's comment in the PR review.

Related review comments: https://github.com/Textualize/textual/pull/2112\#issuecomment-1481015988

---------

Co-authored-by: Will McGugan <willmcgugan@gmail.com>

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Fixed
+
+- Issue with parsing action strings whose arguments contained quoted closing parenthesis https://github.com/Textualize/textual/pull/2112
+- Issues with parsing action strings with tuple arguments https://github.com/Textualize/textual/pull/2112
+
 ### Changed
 
 - DataTable now has height: auto by default. https://github.com/Textualize/textual/issues/2117
@@ -23,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   - `Tree`: `clear`, `reset`
 
 
+
 ## [0.16.0] - 2023-03-22
 
 ### Added

src/textual/actions.py

@@ -6,7 +6,7 @@ import re
 from typing_extensions import Any, TypeAlias
 
 ActionParseResult: TypeAlias = "tuple[str, tuple[Any, ...]]"
-"""An action is its name and the arbitrary tuple of its parameters."""
+"""An action is its name and the arbitrary tuple of its arguments."""
 
 
 class SkipAction(Exception):
@@ -17,7 +17,7 @@ class ActionError(Exception):
     pass
 
 
-re_action_params = re.compile(r"([\w\.]+)(\(.*?\))")
+re_action_args = re.compile(r"([\w\.]+)\((.*)\)")
 
 
 def parse(action: str) -> ActionParseResult:
@@ -30,22 +30,25 @@ def parse(action: str) -> ActionParseResult:
         ActionError: If the action has invalid syntax.
 
     Returns:
-        Action name and parameters
+        Action name and arguments.
     """
-    params_match = re_action_params.match(action)
+    args_match = re_action_args.match(action)
-    if params_match is not None:
+    if args_match is not None:
-        action_name, action_params_str = params_match.groups()
+        action_name, action_args_str = args_match.groups()
+        if action_args_str:
             try:
-            action_params = ast.literal_eval(action_params_str)
+                # We wrap `action_args_str` to be able to disambiguate the cases where
+                # the list of arguments is a comma-separated list of values from the
+                # case where the argument is a single tuple.
+                action_args: tuple[Any, ...] = ast.literal_eval(f"({action_args_str},)")
             except Exception:
                 raise ActionError(
-                f"unable to parse {action_params_str!r} in action {action!r}"
+                    f"unable to parse {action_args_str!r} in action {action!r}"
                 )
+        else:
+            action_args = ()
     else:
         action_name = action
-        action_params = ()
+        action_args = ()
 
-    return (
+    return action_name, action_args
-        action_name,
-        action_params if isinstance(action_params, tuple) else (action_params,),
-    )

tests/test_actions.py

@@ -0,0 +1,86 @@
+from __future__ import annotations
+
+from typing import Any
+
+import pytest
+
+from textual.actions import ActionError, parse
+
+
+@pytest.mark.parametrize(
+    ("action_string", "expected_name", "expected_arguments"),
+    [
+        ("spam", "spam", ()),
+        ("hypothetical_action()", "hypothetical_action", ()),
+        ("another_action(1)", "another_action", (1,)),
+        ("foo(True, False)", "foo", (True, False)),
+        ("foo.bar.baz(3, 3.15, 'Python')", "foo.bar.baz", (3, 3.15, "Python")),
+        ("m1234.n5678(None, [1, 2])", "m1234.n5678", (None, [1, 2])),
+    ],
+)
+def test_parse_action(
+    action_string: str, expected_name: str, expected_arguments: tuple[Any]
+) -> None:
+    action_name, action_arguments = parse(action_string)
+    assert action_name == expected_name
+    assert action_arguments == expected_arguments
+
+
+@pytest.mark.parametrize(
+    ("action_string", "expected_arguments"),
+    [
+        ("f()", ()),
+        ("f(())", ((),)),
+        ("f((1, 2, 3))", ((1, 2, 3),)),
+        ("f((1, 2, 3), (1, 2, 3))", ((1, 2, 3), (1, 2, 3))),
+        ("f(((1, 2), (), None), 0)", (((1, 2), (), None), 0)),
+        ("f((((((1))))))", (1,)),
+        ("f(((((((((1, 2)))))))))", ((1, 2),)),
+        ("f((1, 2), (3, 4))", ((1, 2), (3, 4))),
+        ("f((((((1, 2), (3, 4))))))", (((1, 2), (3, 4)),)),
+    ],
+)
+def test_nested_and_convoluted_tuple_arguments(
+    action_string: str, expected_arguments: tuple[Any]
+) -> None:
+    """Test that tuple arguments are parsed correctly."""
+    _, args = parse(action_string)
+    assert args == expected_arguments
+
+
+@pytest.mark.parametrize(
+    ["action_string", "expected_arguments"],
+    [
+        ("f('')", ("",)),
+        ('f("")', ("",)),
+        ("f('''''')", ("",)),
+        ('f("""""")', ("",)),
+        ("f('(')", ("(",)),
+        ("f(')')", (")",)),  # Regression test for #2088
+        ("f('f()')", ("f()",)),
+    ],
+)
+def test_parse_action_nested_special_character_arguments(
+    action_string: str, expected_arguments: tuple[Any]
+) -> None:
+    """Test that special characters nested in strings are handled correctly.
+
+    See also: https://github.com/Textualize/textual/issues/2088
+    """
+    _, args = parse(action_string)
+    assert args == expected_arguments
+
+
+@pytest.mark.parametrize(
+    "action_string",
+    [
+        "foo(,,,,,)",
+        "bar(1 2 3 4 5)",
+        "baz.spam(Tru, Fals, in)",
+        "ham(not)",
+        "cheese((((()",
+    ],
+)
+def test_parse_action_raises_error(action_string: str) -> None:
+    with pytest.raises(ActionError):
+        parse(action_string)