Skip to content

Commit

Permalink
Add prefer-node-remove rule (#222)
Browse files Browse the repository at this point in the history
Fixes #213
  • Loading branch information
medusalix authored and sindresorhus committed Feb 4, 2019
1 parent 2026ab2 commit fda5517
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 2 deletions.
26 changes: 26 additions & 0 deletions docs/rules/prefer-node-remove.md
@@ -0,0 +1,26 @@
# Prefer `remove` over `parentNode.removeChild` and `parentElement.removeChild`

Enforces the use of, for example, `child.remove();` over `child.parentNode.removeChild(child);` and `child.parentElement.removeChild(child);` for DOM nodes. The DOM function [`.remove()`](https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove) is preferred over the indirect removal of an object with [`.removeChild()`](https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild).

This rule is fixable.


## Fail

```js
foo.parentNode.removeChild(foo);
foo.parentElement.removeChild(foo);
this.parentNode.removeChild(this);
this.parentElement.removeChild(this);
foo.parentNode.removeChild(bar);
foo.parentElement.removeChild(bar);
this.parentNode.removeChild(foo);
this.parentElement.removeChild(foo);
```

## Pass

```js
foo.remove();
this.remove();
```
3 changes: 2 additions & 1 deletion index.js
Expand Up @@ -54,7 +54,8 @@ module.exports = {
'unicorn/no-unreadable-array-destructuring': 'error',
'unicorn/no-unused-properties': 'off',
'unicorn/prefer-node-append': 'error',
'unicorn/prefer-query-selector': 'error'
'unicorn/prefer-query-selector': 'error',
'unicorn/prefer-node-remove': 'error'
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion readme.md
Expand Up @@ -71,7 +71,8 @@ Configure it in `package.json`.
"unicorn/no-unreadable-array-destructuring": "error",
"unicorn/no-unused-properties": "off",
"unicorn/prefer-node-append": "error",
"unicorn/prefer-query-selector": "error"
"unicorn/prefer-query-selector": "error",
"unicorn/prefer-node-remove': 'error"
}
}
}
Expand Down Expand Up @@ -108,6 +109,7 @@ Configure it in `package.json`.
- [no-unused-properties](docs/rules/no-unused-properties.md) - Disallow unused object properties.
- [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `append` over `appendChild`. *(fixable)*
- [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `querySelector` over `getElementById`, `querySelectorAll` over `getElementsByClassName` and `getElementsByTagName`. *(partly fixable)*
- [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `remove` over `parentNode.removeChild` and `parentElement.removeChild`. *(fixable)*


## Recommended config
Expand Down
87 changes: 87 additions & 0 deletions rules/prefer-node-remove.js
@@ -0,0 +1,87 @@
'use strict';
const getDocsUrl = require('./utils/get-docs-url');

const getMethodName = callee => {
const {property} = callee;

if (property.type === 'Identifier') {
return property.name;
}

return null;
};

const getCallerName = callee => {
const {object} = callee;

if (object.type === 'Identifier') {
return object.name;
}

if (object.type === 'MemberExpression') {
const {property} = object;

if (property.type === 'Identifier') {
return property.name;
}
}

return null;
};

const getArgumentName = args => {
const [identifier] = args;

if (identifier.type === 'ThisExpression') {
return 'this';
}

if (identifier.type === 'Identifier') {
return identifier.name;
}

return null;
};

const create = context => {
return {
CallExpression(node) {
const {callee} = node;

if (node.arguments.length === 0 ||
callee.type !== 'MemberExpression' ||
callee.computed
) {
return;
}

const methodName = getMethodName(callee);
const callerName = getCallerName(callee);

if (methodName === 'removeChild' && (
callerName === 'parentNode' ||
callerName === 'parentElement'
)) {
const argumentName = getArgumentName(node.arguments);

if (argumentName) {
context.report({
node,
message: `Prefer \`remove\` over \`${callerName}.removeChild\``,
fix: fixer => fixer.replaceText(node, `${argumentName}.remove()`)
});
}
}
}
};
};

module.exports = {
create,
meta: {
docs: {
url: getDocsUrl(__filename)
},
fixable: 'code'
}
};
81 changes: 81 additions & 0 deletions test/prefer-node-remove.js
@@ -0,0 +1,81 @@
import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/prefer-node-remove';

const ruleTester = avaRuleTester(test, {
env: {
es6: true
}
});

const invalidTestCase = (code, output, parentType) => {
return {
code,
output,
errors: [{message: `Prefer \`remove\` over \`${parentType}.removeChild\``}]
};
};

ruleTester.run('prefer-node-remove', rule, {
valid: [
'foo.remove()',
'this.remove()',
'remove()',
'foo.parentNode.removeChild(\'bar\')',
'foo.parentNode[\'bar\'](foo)',
'foo.parentNode[removeChild](foo)',
'foo.parentNode.removeChild()'
],
invalid: [
invalidTestCase(
'foo.parentNode.removeChild(foo)',
'foo.remove()',
'parentNode'
),
invalidTestCase(
'this.parentNode.removeChild(this)',
'this.remove()',
'parentNode'
),
invalidTestCase(
'parentNode.removeChild(this)',
'this.remove()',
'parentNode'
),
invalidTestCase(
'foo.parentNode.removeChild(bar)',
'bar.remove()',
'parentNode'
),
invalidTestCase(
'this.parentNode.removeChild(foo)',
'foo.remove()',
'parentNode'
),
invalidTestCase(
'foo.parentElement.removeChild(foo)',
'foo.remove()',
'parentElement'
),
invalidTestCase(
'this.parentElement.removeChild(this)',
'this.remove()',
'parentElement'
),
invalidTestCase(
'parentElement.removeChild(this)',
'this.remove()',
'parentElement'
),
invalidTestCase(
'foo.parentElement.removeChild(bar)',
'bar.remove()',
'parentElement'
),
invalidTestCase(
'this.parentElement.removeChild(foo)',
'foo.remove()',
'parentElement'
)
]
});

0 comments on commit fda5517

Please sign in to comment.