Skip to content

Commit

Permalink
feat(linter) eslint plugin unicorn: prefer dom node remove (#1472)
Browse files Browse the repository at this point in the history
  • Loading branch information
camc314 committed Nov 21, 2023
1 parent 1c28fee commit 0f7d6a5
Show file tree
Hide file tree
Showing 4 changed files with 498 additions and 12 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ mod unicorn {
pub mod prefer_date_now;
pub mod prefer_dom_node_append;
pub mod prefer_dom_node_dataset;
pub mod prefer_dom_node_remove;
pub mod prefer_event_target;
pub mod prefer_includes;
pub mod prefer_logical_operator_over_ternary;
Expand Down Expand Up @@ -348,6 +349,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::prefer_date_now,
unicorn::prefer_dom_node_append,
unicorn::prefer_dom_node_dataset,
unicorn::prefer_dom_node_remove,
unicorn::prefer_event_target,
unicorn::prefer_includes,
unicorn::prefer_logical_operator_over_ternary,
Expand Down
12 changes: 0 additions & 12 deletions crates/oxc_linter/src/rules/unicorn/prefer_dom_node_append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,6 @@ declare_oxc_lint!(

impl Rule for PreferDomNodeAppend {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
// if (
// !isMethodCall(node, {
// method: 'appendChild',
// argumentsLength: 1,
// optionalCall: false,
// })
// || isNodeValueNotDomNode(node.callee.object)
// || isNodeValueNotDomNode(node.arguments[0])
// ) {
// return;
// }

let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};
Expand Down
197 changes: 197 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/prefer_dom_node_remove.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
use oxc_ast::{
ast::{Argument, Expression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{
ast_util::{call_expr_method_callee_info, is_method_call},
context::LintContext,
rule::Rule,
AstNode,
};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(prefer-dom-node-remove): Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`.")]
#[diagnostic(
severity(warning),
help(
"Replace `parentNode.removeChild(childNode)` with `childNode{{dotOrQuestionDot}}remove()`."
)
)]
struct PreferDomNodeRemoveDiagnostic(#[label] pub Span);

#[derive(Debug, Default, Clone)]
pub struct PreferDomNodeRemove;

declare_oxc_lint!(
/// ### What it does
///
/// Prefers the use of `child.remove()` over `parentNode.removeChild(child)`.
///
/// ### Why is this bad?
///
/// 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).
///
/// ### Example
/// ```javascript
/// // bad
/// parentNode.removeChild(childNode);
///
/// // good
/// childNode.remove();
/// ```
PreferDomNodeRemove,
pedantic
);

impl Rule for PreferDomNodeRemove {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else { return };

if call_expr.optional {
return;
}

if !is_method_call(call_expr, None, Some(&["removeChild"]), Some(1), Some(1)) {
return;
}

let Argument::Expression(expr) = &call_expr.arguments[0] else {
return;
};

let expr = expr.without_parenthesized();
if matches!(
expr,
Expression::ArrayExpression(_)
| Expression::ArrowExpression(_)
| Expression::ClassExpression(_)
| Expression::FunctionExpression(_)
| Expression::ObjectExpression(_)
| Expression::TemplateLiteral(_)
) || expr.is_literal()
|| expr.is_null_or_undefined()
{
return;
}

ctx.diagnostic(PreferDomNodeRemoveDiagnostic(
call_expr_method_callee_info(call_expr).unwrap().0,
));
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
r"foo.remove()",
r"this.remove()",
r"remove()",
r"foo.parentNode.removeChild('bar')",
r"parentNode.removeChild(undefined)",
r"new parentNode.removeChild(bar);",
r"removeChild(foo);",
r"parentNode[removeChild](bar);",
r"parentNode.foo(bar);",
r"parentNode.removeChild(bar, extra);",
r"parentNode.removeChild();",
r"parentNode.removeChild(...argumentsArray)",
r"parentNode.removeChild?.(foo)",
];

let fail = vec![
r"parentNode.removeChild(foo)",
r"parentNode.removeChild(this)",
r"parentNode.removeChild(some.node)",
r"parentNode.removeChild(getChild())",
r"parentNode.removeChild(lib.getChild())",
r"parentNode.removeChild((() => childNode)())",
r"
async function foo () {
parentNode.removeChild(
await getChild()
);
}
",
r"
async function foo () {
parentNode.removeChild(
(await getChild())
);
}
",
r"parentNode.removeChild((0, child))",
r"parentNode.removeChild( ( (new Image)) )",
r"parentNode.removeChild( new Audio )",
r"
const array = []
parentNode.removeChild([a, b, c].reduce(child => child, child))
",
r"
async function foo () {
const array = []
parentNode.removeChild(
await getChild()
);
}
",
r"
async function foo () {
const array = []
parentNode.removeChild(
(0, childNode)
);
}
",
r"
async function foo () {
const array = []
parentNode.removeChild(
(0, childNode)
);
}
",
r"
async function foo () {
const array = []
parentNode.removeChild(
(0, childNode)
);
}
",
r"if (parentNode.removeChild(foo)) {}",
r"var removed = parentNode.removeChild(child);",
r"const foo = parentNode.removeChild(child);",
r"foo.bar(parentNode.removeChild(child));",
r#"parentNode.removeChild(child) || "foo";"#,
r"parentNode.removeChild(child) + 0;",
r"+parentNode.removeChild(child);",
r#"parentNode.removeChild(child) ? "foo" : "bar";"#,
r"if (parentNode.removeChild(child)) {}",
r"const foo = [parentNode.removeChild(child)]",
r"const foo = { bar: parentNode.removeChild(child) }",
r"function foo() { return parentNode.removeChild(child); }",
r"const foo = () => { return parentElement.removeChild(child); }",
r"foo(bar = parentNode.removeChild(child))",
r"foo().removeChild(child)",
r"foo[doSomething()].removeChild(child)",
r"parentNode?.removeChild(foo)",
r"foo?.parentNode.removeChild(foo)",
r"foo.parentNode?.removeChild(foo)",
r"foo?.parentNode?.removeChild(foo)",
r"foo.bar?.parentNode.removeChild(foo.bar)",
r"a.b?.c.parentNode.removeChild(foo)",
r"a[b?.c].parentNode.removeChild(foo)",
r"a?.b.parentNode.removeChild(a.b)",
];

Tester::new_without_config(PreferDomNodeRemove::NAME, pass, fail).test_and_snapshot();
}
Loading

0 comments on commit 0f7d6a5

Please sign in to comment.