Repository: ember-template-lint/ember-template-lint
Pull request #609 (2019-01-13 07:08:20)

feat: add rule 'no-negated-condition'.
This new rule is inspired by the [no-negated-condition](https://eslint.org/docs/rules/no-negated-condition) rule from eslint. It is compatible with the existing [simple-unless](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/simple-unless.md) rule.


Patch:
diff --git a/docs/rule/no-negated-condition.md b/docs/rule/no-negated-condition.md
new file mode 100644
index 00000000..4e629f02
--- /dev/null
+++ b/docs/rule/no-negated-condition.md
@@ -0,0 +1,92 @@
+## no-negated-condition
+
+It can be hard to reason about negated conditions:
+
+* `if (not condition)`
+* `unless (not condition)`
+
+Negated conditions can often be avoided or simplified by:
+
+* Flipping `if (not condition) ... else ...` to `if .... else ...`
+* Replacing `if (not condition)` with `unless`
+* Replacing `unless (not condition)` with `if`
+
+This rule **forbids** the following:
+
+```hbs
+{{#if (not condition)}}
+  ...
+{{/if}}
+```
+
+```hbs
+{{#if (not condition)}}
+  ...
+{{else}}
+  ...
+{{/if}}
+```
+
+```hbs
+{{#unless (not condition)}}
+  ...
+{{/unless}}
+```
+
+```hbs
+{{#unless (not condition)}}
+  ...
+{{else}}
+  ...
+{{/unless}}
+```
+
+And similar examples with non-block forms like:
+
+```hbs
+<img class={{if (not condition) "some-class" "other-class"}}>
+```
+
+```hbs
+{{input class=(if (not condition) "some-class" "other-class")}}
+```
+
+This rule **allows** the following:
+
+```hbs
+{{#if condition}}
+  ...
+{{/if}}
+```
+
+```hbs
+{{#if condition}}
+  ...
+{{else}}
+  ...
+{{/if}}
+```
+
+```hbs
+{{#unless condition}}
+  ...
+{{/unless}}
+```
+
+And similar examples with non-block forms like:
+
+```hbs
+<img class={{if condition "some-class" "other-class"}}>
+```
+
+```hbs
+{{input class=(if condition "some-class" "other-class")}}
+```
+
+### Related Rules
+
+* [simple-unless](simple-unless.md)
+
+### References
+
+* [no-negated-condition](https://eslint.org/docs/rules/no-negated-condition) from eslint
diff --git a/docs/rule/simple-unless.md b/docs/rule/simple-unless.md
index 8f4dba9e..c74c7881 100644
--- a/docs/rule/simple-unless.md
+++ b/docs/rule/simple-unless.md
@@ -40,3 +40,7 @@ The following values are valid configuration:
   * object --
     * `whitelist` -- array - `['or']` for specific helpers / `[]` for wildcard
     * `maxHelpers` -- number
+
+### Related Rules
+
+  * [no-negated-condition](no-negated-condition.md)
diff --git a/docs/rules.md b/docs/rules.md
index 7affdde1..0234df4c 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -22,6 +22,7 @@
 * [no-input-tagname](rule/no-input-tagname.md)
 * [no-invalid-interactive](rule/no-invalid-interactive.md)
 * [no-log](rule/no-log.md)
+* [no-negated-condition](rule/no-negated-condition.md)
 * [no-nested-interactive](rule/no-nested-interactive.md)
 * [no-outlet-outside-routes](rule/no-outlet-outside-routes.md)
 * [no-partial](rule/no-partial.md)
diff --git a/lib/helpers/ast-node-info.js b/lib/helpers/ast-node-info.js
index 8394d872..2c1aa212 100644
--- a/lib/helpers/ast-node-info.js
+++ b/lib/helpers/ast-node-info.js
@@ -28,6 +28,10 @@ function isPathExpression(node) {
   return node.type === 'PathExpression';
 }
 
+function isSubExpression(node) {
+  return node.type === 'SubExpression';
+}
+
 function isElementNode(node) {
   return node && node.type === 'ElementNode';
 }
@@ -98,6 +102,7 @@ module.exports = {
   isConcatStatement: isConcatStatement,
   isMustacheCommentStatement: isMustacheCommentStatement,
   isPathExpression: isPathExpression,
+  isSubExpression: isSubExpression,
   isComponentNode: isComponentNode,
   isConfigurationHtmlComment: isConfigurationHtmlComment,
   isElementNode: isElementNode,
diff --git a/lib/rules/index.js b/lib/rules/index.js
index eae9ed9c..296afac7 100644
--- a/lib/rules/index.js
+++ b/lib/rules/index.js
@@ -26,6 +26,7 @@ module.exports = {
   'no-input-tagname': require('./lint-no-input-tagname'),
   'no-invalid-interactive': require('./lint-no-invalid-interactive'),
   'no-log': require('./lint-no-log'),
+  'no-negated-condition': require('./lint-no-negated-condition'),
   'no-nested-interactive': require('./lint-no-nested-interactive'),
   'no-outlet-outside-routes': require('./lint-no-outlet-outside-routes'),
   'no-partial': require('./lint-no-partial'),
diff --git a/lib/rules/lint-no-negated-condition.js b/lib/rules/lint-no-negated-condition.js
new file mode 100644
index 00000000..e9fcf7c3
--- /dev/null
+++ b/lib/rules/lint-no-negated-condition.js
@@ -0,0 +1,93 @@
+'use strict';
+
+const Rule = require('./base');
+const AstNodeInfo = require('../helpers/ast-node-info');
+
+const ERROR_MESSAGE_FLIP_IF = 'Flip `if` statement to avoid `if !condition`.';
+const ERROR_MESSAGE_USE_IF = 'Use `if` instead of `unless !condition`.';
+const ERROR_MESSAGE_USE_UNLESS = 'Use `unless` instead of `if !condition`.';
+
+module.exports = class NoNegatedCondition extends Rule {
+  visitor() {
+    function checkNode(node) {
+      const isIf = node.path && node.path.original === 'if';
+      const isUnless = node.path && node.path.original === 'unless';
+      if (!isIf && !isUnless) {
+        // Not a conditional statement.
+        return;
+      }
+
+      if (AstNodeInfo.isBlockStatement(node)) {
+        if (node.inverse && node.inverse.body.length > 0) {
+          // Conditional block statement has an `else` / `else if`
+
+          const firstNode = findFirstNonTextNonCommentNode(node.inverse.body);
+          if (firstNode && AstNodeInfo.isBlockStatement(firstNode)) {
+            // If there's BlockStatement directly inside the body of the `else`,
+            // we need to mark it so we can ignore it later,
+            // to avoid potentially suggesting an `unless` after an `else`.
+            firstNode.isAtBeginningOfElseBody = true;
+
+            if (isIf && firstNode.path.original === 'if') {
+              // Ignore `if ... else if ...` statements as there's nothing wrong with them.
+              return;
+            }
+          }
+        }
+
+        if (isIf && node.isAtBeginningOfElseBody) {
+          // Ignore `if` statements directly following an `else` due to complexity.
+          return;
+        }
+      }
+
+      if (
+        node.params.length > 0 &&
+        AstNodeInfo.isSubExpression(node.params[0]) &&
+        AstNodeInfo.isPathExpression(node.params[0].path) &&
+        node.params[0].path.original === 'not'
+      ) {
+        // Determine what error message to show:
+        const isIfElseBlockStatement = AstNodeInfo.isBlockStatement(node) && node.inverse;
+        const isIfElseNonBlockStatement =
+          !AstNodeInfo.isBlockStatement(node) && node.params.length === 3;
+        const shouldFlipCondition = isIfElseBlockStatement || isIfElseNonBlockStatement;
+        const message = isUnless
+          ? ERROR_MESSAGE_USE_IF
+          : shouldFlipCondition
+          ? ERROR_MESSAGE_FLIP_IF
+          : ERROR_MESSAGE_USE_UNLESS;
+
+        this.log({
+          message,
+          line: node.loc && node.loc.start.line,
+          column: node.loc && node.loc.start.column,
+          source: this.sourceForNode(node),
+        });
+      }
+    }
+
+    return {
+      BlockStatement: checkNode,
+      MustacheStatement: checkNode,
+      SubExpression: checkNode,
+    };
+  }
+};
+
+function findFirstNonTextNonCommentNode(nodes) {
+  for (let i = 0; i < nodes.length; i++) {
+    const currentNode = nodes[i];
+    if (
+      !AstNodeInfo.isTextNode(currentNode) &&
+      !AstNodeInfo.isMustacheCommentStatement(currentNode)
+    ) {
+      return currentNode;
+    }
+  }
+  return null;
+}
+
+module.exports.ERROR_MESSAGE_FLIP_IF = ERROR_MESSAGE_FLIP_IF;
+module.exports.ERROR_MESSAGE_USE_IF = ERROR_MESSAGE_USE_IF;
+module.exports.ERROR_MESSAGE_USE_UNLESS = ERROR_MESSAGE_USE_UNLESS;


Test patch:
diff --git a/test/unit/rules/lint-no-negated-condition.js b/test/unit/rules/lint-no-negated-condition.js
new file mode 100644
index 00000000..da27179e
--- /dev/null
+++ b/test/unit/rules/lint-no-negated-condition.js
@@ -0,0 +1,293 @@
+'use strict';
+
+const generateRuleTests = require('../../helpers/rule-test-harness');
+const rule = require('../../../lib/rules/lint-no-negated-condition');
+
+const { ERROR_MESSAGE_FLIP_IF, ERROR_MESSAGE_USE_IF, ERROR_MESSAGE_USE_UNLESS } = rule;
+
+generateRuleTests({
+  name: 'no-negated-condition',
+
+  config: true,
+
+  good: [
+    // ******************************************
+    // BlockStatement
+    // ******************************************
+
+    // if ...
+    '{{#if condition}}<img>{{/if}}',
+    '{{#if (or c1 c2)}}{{/if}}',
+
+    // if ... else ...
+    '{{#if condition}}<img>{{else}}<img>{{/if}}',
+    '{{#if (or c1 c2)}}<img>{{else}}<img>{{/if}}',
+
+    // if ... else if ...
+    '{{#if condition}}<img>{{else if condition}}<img>{{/if}}',
+    '{{#if (not condition)}}<img>{{else if (not condition2)}}<img>{{/if}}', // we ignore `if ... else if ...`
+
+    // if ... else if ... else ...
+    '{{#if condition}}<img>{{else if condition}}<img>{{else}}<img>{{/if}}',
+    '{{#if (not condition)}}<img>{{else if (not condition2)}}<img>{{else}}<img>{{/if}}', // we ignore `if ... else if ...`
+
+    // unless ...
+    '{{#unless condition}}<img>{{/unless}}',
+    '{{#unless (or c1 c2)}}<img>{{/unless}}',
+
+    // unless ... else ...
+    '{{#unless condition}}<img>{{else}}<img>{{/unless}}',
+    '{{#unless (or c1 c2)}}<img>{{else}}<img>{{/unless}}',
+
+    // unless ... else if ...
+    '{{#unless condition}}<img>{{else if condition}}<img>{{/unless}}',
+    '{{#unless (or c1 c2)}}<img>{{else if (or c1 c2)}}<img>{{/unless}}',
+
+    // unless ... else if ... else ...
+    '{{#unless condition}}<img>{{else if condition}}<img>{{else}}<img>{{/unless}}',
+    '{{#unless (or c1 c2)}}<img>{{else if (or c1 c2)}}<img>{{else}}<img>{{/unless}}',
+
+    // This is valid since we don't want to suggest `unless` directly after an `else`:
+    `{{#if condition}}
+     {{else}}
+       {{! some comment }}
+       {{#if (not condition)}}<img>{{/if}}
+     {{/if}}`,
+
+    // This is valid since we can't yet handle `if` statements directly after an `else`:
+    `{{#if (not condition)}}
+     {{else}}
+       {{#if (not condition)}}<img>{{else}}<img>{{/if}}
+     {{/if}}`,
+
+    // ******************************************
+    // MustacheStatement
+    // ******************************************
+
+    // if ...
+    '<img class={{if condition "some-class"}}>',
+    '<img class={{if (or c1 c2) "some-class"}}>',
+
+    // if ... else ...
+    '<img class={{if condition "some-class" "other-class"}}>',
+    '<img class={{if (or c1 c2) "some-class" "other-class"}}>',
+
+    // unless ...
+    '<img class={{unless condition "some-class"}}>',
+    '<img class={{unless (or c1 c2) "some-class"}}>',
+
+    // unless ... else ...
+    '<img class={{unless condition "some-class" "other-class"}}>',
+    '<img class={{unless (or c1 c2) "some-class" "other-class"}}>',
+
+    // ******************************************
+    // SubExpression
+    // ******************************************
+
+    // if ...
+    '{{input class=(if condition "some-class")}}',
+    '{{input class=(if (or c1 c2) "some-class")}}',
+
+    // if ... else ...
+    '{{input class=(if condition "some-class" "other-class")}}',
+    '{{input class=(if (or c1 c2) "some-class" "other-class")}}',
+  ],
+
+  bad: [
+    // ******************************************
+    // BlockStatement
+    // ******************************************
+
+    // if ...
+    {
+      template: '{{#if (not condition)}}<img>{{/if}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_UNLESS,
+        moduleId: 'layout.hbs',
+        source: '{{#if (not condition)}}<img>{{/if}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // if ... else ...
+    {
+      template: '{{#if (not condition)}}<img>{{else}}<img>{{/if}}',
+
+      result: {
+        message: ERROR_MESSAGE_FLIP_IF,
+        moduleId: 'layout.hbs',
+        source: '{{#if (not condition)}}<img>{{else}}<img>{{/if}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // unless ...
+    {
+      template: '{{#unless (not condition)}}<img>{{/unless}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '{{#unless (not condition)}}<img>{{/unless}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // unless ... else ...
+    {
+      template: '{{#unless (not condition)}}<img>{{else}}<img>{{/unless}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '{{#unless (not condition)}}<img>{{else}}<img>{{/unless}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // unless ... else if ...
+    {
+      template: '{{#unless (not condition)}}<img>{{else if (not condition)}}<img>{{/unless}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '{{#unless (not condition)}}<img>{{else if (not condition)}}<img>{{/unless}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // unless ... else if ... else ...
+    {
+      template:
+        '{{#unless (not condition)}}<img>{{else if (not condition)}}<img>{{else}}<img>{{/unless}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source:
+          '{{#unless (not condition)}}<img>{{else if (not condition)}}<img>{{else}}<img>{{/unless}}',
+        line: 1,
+        column: 0,
+      },
+    },
+
+    // ******************************************
+    // MustacheStatement
+    // ******************************************
+
+    // if ...
+    {
+      template: '<img class={{if (not condition) "some-class"}}>',
+
+      result: {
+        message: ERROR_MESSAGE_USE_UNLESS,
+        moduleId: 'layout.hbs',
+        source: '{{if (not condition) "some-class"}}',
+        line: 1,
+        column: 11,
+      },
+    },
+
+    // if ... else ...
+    {
+      template: '<img class={{if (not condition) "some-class" "other-class"}}>',
+
+      result: {
+        message: ERROR_MESSAGE_FLIP_IF,
+        moduleId: 'layout.hbs',
+        source: '{{if (not condition) "some-class" "other-class"}}',
+        line: 1,
+        column: 11,
+      },
+    },
+
+    // unless ...
+    {
+      template: '<img class={{unless (not condition) "some-class"}}>',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '{{unless (not condition) "some-class"}}',
+        line: 1,
+        column: 11,
+      },
+    },
+
+    // unless ... else ...
+    {
+      template: '<img class={{unless (not condition) "some-class" "other-class"}}>',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '{{unless (not condition) "some-class" "other-class"}}',
+        line: 1,
+        column: 11,
+      },
+    },
+
+    // ******************************************
+    // SubExpression
+    // ******************************************
+
+    // if ...
+    {
+      template: '{{input class=(if (not condition) "some-class")}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_UNLESS,
+        moduleId: 'layout.hbs',
+        source: '(if (not condition) "some-class")',
+        line: 1,
+        column: 14,
+      },
+    },
+
+    // if ... else ...
+    {
+      template: '{{input class=(if (not condition) "some-class" "other-class")}}',
+
+      result: {
+        message: ERROR_MESSAGE_FLIP_IF,
+        moduleId: 'layout.hbs',
+        source: '(if (not condition) "some-class" "other-class")',
+        line: 1,
+        column: 14,
+      },
+    },
+
+    // unless ...
+    {
+      template: '{{input class=(unless (not condition) "some-class")}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '(unless (not condition) "some-class")',
+        line: 1,
+        column: 14,
+      },
+    },
+
+    // unless ... else ...
+    {
+      template: '{{input class=(unless (not condition) "some-class" "other-class")}}',
+
+      result: {
+        message: ERROR_MESSAGE_USE_IF,
+        moduleId: 'layout.hbs',
+        source: '(unless (not condition) "some-class" "other-class")',
+        line: 1,
+        column: 14,
+      },
+    },
+  ],
+});