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

Issue:
Rule: Disallow `{{unless}}`
Good:

``` hbs
{{#unless somePredicate}}
{{/unless}}

<div class="{{unless foo 'no-foo'}}"></div>
```

Bad:

``` hbs
{{#unless (and foo bar)}}
{{/unless}}
```

``` hbs
{{#unless foo}}
{{else}}
{{/unless}}
```

``` hbs
{{#if foo}}
{{else unless bar}}
{{/if}}
```

Mostly because it is impossible to reason about....


Pull request description:
Rule disallow `{{unless}}`
Fixes #30

Patch:
diff --git a/README.md b/README.md
index 22ca41c..7d27cef 100644
--- a/README.md
+++ b/README.md
@@ -488,6 +488,44 @@ whereas this is allowed:
 <div style={{make-background url}}>
 ```
 
+#### unless-helper (default === false)
+
+This rule strongly advises against `{{unless}}` blocks used in conjunction with other
+block helpers (e.g. `{{else}}`, `{{else if}}`), and template helpers.
+
+For example, the rule forbids against the following:
+
+``` hbs
+{{! `if` template helper}}
+
+{{unless (if true) "This is not recommended"}}
+```
+
+``` hbs
+{{! `else` block}}
+
+{{#unless bandwagoner}}
+  Go Niners!
+{{else}}
+  Go Seahawks!
+{{/unless
+```
+
+Common solutions are to use an `{{if}}` block, or refactor potentially confusing
+logic within the template.
+
+``` hbs
+{{#if bandwagoner}}
+  Go Blue Jays!
+{{else}}
+  Go Mariners!
+{{/unless
+```
+
+The following values are valid configuration:
+
+  * boolean -- `true` for enabled / `false` for disabled
+
 #### unused-block-params
 
 This rule forbids unused block parameters except when they are needed to access a later parameter.
diff --git a/lib/config/recommended.js b/lib/config/recommended.js
index a51355a..91d1824 100644
--- a/lib/config/recommended.js
+++ b/lib/config/recommended.js
@@ -14,6 +14,7 @@ module.exports = {
     'img-alt-attributes': true,
     'style-concatenation': true,
     'deprecated-inline-view-helper': true,
+    'unless-helper': false,
     'unused-block-params': true
   }
 };
diff --git a/lib/rules/index.js b/lib/rules/index.js
index 5863bf6..69af2e1 100644
--- a/lib/rules/index.js
+++ b/lib/rules/index.js
@@ -17,5 +17,6 @@ module.exports = {
   'deprecated-inline-view-helper': require('./deprecations/lint-deprecated-inline-view-helper'),
   'invalid-interactive': require('./lint-invalid-interactive'),
   'style-concatenation': require('./lint-style-concatenation'),
+  'unless-helper': require('./lint-unless-helper'),
   'unused-block-params': require('./lint-unused-block-params')
 };
diff --git a/lib/rules/lint-unless-helper.js b/lib/rules/lint-unless-helper.js
new file mode 100644
index 0000000..3c32111
--- /dev/null
+++ b/lib/rules/lint-unless-helper.js
@@ -0,0 +1,84 @@
+'use strict';
+
+const Rule = require('./base');
+
+const messages = {
+  followingElseBlock: 'Using an {{else}} block with {{unless}} should be avoided.',
+  asElseUnlessBlock: 'Using an `{{else unless}}` block should be avoided.',
+  withHelper: 'Using {{unless}} in combination with other helpers should be avoided.'
+};
+
+module.exports = class LintUnlessHelper extends Rule {
+  visitor() {
+    return {
+      MustacheStatement(node) {
+        if (node.params[0].path) {
+          this._withHelper(node);
+        }
+      },
+
+      BlockStatement(node) {
+        let nodeInverse = node.inverse;
+        let nodePathOriginal = node.path.original;
+
+        if (nodeInverse) {
+          if (nodePathOriginal === 'unless') {
+            if (nodeInverse.body[0].path) {
+              this._followingElseIfBlock(node);
+            } else {
+              this._followingElseBlock(node);
+            }
+          } else if (nodePathOriginal === 'if') {
+            if (nodeInverse.body[0].path && nodeInverse.body[0].path.original  === 'unless') {
+              this._asElseUnlessBlock(node);
+            }
+          }
+        } else if (nodePathOriginal === 'unless' && node.params[0].path) {
+          this._withHelper(node);
+        }
+      }
+    };
+  }
+
+  _followingElseBlock(block) {
+    let loc = block.program.loc.end;
+    let actual = '{{else}}';
+
+    this._logMessage(messages.followingElseBlock, loc.line, loc.column, actual);
+  }
+
+  _followingElseIfBlock(block) {
+    let inverse = block.inverse;
+    let loc = block.program.loc.end;
+    let parameter = inverse.body[0].params[0].original;
+    let actual = `{{else if ${parameter}}}`;
+
+    this._logMessage(messages.followingElseBlock, loc.line, loc.column, actual);
+  }
+
+  _asElseUnlessBlock(block) {
+    let inverse = block.inverse;
+    let loc = inverse.body[0].loc.start;
+    let actual = '{{else unless ...';
+
+    this._logMessage(messages.asElseUnlessBlock, loc.line, loc.column, actual);
+  }
+
+  _withHelper(node) {
+    let loc = node.params[0].loc.start;
+    let actual = `{{unless (${node.params[0].path.original} ...`;
+
+    this._logMessage(messages.withHelper, loc.line, loc.column, actual);
+  }
+
+  _logMessage(message, line, column, actual) {
+    return this.log({
+      message: message,
+      line: line,
+      column: column,
+      source: actual
+    });
+  }
+};
+
+module.exports.messages = messages;


Test patch:
diff --git a/test/unit/rules/lint-unless-helper-test.js b/test/unit/rules/lint-unless-helper-test.js
new file mode 100644
index 0000000..7dcf7c4
--- /dev/null
+++ b/test/unit/rules/lint-unless-helper-test.js
@@ -0,0 +1,191 @@
+'use strict';
+
+const generateRuleTests = require('../../helpers/rule-test-harness');
+const messages = require('../../../lib/rules/lint-unless-helper').messages;
+
+generateRuleTests({
+  name: 'unless-helper',
+  config: true,
+
+  good: [
+    '{{#unless isRed}}I\'m blue, da ba dee da ba daa{{/unless}}',
+    '<div class="{{unless foo \'no-foo\'}}"></div>',
+    '<div class="{{if foo \'foo\'}}"></div>',
+    [
+      '{{#unless hamburger}}',
+      '  HOT DOG!',
+      '{{/unless}}'
+    ].join('\n')
+  ],
+
+  bad: [
+    {
+      template: '{{unless (if true)  \'Please no\'}}',
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.withHelper,
+        moduleId: 'layout.hbs',
+        source: '{{unless (if ...',
+        line: 1,
+        column: 9
+      }
+    },
+    {
+      template: '{{unless (and isBad isAwful)  \'notBadAndAwful\'}}',
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.withHelper,
+        moduleId: 'layout.hbs',
+        source: '{{unless (and ...',
+        line: 1,
+        column: 9
+      }
+    },
+    {
+      template: [
+        '{{#unless bandwagoner}}',
+        '  Go Niners!',
+        '{{else}}',
+        '  Go Seahawks!',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.followingElseBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else}}',
+        line: 3,
+        column: 0
+      }
+    },
+    {
+      template: [
+        '{{#unless bandwagoner}}',
+        '  Go Niners!',
+        '{{else if goHawks}}',
+        '  Go Seahawks!',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.followingElseBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else if goHawks}}',
+        line: 3,
+        column: 0
+      }
+    },
+    {
+      template: [
+        '{{#unless bandwagoner}}',
+        '  Go Niners!',
+        '{{else if goPats}}',
+        '  Tom Brady is GOAT',
+        '{{else if goHawks}}',
+        '  Go Seahawks!',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.followingElseBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else if goPats}}',
+        line: 3,
+        column: 0
+      }
+    },
+    {
+      template: [
+        '{{#unless bandwagoner}}',
+        '  Go Niners!',
+        '{{else if goBengals}}',
+        '  Ouch, sorry',
+        '{{else}}',
+        '  Go Seahawks!',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.followingElseBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else if goBengals}}',
+        line: 3,
+        column: 0
+      }
+    },
+    {
+      template: [
+        '{{#if dog}}',
+        '  Ruff Ruff!',
+        '{{else unless cat}}',
+        '  not cat',
+        '{{/if}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.asElseUnlessBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else unless ...',
+        line: 3,
+        column: 0
+      }
+    },
+    {
+      template: [
+        '{{#unless (and isFruit isYellow)}}',
+        '  I am a green celery!',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.withHelper,
+        moduleId: 'layout.hbs',
+        source: '{{unless (and ...',
+        line: 1,
+        column: 10
+      }
+    },
+    {
+      template: [
+        '{{#unless (not isBrown isSticky)}}',
+        '  I think I am a brown stick',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.withHelper,
+        moduleId: 'layout.hbs',
+        source: '{{unless (not ...',
+        line: 1,
+        column: 10
+      }
+    },
+    {
+      template: [
+        '{{#unless (not isBrown isSticky)}}',
+        '  I think I am a brown stick',
+        '{{else}}',
+        '  Not a brown stick',
+        '{{/unless}}'
+      ].join('\n'),
+
+      result: {
+        rule: 'unless-helper',
+        message: messages.followingElseBlock,
+        moduleId: 'layout.hbs',
+        source: '{{else}}',
+        line: 3,
+        column: 0
+      }
+    }
+  ]
+});