diff --git a/docs/rules/prefer-node-remove.md b/docs/rules/prefer-node-remove.md index b3c283ce34..91532a95da 100644 --- a/docs/rules/prefer-node-remove.md +++ b/docs/rules/prefer-node-remove.md @@ -1,6 +1,6 @@ -# Prefer `node.remove()` over `parentNode.removeChild(node)` and `parentElement.removeChild(node)` +# Prefer `childNode.remove()` over `parentNode.removeChild(childNode)` -Enforces the use of, for example, `child.remove();` over `child.parentNode.removeChild(child);` and `child.parentElement.removeChild(child);` for DOM nodes. The DOM function [`Node#remove()`](https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove) is preferred over the indirect removal of an object with [`Node#removeChild()`](https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild). +Enforces the use of, for example, `child.remove();` over `child.parentNode.removeChild(child);`. The DOM function [`Node#remove()`](https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove) is preferred over the indirect removal of an object with [`Node#removeChild()`](https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild). This rule is fixable. @@ -8,14 +8,8 @@ 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); +parentNode.removeChild(foo); +parentNode.removeChild(this); ``` diff --git a/readme.md b/readme.md index e22b75b62c..7cc0a5a0ab 100644 --- a/readme.md +++ b/readme.md @@ -121,7 +121,7 @@ Configure it in `package.json`. - [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) - Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. *(fixable)* - [prefer-negative-index](docs/rules/prefer-negative-index.md) - Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()` and `Array#splice()`. *(fixable)* - [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `Node#append()` over `Node#appendChild()`. *(fixable)* -- [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `node.remove()` over `parentNode.removeChild(node)` and `parentElement.removeChild(node)`. *(fixable)* +- [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`. *(fixable)* - [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)* - [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)* - [prefer-replace-all](docs/rules/prefer-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)* diff --git a/rules/prefer-node-remove.js b/rules/prefer-node-remove.js index 8fbc382c0e..0ca5acc04d 100644 --- a/rules/prefer-node-remove.js +++ b/rules/prefer-node-remove.js @@ -8,24 +8,9 @@ const selector = methodSelector({ length: 1 }); -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 message = 'Prefer Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`.'; +// TODO: support more types of childNode const getArgumentName = arguments_ => { const [identifier] = arguments_; @@ -33,34 +18,26 @@ const getArgumentName = arguments_ => { return 'this'; } - if (identifier.type === 'Identifier') { + if (identifier.type === 'Identifier' && identifier.name !== 'undefined') { return identifier.name; } - - return null; }; const create = context => { return { [selector](node) { - const {callee} = node; - - const callerName = getCallerName(callee); - - if ( - (callerName === 'parentNode' || callerName === 'parentElement') - ) { - const argumentName = getArgumentName(node.arguments); - - if (argumentName) { - const fix = isValueNotUsable(node) ? fixer => fixer.replaceText(node, `${argumentName}.remove()`) : undefined; - - context.report({ - node, - message: `Prefer \`${argumentName}.remove()\` over \`${callerName}.removeChild(${argumentName})\`.`, - fix - }); - } + const argumentName = getArgumentName(node.arguments); + + if (argumentName) { + const fix = isValueNotUsable(node) ? + fixer => fixer.replaceText(node, `${argumentName}.remove()`) : + undefined; + + context.report({ + node, + message, + fix + }); } } }; diff --git a/test/prefer-node-remove.js b/test/prefer-node-remove.js index a8235da548..07160240f8 100644 --- a/test/prefer-node-remove.js +++ b/test/prefer-node-remove.js @@ -5,192 +5,88 @@ import rule from '../rules/prefer-node-remove'; const ruleTester = avaRuleTester(test, { env: { es6: true + }, + parserOptions: { + ecmaVersion: 2020 } }); -const invalidTestCase = (code, output, parentName, argumentName) => { +const message = 'Prefer Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`.'; + +const invalidTestCase = (code, output) => { return { code, output, errors: [ { - message: `Prefer \`${argumentName}.remove()\` over \`${parentName}.removeChild(${argumentName})\`.` + message } ] }; }; +const noFixTestCase = code => ({ + code, + output: code, + errors: [ + { + message + } + ] +}); + ruleTester.run('prefer-node-remove', rule, { valid: [ 'foo.remove()', 'this.remove()', 'remove()', 'foo.parentNode.removeChild(\'bar\')', + 'parentNode.removeChild(undefined)', + // Not `CallExpression` - 'new foo.parentNode.removeChild(foo);', + 'new parentNode.removeChild(bar);', // Not `MemberExpression` 'removeChild(foo);', // `callee.property` is not a `Identifier` - 'foo.parentNode[\'removeChild\'](foo);', + 'parentNode[\'removeChild\'](bar);', // Computed - 'foo.parentNode[removeChild](foo);', + 'parentNode[removeChild](bar);', // Not `removeChild` - 'foo.parentNode.foo(foo);', + 'parentNode.foo(bar);', // More or less argument(s) - 'foo.parentNode.removeChild(foo, two);', - 'foo.parentNode.removeChild();', - 'foo.parentNode.removeChild(...argumentsArray)' + 'parentNode.removeChild(bar, extra);', + 'parentNode.removeChild();', + 'parentNode.removeChild(...argumentsArray)', + + // TODO: support cases bellow, maybe more + 'parentNode.removeChild(some.node)', + 'parentNode.removeChild(get.child())', + 'const foo = async () => parentNode.removeChild(await getChild())', + 'parentNode.removeChild((() => childNode)())' ], invalid: [ invalidTestCase( - 'foo.parentNode.removeChild(foo)', - 'foo.remove()', - 'parentNode', - 'foo' - ), - invalidTestCase( - 'this.parentNode.removeChild(this)', - 'this.remove()', - 'parentNode', - 'this' + 'parentNode.removeChild(foo)', + 'foo.remove()' ), invalidTestCase( 'parentNode.removeChild(this)', - 'this.remove()', - 'parentNode', - 'this' - ), - invalidTestCase( - 'foo.parentNode.removeChild(bar)', - 'bar.remove()', - 'parentNode', - 'bar' - ), - invalidTestCase( - 'this.parentNode.removeChild(foo)', - 'foo.remove()', - 'parentNode', - 'foo' - ), - invalidTestCase( - 'foo.parentElement.removeChild(foo)', - 'foo.remove()', - 'parentElement', - 'foo' - ), - invalidTestCase( - 'this.parentElement.removeChild(this)', - 'this.remove()', - 'parentElement', - 'this' - ), - invalidTestCase( - 'parentElement.removeChild(this)', - 'this.remove()', - 'parentElement', - 'this' - ), - invalidTestCase( - 'foo.parentElement.removeChild(bar)', - 'bar.remove()', - 'parentElement', - 'bar' - ), - invalidTestCase( - 'this.parentElement.removeChild(foo)', - 'foo.remove()', - 'parentElement', - 'foo' - ), - invalidTestCase( - 'if (foo.parentNode.removeChild(foo)) {}', - 'if (foo.parentNode.removeChild(foo)) {}', - 'parentNode', - 'foo' - ), - invalidTestCase( - 'var removed = foo.parentNode.removeChild(foo);', - 'var removed = foo.parentNode.removeChild(foo);', - 'parentNode', - 'foo' - ), - invalidTestCase( - 'const foo = node.parentNode.removeChild(child);', - 'const foo = node.parentNode.removeChild(child);', - 'parentNode', - 'child' - ), - invalidTestCase( - 'console.log(node.parentNode.removeChild(child));', - 'console.log(node.parentNode.removeChild(child));', - 'parentNode', - 'child' - ), - invalidTestCase( - 'node.parentNode.removeChild(child) || "foo";', - 'node.parentNode.removeChild(child) || "foo";', - 'parentNode', - 'child' - ), - invalidTestCase( - 'node.parentNode.removeChild(child) + 0;', - 'node.parentNode.removeChild(child) + 0;', - 'parentNode', - 'child' - ), - invalidTestCase( - 'node.parentNode.removeChild(child) + 0;', - 'node.parentNode.removeChild(child) + 0;', - 'parentNode', - 'child' - ), - invalidTestCase( - '+node.parentNode.removeChild(child);', - '+node.parentNode.removeChild(child);', - 'parentNode', - 'child' - ), - invalidTestCase( - 'node.parentNode.removeChild(child) ? "foo" : "bar";', - 'node.parentNode.removeChild(child) ? "foo" : "bar";', - 'parentNode', - 'child' - ), - invalidTestCase( - 'if (node.parentNode.removeChild(child)) {}', - 'if (node.parentNode.removeChild(child)) {}', - 'parentNode', - 'child' - ), - invalidTestCase( - 'const foo = [node.parentNode.removeChild(child)]', - 'const foo = [node.parentNode.removeChild(child)]', - 'parentNode', - 'child' - ), - invalidTestCase( - 'const foo = { bar: node.parentNode.removeChild(child) }', - 'const foo = { bar: node.parentNode.removeChild(child) }', - 'parentNode', - 'child' - ), - invalidTestCase( - 'function foo() { return node.parentNode.removeChild(child); }', - 'function foo() { return node.parentNode.removeChild(child); }', - 'parentNode', - 'child' - ), - invalidTestCase( - 'const foo = () => { return node.parentNode.removeChild(child); }', - 'const foo = () => { return node.parentNode.removeChild(child); }', - 'parentNode', - 'child' - ), - invalidTestCase( - 'foo(bar = node.parentNode.removeChild(child))', - 'foo(bar = node.parentNode.removeChild(child))', - 'parentNode', - 'child' - ) + 'this.remove()' + ), + // Value of `parentNode.removeChild` call is used + noFixTestCase('if (parentNode.removeChild(foo)) {}'), + noFixTestCase('var removed = parentNode.removeChild(child);'), + noFixTestCase('const foo = parentNode.removeChild(child);'), + noFixTestCase('console.log(parentNode.removeChild(child));'), + noFixTestCase('parentNode.removeChild(child) || "foo";'), + noFixTestCase('parentNode.removeChild(child) + 0;'), + noFixTestCase('+parentNode.removeChild(child);'), + noFixTestCase('parentNode.removeChild(child) ? "foo" : "bar";'), + noFixTestCase('if (parentNode.removeChild(child)) {}'), + noFixTestCase('const foo = [parentNode.removeChild(child)]'), + noFixTestCase('const foo = { bar: parentNode.removeChild(child) }'), + noFixTestCase('function foo() { return parentNode.removeChild(child); }'), + noFixTestCase('const foo = () => { return parentElement.removeChild(child); }'), + noFixTestCase('foo(bar = parentNode.removeChild(child))') ] });