Repository: rwjblue/ember-cli-template-lint (js)

Issue:
Newest htmlbars release (0.14.17) breaks block indentation rule
[This change](https://github.com/tildeio/htmlbars/commit/29a77fb596059d207e95df9f7d2fc40e6c77f120#diff-77a79c73c5f2afcef7e58ed15c331b3bR35) breaks the block indentation rule because now [this check](https://github.com/rwjblue/ember-cli-template-lint/blob/master/ext/plugins/lint-block-indentation.js#L174) passes for text nodes. This is because this:

```handlebars
{{#if foo}}
  {{foo}}
{{/if}}
```

is a MustacheStatement whose first child is a TextNode containing the two spaces before the ```{{foo}}```. Those two spaces' location points to column 0, which the block indentation logic flags as incorrect, and then when trying to log the error, throws an exception on [this line](https://github.com/rwjblue/ember-cli-template-lint/blob/master/ext/plugins/lint-block-indentation.js#L216) because text nodes don't have a path attribute.

I think the fix is to look for an explicit list of node types instead of the ```!child.loc``` check, but I don't have time to prepare a fix right now. If somebody else does, great, otherwise maybe we can just pin the version of htmlbars to 0.14.16 until we have time to get in a fix?

Pull request description:
Fix issues from adding text/comment node location in HTMLBars.
Fixes #78.

Patch:
diff --git a/ext/plugins/lint-block-indentation.js b/ext/plugins/lint-block-indentation.js
index a70a2b8..4b4a4a7 100644
--- a/ext/plugins/lint-block-indentation.js
+++ b/ext/plugins/lint-block-indentation.js
@@ -79,6 +79,10 @@ function childrenFor(node) {
   }
 }
 
+function isTextNode(node) {
+  return node.type === 'TextNode';
+}
+
 module.exports = function(addonContext) {
   var BlockIndentation = buildPlugin(addonContext, 'block-indentation');
 
@@ -179,7 +183,7 @@ module.exports = function(addonContext) {
       var hasLeadingContent = false;
       for (var j = i - 1; j >= 0; j--) {
         var sibling = children[j];
-        if (sibling.loc) {
+        if (sibling.loc && !isTextNode(sibling)) {
           // Found an element or statement. If it's on this line, then we
           // have leading content, so set the flag and break. If it's not
           // on this line, then we've scanned back to a previous line, so
@@ -210,20 +214,32 @@ module.exports = function(addonContext) {
         continue;
       }
 
-      var childStartColumn = child.loc.start.column;
+      var childStartColumn;
+      if (isTextNode(child)) {
+        var withoutLeadingNewLines = child.chars.replace(/^(\r\n|\n)*/, '');
+        var firstNonWhitespace = withoutLeadingNewLines.search(/\S/);
+
+        if (firstNonWhitespace === -1) { continue; }
+
+        childStartColumn = firstNonWhitespace;
+      } else {
+        childStartColumn = child.loc.start.column;
+      }
+
       if (expectedStartColumn !== childStartColumn) {
         var isElementNode = child.type === 'ElementNode';
-        var displayName = isElementNode ? child.tag : child.path.original;
         var display;
 
         if (isElementNode) {
-          display = '<' + displayName + '>';
+          display = '<' + child.tag + '>';
         } else if (child.type === 'BlockStatement'){
-          display = '{{#' + displayName + '}}';
+          display = '{{#' + child.path.original + '}}';
         } else if (child.type === 'MustacheStatement') {
-          display = '{{' + displayName + '}}';
+          display = '{{' + child.path.original + '}}';
+        } else if (isTextNode(child)) {
+          display = child.chars;
         } else {
-          display = displayName;
+          display = child.path.original;
         }
 
         var startLocation = calculateLocationDisplay(this.options.moduleName, child.loc && child.loc.start);
@@ -231,6 +247,7 @@ module.exports = function(addonContext) {
         var warning = 'Incorrect indentation for `' + display + '` beginning at ' + startLocation +
             '. Expected `' + display + '` to be at an indentation of ' + expectedStartColumn + ' but ' +
             'was found at ' + childStartColumn + '.';
+
         this.log(warning);
       }
     }
diff --git a/package.json b/package.json
index 1840806..df97fb8 100644
--- a/package.json
+++ b/package.json
@@ -59,7 +59,7 @@
     "ember-cli-babel": "^5.1.6",
     "exists-sync": "0.0.3",
     "hash-for-dep": "^1.0.2",
-    "htmlbars": "^0.14.16",
+    "htmlbars": "^0.14.17",
     "js-string-escape": "^1.0.0",
     "json-stable-stringify": "^1.0.1",
     "md5-hex": "^1.2.1"


Test patch:
diff --git a/node-tests/unit/plugins/lint-bare-strings-test.js b/node-tests/unit/plugins/lint-bare-strings-test.js
index 08bec12..7ded69d 100644
--- a/node-tests/unit/plugins/lint-bare-strings-test.js
+++ b/node-tests/unit/plugins/lint-bare-strings-test.js
@@ -29,7 +29,7 @@ generateRuleTests({
   ],
 
   bad: [
-    { template: '\n howdy', message: "Non-translated string used (\'layout.hbs\') `\n howdy`" },
-    { template: '1234', message: "Non-translated string used (\'layout.hbs\') `1234`" }
+    { template: '\n howdy', message: "Non-translated string used (\'layout.hbs\'@ L1:C0) `\n howdy`" },
+    { template: '<div>\n  1234\n</div>', message: "Non-translated string used (\'layout.hbs\'@ L1:C5) `\n  1234\n`" }
   ]
 });
diff --git a/node-tests/unit/plugins/lint-block-indentation-test.js b/node-tests/unit/plugins/lint-block-indentation-test.js
index cfbc8b9..1cb4f85 100644
--- a/node-tests/unit/plugins/lint-block-indentation-test.js
+++ b/node-tests/unit/plugins/lint-block-indentation-test.js
@@ -158,7 +158,7 @@ generateRuleTests({
       // has other content preceding it on its line
       template: '{{#if foo}}\n' +
         '  {{foo}} <p>\n' +
-        '    Bar\n' +
+        '            Bar\n' +
         '  </p>\n' +
         '{{/if}}',
       message: "Incorrect indentation for `p` beginning at ('layout.hbs'@ L2:C10). Expected `</p>` ending at ('layout.hbs'@ L4:C6) to be at an indentation of 10 but was found at 2."
diff --git a/node-tests/unit/plugins/lint-html-comments-test.js b/node-tests/unit/plugins/lint-html-comments-test.js
index 0c7c5e3..178145f 100644
--- a/node-tests/unit/plugins/lint-html-comments-test.js
+++ b/node-tests/unit/plugins/lint-html-comments-test.js
@@ -17,12 +17,12 @@ generateRuleTests({
   bad: [
     {
       template: '<!-- comment here -->',
-      message: 'Html comment detected `<!-- comment here -->` at (\'layout.hbs\'). ' +
+      message: 'Html comment detected `<!-- comment here -->` at (\'layout.hbs\'@ L1:C3). ' +
         'Use Handlebars comment instead `{{!-- comment here --}}`'
     },
     {
       template: '<!--comment here-->',
-      message: 'Html comment detected `<!--comment here-->` at (\'layout.hbs\'). ' +
+      message: 'Html comment detected `<!--comment here-->` at (\'layout.hbs\'@ L1:C3). ' +
         'Use Handlebars comment instead `{{!--comment here--}}`'
     }
   ]