Repository: rwjblue/ember-cli-template-lint
Pull request #80 (2016-04-16 14:59:28)

Implement attribute checking for `bare-strings` rule.
These changes update the `bare-strings` rule to allow the following configuration:

   * boolean -- `true` for enabled / `false` for disabled
   * array -- an array of whitelisted strings
   * object -- An object with the following keys:
     * `whitelist` -- An array of whitelisted strings
     * `globalAttributes` -- An array of attributes to check on every element.
     * `elementAttributes` -- An object whose keys are tag names and value is an array of attributes to check for that tag name.

We have also update the default blueprinted value to be `true`, which defaults the configuration to:

```js
{
  whitelist: ['(', ')', ',', '.', '&', '+', '-', '=', '*', '/', '#', '%', '!', '?', ':', '[', ']', '{', '}'],
  globalAttributes: GLOBAL_ATTRIBUTES,
  elementAttributes: TAG_ATTRIBUTES
};
```


Patch:
diff --git a/README.md b/README.md
index 5e4742d..ade2362 100644
--- a/README.md
+++ b/README.md
@@ -74,13 +74,19 @@ This rule forbids the following:
 <h2>Some string here!</h2>
 ```
 
-The following values are valid configuration:
-
-  * boolean -- `true` for enabled / `false` for disabled
-  * array -- an array of whitelisted strings
-
-By default, the following characters are whitelisted:
-`(),.&+-=*/#%!?:[]{}`
+ The following values are valid configuration:
+
+   * boolean -- `true` for enabled / `false` for disabled
+   * array -- an array of whitelisted strings
+   * object -- An object with the following keys:
+     * `whitelist` -- An array of whitelisted strings
+     * `globalAttributes` -- An array of attributes to check on every element.
+     * `elementAttributes` -- An object whose keys are tag names and value is an array of attributes to check for that tag name.
+
+When the config value of `true` is used the following configuration is used:
+ * `whitelist` - `(),.&+-=*/#%!?:[]{}`
+ * `globalAttributes` - `title`
+ * `elementAttributes` - `{ img: ['alt'], input: ['placeholder'] }`
 
 
 #### block-indentation
