From 0707e0dd7396394ef6b3770b16d6089c3a4c0ebb Mon Sep 17 00:00:00 2001
From: Robert Raposa <rraposa@edx.org>
Date: Thu, 24 Mar 2016 15:48:50 -0400
Subject: [PATCH] Add check for multiple page tags

---
 scripts/safe_template_linter.py            | 29 +++++++---
 scripts/tests/test_safe_template_linter.py | 62 +++++++++++++++-------
 2 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/scripts/safe_template_linter.py b/scripts/safe_template_linter.py
index f0ffa00859..d02651d32b 100755
--- a/scripts/safe_template_linter.py
+++ b/scripts/safe_template_linter.py
@@ -150,7 +150,8 @@ class Rules(Enum):
     """
     An Enum of each rule which the linter will check.
     """
-    mako_missing_default = ('mako-missing-default', 'The default page directive with h filter is missing.')
+    mako_missing_default = ('mako-missing-default', 'Missing default <%page expression_filter="h"/>.')
+    mako_multiple_page_tags = ('mako-multiple-page-tags', 'A Mako template can only have one <%page> tag.')
     mako_unparsable_expression = ('mako-unparsable-expression', 'The expression could not be properly parsed.')
     mako_unwanted_html_filter = ('mako-unwanted-html-filter', 'Remove explicit h filters when it is provided by the page directive.')
     mako_invalid_html_filter = ('mako-invalid-html-filter', 'The expression is using an invalid filter in an HTML context.')
@@ -398,23 +399,37 @@ class MakoTemplateLinter(object):
             results: A file results objects to which violations will be added.
 
         """
-        has_page_default = self._has_page_default(mako_template, results)
-        if not has_page_default:
-            results.violations.append(RuleViolation(Rules.mako_missing_default))
+        has_page_default = False
+        if self._has_multiple_page_tags(mako_template):
+            results.violations.append(RuleViolation(Rules.mako_multiple_page_tags))
+        else:
+            has_page_default = self._has_page_default(mako_template)
+            if not has_page_default:
+                results.violations.append(RuleViolation(Rules.mako_missing_default))
         self._check_mako_expressions(mako_template, has_page_default, results)
         results.prepare_results(mako_template)
 
-    def _has_page_default(self, mako_template, results):
+    def _has_multiple_page_tags(self, mako_template):
+        """
+        Checks if the Mako template contains more than one page expression.
+
+        Arguments:
+            mako_template: The contents of the Mako template.
+
+        """
+        count = len(re.findall('<%page ', mako_template, re.IGNORECASE))
+        return count > 1
+
+    def _has_page_default(self, mako_template):
         """
         Checks if the Mako template contains the page expression marking it as
         safe by default.
 
         Arguments:
             mako_template: The contents of the Mako template.
-            results: A list of results into which violations will be added.
 
         """
-        page_h_filter_regex = re.compile('<%page expression_filter=(?:"h"|\'h\')\s*/>')
+        page_h_filter_regex = re.compile('<%page[^>]*expression_filter=(?:"h"|\'h\')[^>]*/>')
         page_match = page_h_filter_regex.search(mako_template)
         return page_match
 
diff --git a/scripts/tests/test_safe_template_linter.py b/scripts/tests/test_safe_template_linter.py
index fafff6a4cb..2084e7bcab 100644
--- a/scripts/tests/test_safe_template_linter.py
+++ b/scripts/tests/test_safe_template_linter.py
@@ -31,32 +31,54 @@ class TestMakoTemplateLinter(TestCase):
 
         self.assertEqual(linter._is_mako_directory(data['directory']), data['expected'])
 
-    def test_check_page_default_with_default_provided(self):
+    @data(
+        {
+            'template': '\n <%page expression_filter="h"/>',
+            'violations': 0,
+            'rule': None
+        },
+        {
+            'template':
+                '\n <%page args="section_data" expression_filter="h" /> ',
+            'violations': 0,
+            'rule': None
+        },
+        {
+            'template':
+                '\n <%page expression_filter="h" /> '
+                '\n <%page args="section_data"/>',
+            'violations': 1,
+            'rule': Rules.mako_multiple_page_tags
+        },
+        {
+            'template': '\n <%page args="section_data" /> ',
+            'violations': 1,
+            'rule': Rules.mako_missing_default
+        },
+        {
+            'template':
+                '\n <%page args="section_data"/> <some-other-tag expression_filter="h" /> ',
+            'violations': 1,
+            'rule': Rules.mako_missing_default
+        },
+        {
+            'template': '\n',
+            'violations': 1,
+            'rule': Rules.mako_missing_default
+        },
+    )
+    def test_check_page_default(self, data):
         """
-        Test _check_mako_file_is_safe with default causes no violation
+        Test _check_mako_file_is_safe with different page defaults
         """
         linter = MakoTemplateLinter()
         results = FileResults('')
-        mako_template = """
-            <%page expression_filter="h"/>
-        """
 
-        linter._check_mako_file_is_safe(mako_template, results)
+        linter._check_mako_file_is_safe(data['template'], results)
 
-        self.assertEqual(len(results.violations), 0)
-
-    def test_check_page_default_with_no_default_provided(self):
-        """
-        Test _check_mako_file_is_safe with no default causes violation
-        """
-        linter = MakoTemplateLinter()
-        results = FileResults('')
-        mako_template = ""
-
-        linter._check_mako_file_is_safe(mako_template, results)
-
-        self.assertEqual(len(results.violations), 1)
-        self.assertEqual(results.violations[0].rule, Rules.mako_missing_default)
+        self.assertEqual(len(results.violations), data['violations'])
+        if data['violations'] > 0:
+            self.assertEqual(results.violations[0].rule, data['rule'])
 
     def test_check_mako_expressions_in_html(self):
         """