Repository: ember-template-lint/ember-template-lint
Pull request #846 (2019-10-06 20:28:00)

Update `table-groups` to enforce required ordering of table children
This makes sure that a table groups are in the correct order according to the spec. It only verifies the order, not the cardinality of the table-groups.

Fixed the ordering part of #285. Not sure if we should also lint the carinality of the groups (some groups are only allowed once, some must appear, some zero to many times), e.g., should the linter warn when a table has two tbody groups?


Patch:
diff --git a/lib/rules/lint-table-groups.js b/lib/rules/lint-table-groups.js
index 86d085ad..4277fd91 100644
--- a/lib/rules/lint-table-groups.js
+++ b/lib/rules/lint-table-groups.js
@@ -4,8 +4,10 @@ const AstNodeInfo = require('../helpers/ast-node-info');
 const Rule = require('./base');
 
 const message = 'Tables must have a table group (thead, tbody or tfoot).';
+const orderingMessage =
+  'Tables must have table groups in the correct order (caption, colgroup, thead, tbody then tfoot).';
 
-const ALLOWED_TABLE_CHILDREN = ['thead', 'tbody', 'tfoot', 'caption', 'colgroup'];
+const ALLOWED_TABLE_CHILDREN = ['caption', 'colgroup', 'thead', 'tbody', 'tfoot'];
 
 // For effective children, we skip over any control flow helpers,
 // since we know that they don't render anything on their own
@@ -24,33 +26,37 @@ function getEffectiveChildren(node) {
 function isAllowedTableChild(node) {
   let tagNamePair;
   let tagNameAttribute;
+  let index;
   switch (node.type) {
     case 'BlockStatement':
     case 'MustacheStatement':
       tagNamePair = node.hash.pairs.find(pair => pair.key === 'tagName');
       if (tagNamePair) {
-        return ALLOWED_TABLE_CHILDREN.includes(tagNamePair.value.value);
+        index = ALLOWED_TABLE_CHILDREN.indexOf(tagNamePair.value.value);
+        return { allowed: index > -1, index };
       } else if (node.path.original === 'yield') {
-        return true;
+        return { allowed: true, index: -1 };
       }
       break;
     case 'ElementNode':
-      if (ALLOWED_TABLE_CHILDREN.includes(node.tag)) {
-        return true;
+      index = ALLOWED_TABLE_CHILDREN.indexOf(node.tag);
+      if (index > -1) {
+        return { allowed: true, index };
       }
       tagNameAttribute = node.attributes.find(attribute => attribute.name === '@tagName');
       if (tagNameAttribute) {
-        return ALLOWED_TABLE_CHILDREN.includes(tagNameAttribute.value.chars);
+        index = ALLOWED_TABLE_CHILDREN.indexOf(tagNameAttribute.value.chars);
+        return { allowed: index > -1, index };
       }
       break;
     case 'CommentStatement':
     case 'MustacheCommentStatement':
-      return true;
+      return { allowed: true, index: -1 };
     case 'TextNode':
-      return !/\S/.test(node.chars);
+      return { allowed: !/\S/.test(node.chars), index: -1 };
   }
 
-  return false;
+  return { allowed: false };
 }
 
 module.exports = class TableGroups extends Rule {
@@ -58,13 +64,29 @@ module.exports = class TableGroups extends Rule {
     return {
       ElementNode(node) {
         if (node.tag === 'table') {
-          if (!getEffectiveChildren(node).every(isAllowedTableChild)) {
-            this.log({
-              message,
-              line: node.loc && node.loc.start.line,
-              column: node.loc && node.loc.start.column,
-              source: this.sourceForNode(node),
-            });
+          const children = getEffectiveChildren(node);
+          let currentAllowedIndex = 0;
+          for (let i = 0; i < children.length; i++) {
+            const allowedWithIndex = isAllowedTableChild(children[i]);
+            if (!allowedWithIndex.allowed) {
+              this.log({
+                message,
+                line: node.loc && node.loc.start.line,
+                column: node.loc && node.loc.start.column,
+                source: this.sourceForNode(node),
+              });
+              break;
+            }
+            if (allowedWithIndex.index > -1 && allowedWithIndex.index < currentAllowedIndex) {
+              this.log({
+                message: orderingMessage,
+                line: node.loc && node.loc.start.line,
+                column: node.loc && node.loc.start.column,
+                source: this.sourceForNode(node),
+              });
+              break;
+            }
+            currentAllowedIndex = allowedWithIndex.index;
           }
         }
       },
@@ -73,3 +95,4 @@ module.exports = class TableGroups extends Rule {
 };
 
 module.exports.message = message;
+module.exports.orderingMessage = orderingMessage;


Test patch:
diff --git a/test/unit/rules/lint-table-groups-test.js b/test/unit/rules/lint-table-groups-test.js
index 8ccede12..f032adeb 100644
--- a/test/unit/rules/lint-table-groups-test.js
+++ b/test/unit/rules/lint-table-groups-test.js
@@ -2,6 +2,7 @@
 
 const generateRuleTests = require('../../helpers/rule-test-harness');
 const message = require('../../../lib/rules/lint-table-groups').message;
+const orderingMessage = require('../../../lib/rules/lint-table-groups').orderingMessage;
 
 generateRuleTests({
   name: 'table-groups',
@@ -149,6 +150,7 @@ generateRuleTests({
       '</tbody>' +
       '</table>',
     '<table>\n' + '<tbody>\n' + '</tbody>\n' + '</table>',
+    '<table><colgroup></colgroup><colgroup></colgroup><tbody></tbody></table>',
   ],
 
   bad: [
@@ -393,5 +395,35 @@ generateRuleTests({
         column: 0,
       },
     },
+    {
+      template: '<table><tfoot /><thead /></table>',
+      result: {
+        message: orderingMessage,
+        moduleId: 'layout.hbs',
+        source: '<table><tfoot /><thead /></table>',
+        line: 1,
+        column: 0,
+      },
+    },
+    {
+      template: '<table><tbody /><caption /></table>',
+      result: {
+        message: orderingMessage,
+        moduleId: 'layout.hbs',
+        source: '<table><tbody /><caption /></table>',
+        line: 1,
+        column: 0,
+      },
+    },
+    {
+      template: '<table><tbody /><colgroup /></table>',
+      result: {
+        message: orderingMessage,
+        moduleId: 'layout.hbs',
+        source: '<table><tbody /><colgroup /></table>',
+        line: 1,
+        column: 0,
+      },
+    },
   ],
 });