diff --git a/blueprints/ember-cli-template-lint/files/.template-lintrc.js b/blueprints/ember-cli-template-lint/files/.template-lintrc.js
index e86e060..715dcfd 100644
--- a/blueprints/ember-cli-template-lint/files/.template-lintrc.js
+++ b/blueprints/ember-cli-template-lint/files/.template-lintrc.js
@@ -2,7 +2,7 @@
 'use strict';
 
 module.exports = {
-  'bare-strings': ['(', ')', ',', '.', '&', '+', '-', '=', '*', '/', '#', '%', '!', '?', ':', '[', ']', '{', '}'],
+  'bare-strings': true,
   'block-indentation': 2,
   'html-comments': true,
   'nested-interactive': true,
diff --git a/ext/helpers/ast-node-info.js b/ext/helpers/ast-node-info.js
index c0444d9..c6c0a30 100644
--- a/ext/helpers/ast-node-info.js
+++ b/ext/helpers/ast-node-info.js
@@ -10,4 +10,12 @@ AstNodeInfo.isNonConfigurationHtmlComment = function(node) {
   return node.type === 'CommentStatement' && node.value.trim().indexOf('template-lint ') !== 0;
 };
 
+AstNodeInfo.isTextNode = function(node) {
+  return node.type === 'TextNode';
+};
+
+AstNodeInfo.isElementNode = function(node) {
+  return node.type === 'ElementNode';
+};
+
 module.exports = AstNodeInfo;
diff --git a/ext/plugins/lint-bare-strings.js b/ext/plugins/lint-bare-strings.js
index c9fb572..e356445 100644
--- a/ext/plugins/lint-bare-strings.js
+++ b/ext/plugins/lint-bare-strings.js
@@ -16,10 +16,50 @@
 
    * boolean -- `true` for enabled / `false` for disabled
    * array -- an array of whitelisted strings
+   * object -- An object with the following keys:
+     * `whitelist` -- An array of whitelisted strings
+     * `globalAttributes` -- An array of attributes to check on every element.
+     * `elementAttributes` -- An object whose keys are tag names and value is an array of attributes to check for that tag name.
  */
 
 var calculateLocationDisplay = require('../helpers/calculate-location-display');
 var buildPlugin = require('./base');
+var astInfo = require('../helpers/ast-node-info');
+
+var GLOBAL_ATTRIBUTES = [
+  'title'
+];
+
+var TAG_ATTRIBUTES = {
+  'input': [ 'placeholder' ],
+  'img': [ 'alt' ]
+};
+
+var DEFAULT_CONFIG = {
+  whitelist: ['(', ')', ',', '.', '&', '+', '-', '=', '*', '/', '#', '%', '!', '?', ':', '[', ']', '{', '}'],
+  globalAttributes: GLOBAL_ATTRIBUTES,
+  elementAttributes: TAG_ATTRIBUTES
+};
+
+function isValidConfigObjectFormat(config) {
+  for (var key in config) {
+    var value = config[key];
+    var valueType = typeof value;
+    var valueIsArray = Array.isArray(value);
+
+    if (key === 'whitelist' && !valueIsArray) {
+      return false;
+    } else if (key === 'globalAttributes' && !valueIsArray) {
+      return false;
+    } else if (key === 'elementAttributes' && valueType === 'object') {
+      if (valueIsArray) { return false; }
+    } else if (!DEFAULT_CONFIG[key]){
+      return false;
+    }
+  }
+
+  return true;
+}
 
 module.exports = function(addonContext) {
   var LogStaticStrings = buildPlugin(addonContext, 'bare-strings');
@@ -30,14 +70,30 @@ module.exports = function(addonContext) {
     var errorMessage = 'The bare-strings rule accepts one of the following values.\n ' +
           '  * boolean - `true` to enable / `false` to disable\n' +
           '  * array -- an array of strings to whitelist\n' +
+          '  * object -- An object with the following keys:' +
+          '    * `whitelist` -- An array of whitelisted strings ' +
+          '    * `globalAttributes` -- An array of attributes to check on every element.' +
+          '    * `elementAttributes` -- An object whose keys are tag names and value is an array of attributes to check for that tag name. ' +
           '\nYou specified `' + JSON.stringify(config) + '`';
 
     switch (configType) {
     case 'boolean':
-      return config;
+      // if `true` use `DEFAULT_CONFIG`
+      return config ? DEFAULT_CONFIG : false;
     case 'object':
       if (Array.isArray(config)) {
-        return config;
+        return {
+          whitelist: config,
+          globalAttributes: GLOBAL_ATTRIBUTES,
+          elementAttributes: TAG_ATTRIBUTES
+        };
+      } else if (isValidConfigObjectFormat(config)) {
+        // default any missing keys to empty values
+        return {
+          whitelist: config.whitelist || [],
+          globalAttributes: config.globalAttributes || [],
+          elementAttributes: config.elementAttributes || {}
+        };
       } else {
         throw new Error(errorMessage);
       }
@@ -49,27 +105,58 @@ module.exports = function(addonContext) {
   };
 
   LogStaticStrings.prototype.process = function(node) {
-    var locationDisplay = calculateLocationDisplay(this.options.moduleName, node.loc && node.loc.start);
-    var warning = 'Non-translated string used ' + locationDisplay + ' `' + node.chars + '`';
-
-    this.log(warning);
+    if (astInfo.isTextNode(node)) {
+      this._checkNodeAndLog(node, '', node.loc);
+    } else if (astInfo.isElementNode(node)){
+      var tagName = node.tag;
+      for (var i = 0; i < node.attributes.length; i++) {
+        this._getBareStringAttribute(tagName, node.attributes[i]);
+      }
+    }
   };
 
-  LogStaticStrings.prototype.detect = function(node) {
-    if (node.type !== 'TextNode') { return false;}
+  LogStaticStrings.prototype._getBareStringAttribute = function(tag, attribute) {
+    var attributeType = attribute.name;
+    var attributeValueNode = attribute.value;
+    var additionalDescription = 'in `' + attributeType + '` attribute ';
+    var isGlobalAttribute = this.config.globalAttributes.indexOf(attributeType) > -1;
+    var isElementAttribute = this.config.elementAttributes[tag] && this.config.elementAttributes[tag].indexOf(attributeType) > -1;
 
-    var string = node.chars;
-    var whitelist = Array.isArray(this.config) ? this.config : null;
+    if (astInfo.isTextNode(attributeValueNode) && (isGlobalAttribute || isElementAttribute)) {
+      this._checkNodeAndLog(attributeValueNode, additionalDescription, attribute.loc);
+    }
+  };
+
+  LogStaticStrings.prototype._getBareString = function(_string) {
+    var whitelist = this.config.whitelist;
+    var string = _string;
 
     if (whitelist) {
       for (var i = 0; i < whitelist.length; i++) {
         var entry = whitelist[i];
 
-        string = string.replace(entry, '');
+        while (string.indexOf(entry) > -1) {
+          string = string.replace(entry, '');
+        }
       }
     }
 
-    return string.trim() !== '';
+    return string.trim() !== '' ? _string : null;
+  };
+
+  LogStaticStrings.prototype._checkNodeAndLog = function(node, additionalDescription, loc) {
+    var bareStringText = this._getBareString(node.chars);
+
+    if (bareStringText) {
+      var locationDisplay = calculateLocationDisplay(this.options.moduleName, loc && loc.start);
+      var warning = 'Non-translated string used ' + additionalDescription + locationDisplay + ': `' + bareStringText + '`.';
+
+      this.log(warning);
+    }
+  };
+
+  LogStaticStrings.prototype.detect = function(node) {
+    return astInfo.isElementNode(node) || astInfo.isTextNode(node);
   };
 
   return LogStaticStrings;


Test patch:
diff --git a/node-tests/helpers/rule-test-harness.js b/node-tests/helpers/rule-test-harness.js
index 16b1608..1e2db75 100644
--- a/node-tests/helpers/rule-test-harness.js
+++ b/node-tests/helpers/rule-test-harness.js
@@ -40,6 +40,7 @@ module.exports = function(options) {
 
     options.bad.forEach(function(badItem) {
       var testMethod = badItem.focus ? it.only : it;
+      var expectedMessages = badItem.messages || [badItem.message];
 
       testMethod('logs a message in the console when given `' + badItem.template + '`', function() {
         if (badItem.config) {
@@ -48,7 +49,7 @@ module.exports = function(options) {
 
         compile(badItem.template);
 
-        assert.deepEqual(messages, [badItem.message]);
+        assert.deepEqual(messages, expectedMessages);
       });
 
       it('passes with `' + badItem.template + '` when rule is disabled', function() {
@@ -73,12 +74,13 @@ module.exports = function(options) {
 
     options.good.forEach(function(item) {
       var template = typeof item === 'object' ? item.template : item;
+      var testMethod = typeof item === 'object' && item.focus ? it.only : it;
 
-      it('passes when given `' + template + '`', function() {
+      testMethod('passes when given `' + template + '`', function() {
         if (typeof item === 'string') {
           compile(item);
         } else {
-          if (item.config) {
+          if (item.config !== undefined) {
             config[options.name] = item.config;
           }
 
diff --git a/node-tests/unit/plugins/lint-bare-strings-test.js b/node-tests/unit/plugins/lint-bare-strings-test.js
index 7ded69d..b544de2 100644
--- a/node-tests/unit/plugins/lint-bare-strings-test.js
+++ b/node-tests/unit/plugins/lint-bare-strings-test.js
@@ -5,7 +5,7 @@ var generateRuleTests = require('../../helpers/rule-test-harness');
 generateRuleTests({
   name: 'bare-strings',
 
-  config: ['(', ')', ',', '.', '&', '+', '-', '=', '*', '/', '#', '%', '!', '?', ':', '[', ']', '{', '}'],
+  config: true,
 
   good: [
     '{{t "howdy"}}',
@@ -17,6 +17,10 @@ generateRuleTests({
       config: ['foo'],
       template: '\nfoo'
     },
+    {
+      config: ['tarzan!'],
+      template: 'tarzan!\t\n  tarzan!'
+    },
     {
       config: ['/', '"'],
       template: '{{t "foo"}} / "{{name}}"'
@@ -25,11 +29,73 @@ generateRuleTests({
     '{{t "foo"}}, {{t "bar"}} ({{length}})',
     '(),.&+-=*/#%!?:[]{}',
     '<!-- template-lint bare-strings=false -->',
-    '<!-- template-lint enabled=false -->'
+    '<!-- template-lint enabled=false -->',
+
+    // placeholder is a <input> specific attribute
+    '<div placeholder="wat?"></div>',
+
+    {
+      // config as array is whitelist of chars
+      config: ['/', '"'],
+      template: '{{t "foo"}} / "{{name}}"'
+    },
+
+    {
+      config: true,
+      template: '\n {{translate "greeting"}},'
+    },
+
+    {
+      config: false,
+      template: '\nfoobar'
+    },
+
+    {
+      // override the globalAttributes list
+      config: { globalAttributes: [] },
+      template: '<a title="hahaha trolol"></a>'
+    },
+
+    {
+      // override the elementAttributes list
+      config: { elementAttributes: { }},
+      template: '<input placeholder="hahaha">'
+    }
   ],
 
   bad: [
-    { 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`" }
+    { 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`." },
+
+    {
+      template: '<a title="hahaha trolol"></a>',
+      message: "Non-translated string used in `title` attribute ('layout.hbs'@ L1:C4): `hahaha trolol`."
+    },
+
+    {
+      template: '<input placeholder="trolol">',
+      message: "Non-translated string used in `placeholder` attribute ('layout.hbs'@ L1:C8): `trolol`."
+    },
+
+    {
+      config: { globalAttributes: ['data-foo'] },
+      template: '<div data-foo="derpy"></div>',
+      message: "Non-translated string used in `data-foo` attribute ('layout.hbs'@ L1:C6): `derpy`."
+    },
+
+    {
+      config: { elementAttributes: { img: ['data-alt']} },
+      template: '<img data-alt="some alternate here">',
+      message: "Non-translated string used in `data-alt` attribute ('layout.hbs'@ L1:C6): `some alternate here`."
+    },
+
+    {
+      // multiple bare strings are all logged
+      template: '<div>Bady\n  <input placeholder="trolol">\n</div>',
+      messages: [
+        "Non-translated string used (\'layout.hbs\'@ L1:C5): `Bady\n  `.",
+        "Non-translated string used in `placeholder` attribute ('layout.hbs'@ L2:C10): `trolol`."
+      ]
+    }
   ]
 });