moodlehq · kabalin · Feb 21, 2019 · Feb 21, 2019
diff --git a/mustache_lint/mustache_lint.php b/mustache_lint/mustache_lint.php
@@ -192,16 +192,59 @@ function print_message($severity, $mesage) {
 }
 
 /**
- * Wrap the template content in a html5 wrapper and validate it
+ * Fix partials and wrap the template content in a html5 wrapper.
+ *
+ * There could be the case when partial template got elements that have a strict
+ * parent element requirement. We can wrap most obvious elements into correct parent,
+ * assuming the partial template is "balanced" i.e. contain equal number of start
+ * and end tags (though, if it is not equial, the partial template will fail
+ * validation anyway). This partial fixing wrapper will not be required if we
+ * find a way to exclude partials from linting at some point (MDL-56504).
+ *
+ * @param string $content the raw template as string.
+ * @return string $wrappedcontent
  */
-function check_html_validation($content) {
-    if (strpos($content, '<head>') === false) {
-        // Primative detection if we have full html body, if not, wrap it.
-        // (This isn't bulletproof, obviously).
-        $wrappedcontent = "<!DOCTYPE html><head><title>Validate</title></head><body>\n{$content}\n</body></html>";
-    } else {
-        $wrappedcontent = $content;
+function wrap_content($content) {
+    // Primitive detection if we have full html body, if not, we need to wrap it.
+    if (strpos($content, '<head>')) {
+        return $content;
+    }
+
+    // Mapping of child elements to their required parent elements.
+    $reqparent = [
+        // List.
+        'li' => 'ul',
+        // Table.
+        'td' => 'tr',
+        'th' => 'tr',
+        'tr' => 'table',
+        'thead' => 'table',
+        'tbody' => 'table',
+        'tfoot' => 'table',
+        // Definition list.
+        'dd' => 'dl',
+        'dt' => 'dl',
+    ];
+
+    // Determine if the first element has strict parent element requirement and wrap it if necessary.
+    // Iterate through, as further wrapping may be needed.
+    while (preg_match('@^<(' . join('|', array_keys($reqparent)) . ')[^>]*>@is', trim($content), $matches)) {
+        $parent = $reqparent[$matches[1]];
+        $content = "<{$parent}>\n{$content}\n</{$parent}>";
     }
+
+    // Wrap in html5 body.
+    $wrappedcontent = "<!DOCTYPE html><head><title>Validate</title></head><body>\n{$content}\n</body></html>";
+    return $wrappedcontent;
+}
+
+/**
+ * Validate template content.
+ */
+function check_html_validation($content) {
+    // We need to wrap template content first.
+    $wrappedcontent = wrap_content($content);
+    // And then call validator.
     $response = validate_html($wrappedcontent);
 
     if (!$response || !isset($response->messages)) {

diff --git a/tests/1-mustache_lint.bats b/tests/1-mustache_lint.bats
@@ -76,7 +76,6 @@ setup () {
 
     # Assert result
     assert_failure
-    assert_output --partial "NPM installed validator found."
     assert_output --partial "Running mustache lint from $GIT_PREVIOUS_COMMIT to $GIT_COMMIT"
     assert_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: End tag “p” seen, but there were open elements. (ello World</p></bo)"
     assert_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: Unclosed element “span”. (<body><p><span>Hello )"
@@ -100,6 +99,24 @@ setup () {
     assert_output --partial "No mustache problems found"
 }
 
+@test "mustache_lint: Partial contains elements with strict parent requirement" {
+    # Set up.
+    git_apply_fixture 31-mustache_lint-partial.patch
+    export GIT_PREVIOUS_COMMIT=$FIXTURE_HASH_BEFORE
+    export GIT_COMMIT=$FIXTURE_HASH_AFTER
+
+    ci_run mustache_lint/mustache_lint.sh
+
+    # Assert result
+    assert_success
+    # We should not have a validation warning about stray tags.
+    refute_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: Stray start tag “td”."
+    refute_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: Stray end tag “td”."
+
+    assert_output --partial "lib/templates/linting.mustache - OK: Mustache rendered html succesfully"
+    assert_output --partial "No mustache problems found"
+}
+
 @test "mustache_lint: Full HTML page doesn't get embeded in <html> body" {
     # Set up.
     git_apply_fixture 31-mustache_lint-full-html-body.patch
@@ -110,7 +127,7 @@ setup () {
 
     # Assert result
     assert_success
-    # We should not have a vlidation warning about multiple 'html' tags.
+    # We should not have a validation warning about multiple 'html' tags.
     refute_output --partial 'Stray start tag “html”.'
 
     assert_output --partial "lib/templates/full-html-page.mustache - OK: Mustache rendered html succesfully"

diff --git a/tests/fixtures/31-mustache_lint-partial.patch b/tests/fixtures/31-mustache_lint-partial.patch
@@ -0,0 +1,53 @@
+From 21dcf559cc7850d03729f7679e2f1ce87db5cb4c Mon Sep 17 00:00:00 2001
+From: Ruslan Kabalin <[email protected]>
+Date: Fri, 18 Jan 2019 18:26:08 +0000
+Subject: [PATCH] MDL-56504 mustache: fixture for partial problem detection
+
+---
+ lib/templates/linting.mustache | 35 +++++++++++++++++++++++++++++++++++
+ 1 file changed, 35 insertions(+)
+ create mode 100644 lib/templates/linting.mustache
+
+diff --git a/lib/templates/linting.mustache b/lib/templates/linting.mustache
+new file mode 100644
+index 00000000000..dffe9239d92
+--- /dev/null
++++ b/lib/templates/linting.mustache
+@@ -0,0 +1,35 @@
++{{!
++    This file is part of Moodle - http://moodle.org/
++
++    Moodle is free software: you can redistribute it and/or modify
++    it under the terms of the GNU General Public License as published by
++    the Free Software Foundation, either version 3 of the License, or
++    (at your option) any later version.
++
++    Moodle is distributed in the hope that it will be useful,
++    but WITHOUT ANY WARRANTY; without even the implied warranty of
++    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++    GNU General Public License for more details.
++
++    You should have received a copy of the GNU General Public License
++    along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
++}}
++{{!
++    @template core/linting
++
++    A test for for mustache linting tests.
++
++    Classes required for JS:
++    * none
++
++    Data attributes required for JS:
++    * none
++
++    Context variables required for this template:
++    * none
++
++    Example context (json):
++    {
++    }
++}}
++<td>Hello World</td>
+--
+2.11.0