Repository: ember-cli/eslint-plugin-ember
Pull request #364 (2018-12-27 17:40:54)

feat: rename 'assert-arg-order' rule to 'no-invalid-debug-function-arguments' to also detect invalid usages of 'deprecate' and 'warn'.
Renames the `assert-arg-order` rule to `no-invalid-debug-function-arguments`. Generalizes it to detect the `assert` argument order mistake along with two other similar functions (`deprecate` and `warn`).

[Requested](https://github.com/ember-cli/eslint-plugin-ember/pull/362#issuecomment-450131918) by @Turbo87.

bmish: @rwjblue good idea, I renamed the rule to `no-invalid-debug-arguments`.

Patch:
diff --git a/README.md b/README.md
index 8696f268..e5884b67 100644
--- a/README.md
+++ b/README.md
@@ -103,7 +103,6 @@ The `--fix` option on the command line automatically fixes problems reported by
 
 |    | Rule ID | Description |
 |:---|:--------|:------------|
-|  | [assert-arg-order](./docs/rules/assert-arg-order.md) | Catch usages of Ember's `assert()` function that have the arguments passed in the wrong order. |
 | :white_check_mark: | [jquery-ember-run](./docs/rules/jquery-ember-run.md) | Prevents usage of jQuery without Ember Run Loop |
 | :white_check_mark: | [no-attrs-in-components](./docs/rules/no-attrs-in-components.md) | Disallow usage of this.attrs in components |
 | :white_check_mark: | [no-attrs-snapshot](./docs/rules/no-attrs-snapshot.md) | Disallow use of attrs snapshot in `didReceiveAttrs` and `didUpdateAttrs` |
@@ -112,6 +111,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 | :white_check_mark: | [no-duplicate-dependent-keys](./docs/rules/no-duplicate-dependent-keys.md) | Disallow repeating dependent keys |
 | :wrench: | [no-ember-super-in-es-classes](./docs/rules/no-ember-super-in-es-classes.md) | Prevents use of `this._super` in ES class methods |
 | :white_check_mark: | [no-ember-testing-in-module-scope](./docs/rules/no-ember-testing-in-module-scope.md) | Prevents use of Ember.testing in module scope |
+|  | [no-invalid-debug-function-arguments](./docs/rules/no-invalid-debug-function-arguments.md) | Catch usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. |
 | :white_check_mark: | [no-side-effects](./docs/rules/no-side-effects.md) | Warns about unexpected side effects in computed properties |
 | :white_check_mark: | [require-super-in-init](./docs/rules/require-super-in-init.md) | Enforces super calls in init hooks |
 | :white_check_mark: | [routes-segments-snake-case](./docs/rules/routes-segments-snake-case.md) | Enforces usage of snake_cased dynamic segments in routes |
diff --git a/docs/rules/assert-arg-order.md b/docs/rules/assert-arg-order.md
deleted file mode 100644
index 93d03d2f..00000000
--- a/docs/rules/assert-arg-order.md
+++ /dev/null
@@ -1,29 +0,0 @@
-# Catch usages of Ember&#39;s `assert()` function that have the arguments passed in the wrong order. (assert-arg-order)
-
-This rule aims to catch a common mistake when using Ember's `assert(String description, Boolean condition)` function. When writing an `assert`, the author may mistakenly pass the arguments in the reverse order, and not notice because the `assert` will pass with a truthy string as its second argument.
-
-## Rule Details
-
-Examples of **incorrect** code for this rule:
-
-```js
-import { assert } from '@ember/debug';
-
-...
-
-assert(label, 'Label must be present.');
-```
-
-Examples of **correct** code for this rule:
-
-```js
-import { assert } from '@ember/debug';
-
-...
-
-assert('Label must be present.', label);
-```
-
-## Further Reading
-
-* See the [documentation](https://www.emberjs.com/api/ember/release/functions/@ember%2Fdebug/assert) for the Ember `assert` function.
diff --git a/docs/rules/no-invalid-debug-function-arguments.md b/docs/rules/no-invalid-debug-function-arguments.md
new file mode 100644
index 00000000..02e1c395
--- /dev/null
+++ b/docs/rules/no-invalid-debug-function-arguments.md
@@ -0,0 +1,42 @@
+# Catch usages of Ember&#39;s `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order. (no-invalid-debug-function-arguments)
+
+This rule aims to catch a common mistake when using Ember's debug functions:
+* `assert(String description, Boolean condition)`
+* `warn(String description, Boolean condition, Object options)`
+* `deprecate(String description, Boolean condition, Object options)`
+
+When calling one of these functions, the author may mistakenly pass the `description` and `condition` arguments in the reverse order, and not notice because the function will be silent with a truthy string as the `condition`.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+import { assert, warn } from '@ember/debug';
+import { deprecate } from '@ember/application/deprecations';
+
+...
+
+assert(label, 'Label must be present.');
+warn(label, 'Label must be present.', { id: 'missing-label' });
+deprecate(title, 'Title is no longer supported.', { id: 'unwanted-title', until: 'some-version' });
+```
+
+Examples of **correct** code for this rule:
+
+```js
+import { assert, warn } from '@ember/debug';
+import { deprecate } from '@ember/application/deprecations';
+
+...
+
+assert('Label must be present.', label);
+warn('Label must be present.', label, { id: 'missing-label' });
+deprecate('Title is no longer supported.', title, { id: 'unwanted-title', until: 'some-version' });
+```
+
+## Further Reading
+
+* See the [documentation](https://www.emberjs.com/api/ember/release/functions/@ember%2Fdebug/assert) for the Ember `assert` function.
+* See the [documentation](https://www.emberjs.com/api/ember/release/functions/@ember%2Fdebug/warn) for the Ember `warn` function.
+* See the [documentation](https://emberjs.com/api/api/ember/release/functions/@ember%2Fapplication%2Fdeprecations/deprecate) for the Ember `deprecate` function.
diff --git a/lib/index.js b/lib/index.js
index 39962ed6..7fd09f26 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -4,7 +4,6 @@
 module.exports = {
   rules: {
     'alias-model-in-controller': require('./rules/alias-model-in-controller'),
-    'assert-arg-order': require('./rules/assert-arg-order'),
     'avoid-leaking-state-in-components': require('./rules/avoid-leaking-state-in-components'),
     'avoid-leaking-state-in-ember-objects': require('./rules/avoid-leaking-state-in-ember-objects'),
     'avoid-using-needs-in-controllers': require('./rules/avoid-using-needs-in-controllers'),
@@ -23,6 +22,7 @@ module.exports = {
     'no-empty-attrs': require('./rules/no-empty-attrs'),
     'no-function-prototype-extensions': require('./rules/no-function-prototype-extensions'),
     'no-global-jquery': require('./rules/no-global-jquery'),
+    'no-invalid-debug-function-arguments': require('./rules/no-invalid-debug-function-arguments'),
     'no-jquery': require('./rules/no-jquery'),
     'no-new-mixins': require('./rules/no-new-mixins'),
     'no-observers': require('./rules/no-observers'),
diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js
index bf184c58..ec69ec3f 100644
--- a/lib/recommended-rules.js
+++ b/lib/recommended-rules.js
@@ -6,7 +6,6 @@
  */
 module.exports = {
   "ember/alias-model-in-controller": "off",
-  "ember/assert-arg-order": "off",
   "ember/avoid-leaking-state-in-components": "off",
   "ember/avoid-leaking-state-in-ember-objects": "error",
   "ember/avoid-using-needs-in-controllers": "error",
@@ -25,6 +24,7 @@ module.exports = {
   "ember/no-empty-attrs": "off",
   "ember/no-function-prototype-extensions": "error",
   "ember/no-global-jquery": "error",
+  "ember/no-invalid-debug-function-arguments": "off",
   "ember/no-jquery": "off",
   "ember/no-new-mixins": "off",
   "ember/no-observers": "error",
diff --git a/lib/rules/assert-arg-order.js b/lib/rules/no-invalid-debug-function-arguments.js
similarity index 50%
rename from lib/rules/assert-arg-order.js
rename to lib/rules/no-invalid-debug-function-arguments.js
index 6ce5f020..e5f60659 100644
--- a/lib/rules/assert-arg-order.js
+++ b/lib/rules/no-invalid-debug-function-arguments.js
@@ -7,23 +7,26 @@ const emberUtils = require('../utils/ember');
 // Rule Definition
 //------------------------------------------------------------------------------
 
-const ERROR_MESSAGE = "Usage of Ember's assert(String description, Boolean condition) function has its arguments passed in the wrong order.";
+const ERROR_MESSAGE = "Usage of Ember's `assert` / `warn` / `deprecate` function has its arguments passed in the wrong order. `String description` should come before `Boolean condition`.";
+const DEBUG_FUNCTIONS = ['assert', 'deprecate', 'warn'];
 
 module.exports = {
   meta: {
     docs: {
-      description: "Catch usages of Ember's `assert()` function that have the arguments passed in the wrong order.",
+      description: "Catch usages of Ember's `assert()` / `warn()` / `deprecate()` functions that have the arguments passed in the wrong order.",
       category: 'Possible Errors',
       recommended: false
     },
-    fixable: null,
-    ERROR_MESSAGE
+    fixable: null
   },
 
+  ERROR_MESSAGE,
+  DEBUG_FUNCTIONS,
+
   create(context) {
     return {
       CallExpression(node) {
-        if (isAssertFunctionWithReversedArgs(node)) {
+        if (isDebugFunctionWithReversedArgs(node)) {
           context.report({
             node,
             message: ERROR_MESSAGE
@@ -34,18 +37,18 @@ module.exports = {
   }
 };
 
-function isAssertFunctionWithReversedArgs(node) {
+function isDebugFunctionWithReversedArgs(node) {
   return (
-    isAssertFunction(node) &&
-    node.arguments.length === 2 &&
+    isDebugFunction(node) &&
+    node.arguments.length >= 2 &&
     !isString(node.arguments[0]) &&
     isString(node.arguments[1])
   );
 }
 
-function isAssertFunction(node) {
-  // Detect `Ember.assert()` or `assert()`.
-  return emberUtils.isModule(node, 'assert');
+function isDebugFunction(node) {
+  // Example: Detects `Ember.assert()` or `assert()`.
+  return DEBUG_FUNCTIONS.some(debugFunction => emberUtils.isModule(node, debugFunction));
 }
 
 function isString(node) {


Test patch:
diff --git a/tests/lib/rules/assert-arg-order.js b/tests/lib/rules/assert-arg-order.js
deleted file mode 100644
index f46c89c4..00000000
--- a/tests/lib/rules/assert-arg-order.js
+++ /dev/null
@@ -1,75 +0,0 @@
-//------------------------------------------------------------------------------
-// Requirements
-//------------------------------------------------------------------------------
-
-const rule = require('../../../lib/rules/assert-arg-order');
-const RuleTester = require('eslint').RuleTester;
-
-const { ERROR_MESSAGE } = rule;
-
-//------------------------------------------------------------------------------
-// Tests
-//------------------------------------------------------------------------------
-
-const ruleTester = new RuleTester({
-  parserOptions: {
-    ecmaVersion: 2015,
-    sourceType: 'module'
-  }
-});
-ruleTester.run('assert-arg-order', rule, {
-  valid: [
-    {
-      code: "myFunction(true, 'My string.');"
-    },
-    {
-      code: "Ember.myFunction(true, 'My string.');"
-    },
-    {
-      code: "OtherClass.assert(true, 'My string.');"
-    },
-    {
-      code: "assert('My description.');"
-    },
-    {
-      code: "Ember.assert('My description.');"
-    },
-    {
-      code: "assert('My description.', true);"
-    },
-    {
-      code: "Ember.assert('My description.', true);"
-    },
-    {
-      code: "const CONDITION = true; assert('My description.', CONDITION);"
-    },
-    {
-      code: "const DESCRIPTION = 'description'; assert(DESCRIPTION, true);"
-    },
-    {
-      code: "const DESCRIPTION = 'description'; const CONDITION = true; assert(DESCRIPTION, CONDITION);"
-    }
-  ],
-  invalid: [
-    {
-      code: "assert(true, 'My description.');",
-      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
-    },
-    {
-      code: "Ember.assert(true, 'My description.');",
-      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
-    },
-    {
-      code: 'assert(true, `My ${123} description.`);', // eslint-disable-line no-template-curly-in-string
-      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
-    },
-    {
-      code: "const CONDITION = true; assert(CONDITION, 'My description.');",
-      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
-    },
-    {
-      code: 'const CONDITION = true; assert(CONDITION, `My ${123} description.`);', // eslint-disable-line no-template-curly-in-string
-      errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
-    }
-  ]
-});
diff --git a/tests/lib/rules/no-invalid-debug-function-arguments.js b/tests/lib/rules/no-invalid-debug-function-arguments.js
new file mode 100644
index 00000000..45b5f4a7
--- /dev/null
+++ b/tests/lib/rules/no-invalid-debug-function-arguments.js
@@ -0,0 +1,116 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-invalid-debug-function-arguments');
+const RuleTester = require('eslint').RuleTester;
+
+const { DEBUG_FUNCTIONS, ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const VALID_USAGES_BASIC = [
+  {
+    code: "myFunction(true, 'My string.');"
+  },
+  {
+    code: "Ember.myFunction(true, 'My string.');"
+  }
+];
+
+const VALID_USAGES_FOR_EACH_DEBUG_FUNCTION = flatten(DEBUG_FUNCTIONS.map(debugFunction => [
+  {
+    code: `OtherClass.${debugFunction}(true, 'My string.');`
+  },
+  {
+    code: `${debugFunction}();`
+  },
+  {
+    code: `Ember.${debugFunction}();`
+  },
+  {
+    code: `${debugFunction}('My description.');`
+  },
+  {
+    code: `Ember.${debugFunction}('My description.');`
+  },
+  {
+    code: `${debugFunction}('My description.', true);`
+  },
+  {
+    code: `Ember.${debugFunction}('My description.', true);`
+  },
+  {
+    code: `${debugFunction}('My description.', true, { id: 'some-id' });`
+  },
+  {
+    code: `Ember.${debugFunction}('My description.', true, { id: 'some-id' });`
+  },
+  {
+    code: `const CONDITION = true; ${debugFunction}('My description.', CONDITION);`
+  },
+  {
+    code: `const DESCRIPTION = 'description'; ${debugFunction}(DESCRIPTION, true);`
+  },
+  {
+    code: `const DESCRIPTION = 'description'; const CONDITION = true; ${debugFunction}(DESCRIPTION, CONDITION);`
+  }
+]));
+
+const VALID_USAGES = [
+  ...VALID_USAGES_BASIC,
+  ...VALID_USAGES_FOR_EACH_DEBUG_FUNCTION
+];
+
+const INVALID_USAGES = flatten(DEBUG_FUNCTIONS.map(debugFunction => [
+  {
+    code: `${debugFunction}(true, 'My description.');`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `Ember.${debugFunction}(true, 'My description.');`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `${debugFunction}(true, 'My description.', { id: 'some-id' });`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `Ember.${debugFunction}(true, 'My description.', { id: 'some-id' });`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `${debugFunction}(true, \`My \${123} description.\`);`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `const CONDITION = true; ${debugFunction}(CONDITION, 'My description.');`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  },
+  {
+    code: `const CONDITION = true; ${debugFunction}(CONDITION, \`My \${123} description.\`);`,
+    errors: [{ message: ERROR_MESSAGE, type: 'CallExpression' }]
+  }
+]));
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2015,
+    sourceType: 'module'
+  }
+});
+
+ruleTester.run('no-invalid-debug-function-arguments', rule, {
+  valid: VALID_USAGES,
+  invalid: INVALID_USAGES
+});
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+function flatten(arr) {
+  return arr.reduce((acc, val) => acc.concat(val));
+}