Repository: ember-template-lint/ember-template-lint
Pull request #460 (2018-07-25 15:54:57)

Fix issue with attribute-indentation and ElementNode's without attributes.
After releasing 1.0.0-beta.1 and updating a few small apps I identified an issue in the logic for validating an element's attributes. Essentially, if there were no attributes we would attempt to call `node.params.length` (and `node.params` is `undefined`).

This fixes the issue by:

* simplify `validateNonBlockForm` and `validateBlockForm` by moving `ElementNode` specific code into the `ElementNode` visitor directly.
* correct issues with detecting the "end" of the open tag for `ElementNode`'s (once the prior issue was fixed this cropped up) by extracting a specific method for `getEndLocationForOpen`.


Patch:
diff --git a/lib/rules/lint-attribute-indentation.js b/lib/rules/lint-attribute-indentation.js
index 8a2d9a24..0ee82633 100644
--- a/lib/rules/lint-attribute-indentation.js
+++ b/lib/rules/lint-attribute-indentation.js
@@ -8,11 +8,8 @@ const getWhiteSpaceLength = function(statement) {
   return (whiteSpace[0] || '').length;
 };
 
-const canApplyRule = function(node, type, config) {
-  let end;
-  let start = node.loc.start;
-
-  if (type === 'BlockStatement') {
+function getEndLocationForOpen(node) {
+  if (node.type === 'BlockStatement') {
     /*
       For a block statement, the start of the program block is the end of the open invocation.
 
@@ -21,10 +18,17 @@ const canApplyRule = function(node, type, config) {
       {{/contact-details}}
     */
 
-    end = node.program.loc.start;
+    return node.program.loc.start;
+  } else if (node.type === 'ElementNode' && node.children.length > 0) {
+    return node.children[0].loc.start;
   } else {
-    end = node.loc.end;
+    return node.loc.end;
   }
+}
+
+const canApplyRule = function(node, type, config) {
+  let end = getEndLocationForOpen(node);
+  let start = node.loc.start;
 
   if (start.line === end.line) {
     return (end.column - start.column)  > config.maxLength;
@@ -303,10 +307,11 @@ module.exports = class AttributeSpacing extends Rule {
     /*
       Validates the close brace `}}` for Handlebars and `>` for HTML/SVG elements of the non-block form.
     */
+    let end = getEndLocationForOpen(node);
     const actualColumnStartLocation = node.type === 'ElementNode' && !node.selfClosing ? 1 : 2;
     const actualStartLocation = {
-      line: node.loc.end.line,
-      column: node.loc.end.column - actualColumnStartLocation
+      line: end.line,
+      column: end.column - actualColumnStartLocation
     };
 
     const expectedStartLocation = {
@@ -363,31 +368,19 @@ module.exports = class AttributeSpacing extends Rule {
   }
 
   validateNonBlockForm(node) {
-    if (node.type === 'ElementNode') {
-      if (node.attributes.length) {
-        const expectedStartLine = this.validateParams(node);
-        this.validateCloseBrace(node, expectedStartLine);
-        return expectedStartLine;
-      }
-    } else {
-      // no need to validate if no positional and named params are present.
-      if (node.params.length || node.hash.pairs.length) {
-        const expectedStartLine = this.validateParams(node);
-        this.validateCloseBrace(node, expectedStartLine);
-        return expectedStartLine;
-      }
+    // no need to validate if no positional and named params are present.
+    if (node.params.length || node.hash.pairs.length) {
+      const expectedStartLine = this.validateParams(node);
+      this.validateCloseBrace(node, expectedStartLine);
+      return expectedStartLine;
     }
   }
 
   validateBlockForm(node) {
-    if ((node.type === 'ElementNode' && node.attributes.length) || node.params.length || node.hash.pairs.length) {
+    if (node.params.length || node.hash.pairs.length) {
       this.validateParams(node);
     }
-    if (node.type === 'ElementNode') {
-      const lastChild = node.children[node.children.length - 1];
-      const expectedStartLine = lastChild.type === 'BlockStatement' ? lastChild.loc.end.line + 1 : lastChild.loc.end.line;
-      this.validateClosingTag(node, expectedStartLine);
-    } else if (node.program.blockParams && node.program.blockParams.length) {
+    if (node.program.blockParams && node.program.blockParams.length) {
       this.validateBlockParams(node);
     }
   }
@@ -401,7 +394,7 @@ module.exports = class AttributeSpacing extends Rule {
         return {
           maxLength: 80,
           indentation: 2,
-          'process-elements': true,
+          processElements: true,
         };
       }
       return false;
@@ -452,10 +445,15 @@ module.exports = class AttributeSpacing extends Rule {
       ElementNode(node) {
         if (this.config.processElements) {
           if (canApplyRule(node, node.type, this.config)) {
+            if (node.attributes.length) {
+              let expectedCloseBraceLine = this.validateParams(node);
+              this.validateCloseBrace(node, expectedCloseBraceLine);
+            }
+
             if (node.children.length) {
-              return this.validateBlockForm(node);
-            } else {
-              return this.validateNonBlockForm(node);
+              const lastChild = node.children[node.children.length - 1];
+              const expectedStartLine = lastChild.type === 'BlockStatement' ? lastChild.loc.end.line + 1 : lastChild.loc.end.line;
+              this.validateClosingTag(node, expectedStartLine);
             }
           }
           return node.loc.end.line;


Test patch:
diff --git a/test/unit/recommended-config-test.js b/test/unit/recommended-config-test.js
index 61f1c3c2..870c8399 100644
--- a/test/unit/recommended-config-test.js
+++ b/test/unit/recommended-config-test.js
@@ -56,7 +56,8 @@ describe('recommended config', function() {
   @selected={{@pageSize}}
   @options={{this.availablePageSizes}}
   @searchEnabled={{false}}
-  @onchange={{action this.changePageSize}} as |size|>
+  @onchange={{action this.changePageSize}} as |size|
+>
   {{size}}
 </PowerSelect>`);
 });
diff --git a/test/unit/rules/lint-attribute-indentation-test.js b/test/unit/rules/lint-attribute-indentation-test.js
index a3c896a0..4fafc2cc 100644
--- a/test/unit/rules/lint-attribute-indentation-test.js
+++ b/test/unit/rules/lint-attribute-indentation-test.js
@@ -14,18 +14,15 @@ generateRuleTests({
         'process-elements': true
       },
       template: '<SiteHeader' + '\n' +
-      '  @selected={{this.user.country}} as |Option|>' + '\n' +
-      '{{#each this.availableCountries as |country|}}'  + '\n' +
+      '  @selected={{this.user.country}} as |Option|' + '\n' +
+      '>{{#each this.availableCountries as |country|}}'  + '\n' +
       '<Option @value={{country}}>{{country.name}}</Option>' + '\n' +
       '{{/each}}' + '\n' +
       '</SiteHeader>',
     },
     //Non Block form one line
     '<input disabled>',
-    //Non Block with wrong indentation, configuration off
-    '<input' + '\n' +
-    'disabled' + '\n' +
-    '>',
+
     //Non Block with wrong indentation, configuration explicitly off
     {
       config: {
@@ -298,6 +295,7 @@ generateRuleTests({
     'as |contact|}}' + '\n' +
     '  {{contact.fullName}}' + '\n' +
     '{{/contact-details}}',
+    '<div>\n  <p></p>\n</div>',
   ],
 
   bad: [{
@@ -461,6 +459,14 @@ generateRuleTests({
       'line': 1,
       'column': 75,
       'source': '<a href="https://www.emberjs.com" class="emberjs-home link" rel="noopener" target="_blank">Ember JS</a>'
+    }, {
+      'rule': 'attribute-indentation',
+      'severity': 2,
+      'moduleId': 'layout.hbs',
+      'message': `Incorrect indentation of close bracket '>' for the element '<a>' beginning at L1:C90. Expected '<a>' to be at L6:C0.`,
+      'line': 1,
+      'column': 90,
+      'source': '<a href="https://www.emberjs.com" class="emberjs-home link" rel="noopener" target="_blank">Ember JS</a>'
     }, {
       'rule': 'attribute-indentation',
       'severity': 2,