Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
prefer-node-remove: Only fix when expression is not used (#498)
  • Loading branch information
brettz9 authored and sindresorhus committed Jan 21, 2020
1 parent 5724f58 commit b1d3f37
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 32 deletions.
33 changes: 3 additions & 30 deletions 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,
Expand Down
5 changes: 4 additions & 1 deletion 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;
Expand Down Expand Up @@ -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
});
}
}
Expand Down
25 changes: 25 additions & 0 deletions 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));
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions test/prefer-node-append.js
Expand Up @@ -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]
}
]
});
90 changes: 90 additions & 0 deletions test/prefer-node-remove.js
Expand Up @@ -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'
)
]
});

0 comments on commit b1d3f37

Please sign in to comment.