Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert-arg-order: Rename to no-invalid-debug-function-arguments and detect invalid usages of deprecate and warn too #364

Merged
merged 1 commit into from Dec 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -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` |
Expand All @@ -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 |
Expand Down
29 changes: 0 additions & 29 deletions docs/rules/assert-arg-order.md

This file was deleted.

42 changes: 42 additions & 0 deletions docs/rules/no-invalid-debug-function-arguments.md
@@ -0,0 +1,42 @@
# Catch usages of Ember'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.
2 changes: 1 addition & 1 deletion lib/index.js
Expand Up @@ -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'),
Expand All @@ -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'),
Expand Down
2 changes: 1 addition & 1 deletion lib/recommended-rules.js
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
75 changes: 0 additions & 75 deletions tests/lib/rules/assert-arg-order.js

This file was deleted.

116 changes: 116 additions & 0 deletions 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));
}