From 1cd6d39e6275338bb8c3e74a93ff22e60619600a Mon Sep 17 00:00:00 2001
From: Mark Bakhit <16909269+Archmonger@users.noreply.github.com>
Date: Tue, 27 Jun 2023 23:59:01 -0700
Subject: [PATCH] Simplified regex expressions (#157)

- Clean up component regex, which can potentially increase regex performance.
- Remove loops from component tests
---
 src/reactpy_django/utils.py        |   6 +-
 tests/test_app/tests/test_regex.py | 173 +++++++++++++++++++----------
 2 files changed, 115 insertions(+), 64 deletions(-)

diff --git a/src/reactpy_django/utils.py b/src/reactpy_django/utils.py
index a9edbd3f..219f5c3e 100644
--- a/src/reactpy_django/utils.py
+++ b/src/reactpy_django/utils.py
@@ -27,9 +27,9 @@
 
 _logger = logging.getLogger(__name__)
 _component_tag = r"(?P<tag>component)"
-_component_path = r"(?P<path>(\"[^\"'\s]+\")|('[^\"'\s]+'))"
-_component_kwargs = r"(?P<kwargs>(.*?|\s*?)*)"
-COMMENT_REGEX = re.compile(r"(<!--)(.|\s)*?(-->)")
+_component_path = r"(?P<path>\"[^\"'\s]+\"|'[^\"'\s]+')"
+_component_kwargs = r"(?P<kwargs>[\s\S]*?)"
+COMMENT_REGEX = re.compile(r"<!--[\s\S]*?-->")
 COMPONENT_REGEX = re.compile(
     r"{%\s*"
     + _component_tag
diff --git a/tests/test_app/tests/test_regex.py b/tests/test_app/tests/test_regex.py
index 096107b5..cee32751 100644
--- a/tests/test_app/tests/test_regex.py
+++ b/tests/test_app/tests/test_regex.py
@@ -5,81 +5,132 @@
 
 class RegexTests(TestCase):
     def test_component_regex(self):
-        for component in {
-            r'{%component "my.component"%}',
-            r'{%component  "my.component"%}',
-            r"{%component 'my.component'%}",
-            r'{% component "my.component" %}',
-            r"{% component 'my.component' %}",
-            r'{% component "my.component" class="my_thing" %}',
+        # Real component matches
+        self.assertRegex(r'{%component "my.component"%}', COMPONENT_REGEX)
+        self.assertRegex(r'{%component  "my.component"%}', COMPONENT_REGEX)
+        self.assertRegex(r"{%component 'my.component'%}", COMPONENT_REGEX)
+        self.assertRegex(r'{% component "my.component" %}', COMPONENT_REGEX)
+        self.assertRegex(r"{% component 'my.component' %}", COMPONENT_REGEX)
+        self.assertRegex(
+            r'{% component "my.component" class="my_thing" %}', COMPONENT_REGEX
+        )
+        self.assertRegex(
             r'{% component "my.component" class="my_thing" attr="attribute" %}',
+            COMPONENT_REGEX,
+        )
+        self.assertRegex(
             r"""{%
+            component   
+            "my.component"  
+            class="my_thing"  
+            attr="attribute"  
 
-                component   
-                "my.component"  
-                class="my_thing"  
-                attr="attribute"  
+        %}""",  # noqa: W291
+            COMPONENT_REGEX,
+        )
 
-            %}""",  # noqa: W291
-        }:
-            self.assertRegex(component, COMPONENT_REGEX)
+        # Fake component matches
+        self.assertNotRegex(r'{% not_a_real_thing "my.component" %}', COMPONENT_REGEX)
+        self.assertNotRegex(r"{% component my.component %}", COMPONENT_REGEX)
+        self.assertNotRegex(r"""{% component 'my.component" %}""", COMPONENT_REGEX)
+        self.assertNotRegex(r'{ component "my.component" }', COMPONENT_REGEX)
+        self.assertNotRegex(r'{{ component "my.component" }}', COMPONENT_REGEX)
+        self.assertNotRegex(r"component", COMPONENT_REGEX)
+        self.assertNotRegex(r"{%%}", COMPONENT_REGEX)
+        self.assertNotRegex(r" ", COMPONENT_REGEX)
+        self.assertNotRegex(r"", COMPONENT_REGEX)
+        self.assertNotRegex(r'{% component " my.component " %}', COMPONENT_REGEX)
+        self.assertNotRegex(
+            r"""{% component "my.component COMPONENT_REGEX)
+        self.assertNotRegex(            " %}""",
+            COMPONENT_REGEX,
+        )
+        self.assertNotRegex(r'{{ component """ }}', COMPONENT_REGEX)
+        self.assertNotRegex(r'{{ component "" }}', COMPONENT_REGEX)
 
-        for fake_component in {
-            r'{% not_a_real_thing "my.component" %}',
-            r"{% component my.component %}",
-            r"""{% component 'my.component" %}""",
-            r'{ component "my.component" }',
-            r'{{ component "my.component" }}',
-            r"component",
-            r"{%%}",
-            r" ",
-            r"",
-            r'{% component " my.component " %}',
-            r"""{% component "my.component
-                        " %}""",
-            r'{{ component """ }}',
-            r'{{ component "" }}',
-        }:
-            self.assertNotRegex(fake_component, COMPONENT_REGEX)
+        # Make sure back-to-back components are not merged into one match
+        double_component_match = COMPONENT_REGEX.search(
+            r'{% component "my.component" %} {% component "my.component" %}'
+        )
+        self.assertTrue(double_component_match[0] == r'{% component "my.component" %}')  # type: ignore
 
     def test_comment_regex(self):
-        for comment in {
-            r"<!-- comment -->",
+        # Real comment matches
+        self.assertRegex(r"<!-- comment -->", COMMENT_REGEX)
+        self.assertRegex(
             r"""<!-- comment
-            -->""",
+        -->""",
+            COMMENT_REGEX,
+        )
+        self.assertRegex(
             r"""<!--
-             comment -->""",
+            comment -->""",
+            COMMENT_REGEX,
+        )
+        self.assertRegex(
             r"""<!--
-            comment
-            -->""",
+        comment
+        -->""",
+            COMMENT_REGEX,
+        )
+        self.assertRegex(
             r"""<!-- 
-            a comment
-            another comments
-            drink some cement 
-            -->""",  # noqa: W291
-        }:
-            self.assertRegex(comment, COMMENT_REGEX)
+        a comment
+        another comments
+        drink some cement 
+        -->""",  # noqa: W291
+            COMMENT_REGEX,
+        )
 
-        for fake_comment in {
-            r"<!-- a comment ",
-            r"another comment -->",
-            r"<! - - comment - - >",
-            r'{% component "my.component" %}',
-        }:
-            self.assertNotRegex(fake_comment, COMMENT_REGEX)
+        # Fake comment matches
+        self.assertNotRegex(r"<!-- a comment ", COMMENT_REGEX)
+        self.assertNotRegex(r"another comment -->", COMMENT_REGEX)
+        self.assertNotRegex(r"<! - - comment - - >", COMMENT_REGEX)
+        self.assertNotRegex(r'{% component "my.component" %}', COMMENT_REGEX)
 
-        for embedded_comment in {
-            r'{% component "my.component" %} <!-- comment -->',
-            r'<!-- comment --> {% component "my.component" %}',
-            r'<!-- comment --> {% component "my.component" %} <!-- comment -->',
-            r"""<!-- comment
+        # Components surrounded by comments
+        self.assertEquals(
+            COMMENT_REGEX.sub(
+                "", r'{% component "my.component" %} <!-- comment -->'
+            ).strip(),
+            '{% component "my.component" %}',
+        )
+        self.assertEquals(
+            COMMENT_REGEX.sub(
+                "", r'<!-- comment --> {% component "my.component" %}'
+            ).strip(),
+            '{% component "my.component" %}',
+        )
+        self.assertEquals(
+            COMMENT_REGEX.sub(
+                "", r'<!-- comment --> {% component "my.component" %} <!-- comment -->'
+            ).strip(),
+            '{% component "my.component" %}',
+        )
+        self.assertEquals(
+            COMMENT_REGEX.sub(
+                "",
+                r"""<!-- comment
                     --> {% component "my.component" %}
                     <!-- comment -->
                 <!--
                 comment -->""",
-        }:  # noqa: W291
-            text = COMMENT_REGEX.sub("", embedded_comment)
-            if text.strip() != '{% component "my.component" %}':
-                raise self.failureException(
-                    f"Regex pattern {COMMENT_REGEX.pattern} failed to remove comment from {embedded_comment}"
-                )
+            ).strip(),
+            '{% component "my.component" %}',
+        )
+
+        # Components surrounded by comments
+        self.assertEquals(
+            COMMENT_REGEX.sub("", r'<!-- {% component "my.component" %} -->'),
+            "",
+        )
+        self.assertEquals(
+            COMMENT_REGEX.sub(
+                "",
+                r"""<!--
+            {% component "my.component" %} 
+            {% component "my.component" %} 
+            -->""",  # noqa: W291
+            ),
+            "",
+        )