diff --git a/rules/prefer-node-append.js b/rules/prefer-node-append.js index 9144a8ba9e..4b69928c1d 100644 --- a/rules/prefer-node-append.js +++ b/rules/prefer-node-append.js @@ -1,43 +1,16 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); +const isValueUsed = require('./utils/is-value-used'); const getMethodName = memberExpression => memberExpression.property.name; -const ignoredParentTypes = [ - 'ArrayExpression', - 'IfStatement', - 'MemberExpression', - 'Property', - 'ReturnStatement', - 'VariableDeclarator' -]; - -const ignoredGrandparentTypes = [ - 'ExpressionStatement' -]; - const create = context => { return { CallExpression(node) { - const { - callee, - parent - } = node; - - const { - parent: grandparent - } = (parent || {}); + const {callee} = node; if (callee.type === 'MemberExpression' && getMethodName(callee) === 'appendChild') { - let fix = fixer => fixer.replaceText(callee.property, 'append'); - - if (parent && ignoredParentTypes.includes(parent.type)) { - fix = undefined; - } - - if (grandparent && ignoredGrandparentTypes.includes(grandparent.type)) { - fix = undefined; - } + const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(callee.property, 'append'); context.report({ node, diff --git a/rules/prefer-node-remove.js b/rules/prefer-node-remove.js index 280c09d3fe..b5d6a28079 100644 --- a/rules/prefer-node-remove.js +++ b/rules/prefer-node-remove.js @@ -1,5 +1,6 @@ 'use strict'; const getDocumentationUrl = require('./utils/get-documentation-url'); +const isValueUsed = require('./utils/is-value-used'); const getMethodName = callee => { const {property} = callee; @@ -65,10 +66,12 @@ const create = context => { const argumentName = getArgumentName(node.arguments); if (argumentName) { + const fix = isValueUsed(node) ? undefined : fixer => fixer.replaceText(node, `${argumentName}.remove()`); + context.report({ node, message: `Prefer \`${argumentName}.remove()\` over \`${callerName}.removeChild(${argumentName})\`.`, - fix: fixer => fixer.replaceText(node, `${argumentName}.remove()`) + fix }); } } diff --git a/rules/utils/is-value-used.js b/rules/utils/is-value-used.js new file mode 100644 index 0000000000..f468361a48 --- /dev/null +++ b/rules/utils/is-value-used.js @@ -0,0 +1,25 @@ +'use strict'; + +const ignoredParentTypes = [ + 'ArrayExpression', + 'AssignmentExpression', + 'IfStatement', + 'MemberExpression', + 'Property', + 'ReturnStatement', + 'VariableDeclarator' +]; + +const ignoredGrandparentTypes = [ + 'ExpressionStatement' +]; + +module.exports = function (node) { + const {parent} = node; + const { + parent: grandparent + } = (parent || {}); + + return (parent && ignoredParentTypes.includes(parent.type)) || + (grandparent && ignoredGrandparentTypes.includes(grandparent.type)); +}; diff --git a/test/integration/eslint-config-unicorn-tester/package-lock.json b/test/integration/eslint-config-unicorn-tester/package-lock.json index 55060d39ca..f52f37b93c 100644 --- a/test/integration/eslint-config-unicorn-tester/package-lock.json +++ b/test/integration/eslint-config-unicorn-tester/package-lock.json @@ -394,7 +394,7 @@ "lodash.kebabcase": "^4.1.1", "lodash.snakecase": "^4.1.1", "lodash.upperfirst": "^4.3.1", - "read-pkg-up": "^7.0.0", + "read-pkg-up": "^7.0.1", "regexp-tree": "^0.1.17", "regexpp": "^3.0.0", "reserved-words": "^0.1.2", diff --git a/test/prefer-node-append.js b/test/prefer-node-append.js index 5a7324b86c..ea2fb3db8a 100644 --- a/test/prefer-node-append.js +++ b/test/prefer-node-append.js @@ -118,6 +118,11 @@ ruleTester.run('prefer-node-append', rule, { code: 'const foo = () => { return node.appendChild(child); }', output: 'const foo = () => { return node.appendChild(child); }', errors: [error] + }, + { + code: 'foo(bar = node.appendChild(child))', + output: 'foo(bar = node.appendChild(child))', + errors: [error] } ] }); diff --git a/test/prefer-node-remove.js b/test/prefer-node-remove.js index f1925f8d75..eea01bf47b 100644 --- a/test/prefer-node-remove.js +++ b/test/prefer-node-remove.js @@ -90,6 +90,96 @@ ruleTester.run('prefer-node-remove', rule, { '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' ) ] });