diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index b4728ac53..acafcd78f 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -244,6 +244,26 @@ pub fn outermost_paren<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) node } +pub fn outermost_paren_parent<'a, 'b>( + node: &'b AstNode<'a>, + ctx: &'b LintContext<'a>, +) -> Option<&'b AstNode<'a>> { + let mut node = node; + + loop { + if let Some(parent) = ctx.nodes().parent_node(node.id()) { + if let AstKind::ParenthesizedExpression(_) = parent.kind() { + node = parent; + continue; + } + } + + break; + } + + ctx.nodes().parent_node(node.id()) +} + pub fn get_name_from_property_key(key: &PropertyKey<'_>) -> Option { match key { PropertyKey::Identifier(ident) => Some(ident.name.clone()), diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 45f8f6f6d..c41725d4c 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -154,6 +154,7 @@ mod unicorn { pub mod no_static_only_class; pub mod no_thenable; pub mod no_unnecessary_await; + pub mod no_useless_fallback_in_spread; pub mod prefer_add_event_listener; pub mod prefer_array_flat_map; pub mod prefer_blob_reading_methods; @@ -289,6 +290,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::no_static_only_class, unicorn::no_thenable, unicorn::no_unnecessary_await, + unicorn::no_useless_fallback_in_spread, unicorn::prefer_add_event_listener, unicorn::prefer_array_flat_map, unicorn::prefer_blob_reading_methods, diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_fallback_in_spread.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_fallback_in_spread.rs new file mode 100644 index 000000000..0e7d1947c --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_fallback_in_spread.rs @@ -0,0 +1,128 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use oxc_syntax::operator::LogicalOperator; + +use crate::{ast_util::outermost_paren_parent, context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals")] +#[diagnostic(severity(warning), help("Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback."))] +struct NoUselessFallbackInSpreadDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoUselessFallbackInSpread; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow useless fallback when spreading in object literals. + /// + /// ### Why is this bad? + /// + /// Spreading [falsy values](https://developer.mozilla.org/en-US/docs/Glossary/Falsy) in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + /// + /// ### Example + /// ```javascript + /// // bad + /// const object = { ...(foo || {}) } + /// + /// // good + /// const object = { ...foo } + /// const object = { ...(foo || { not: "empty" }) } + /// + /// ``` + NoUselessFallbackInSpread, + correctness +); + +impl Rule for NoUselessFallbackInSpread { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::LogicalExpression(logical_expression) = node.kind() else { return }; + + if !matches!(logical_expression.operator, LogicalOperator::Or | LogicalOperator::Coalesce) { + return; + } + + let Expression::ObjectExpression(object_expression) = + &logical_expression.right.without_parenthesized() + else { + return; + }; + + if object_expression.properties.len() != 0 { + return; + } + + let Some(parent) = outermost_paren_parent(node, ctx) else { return }; + + let AstKind::SpreadElement(spread_element) = parent.kind() else { return }; + + let Some(parent) = outermost_paren_parent(parent, ctx) else { return }; + + if !matches!(parent.kind(), AstKind::ObjectExpression(_)) { + return; + } + + ctx.diagnostic(NoUselessFallbackInSpreadDiagnostic(spread_element.span)); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r#"const array = [...(foo || [])]"#, + r#"const array = [...(foo || {})]"#, + r#"const array = [...(foo && {})]"#, + r#"const object = {...(foo && {})}"#, + r#"const object = {...({} || foo)}"#, + r#"const object = {...({} && foo)}"#, + r#"const object = {...({} ?? foo)}"#, + r#"const object = {...(foo ? foo : {})}"#, + r#"const object = {...foo}"#, + r#"const object = {...(foo ?? ({} || {}))}"#, + r#"const {...foo} = object"#, + r#"function foo({...bar}){}"#, + r#"const object = {...(foo || {}).toString()}"#, + r#"const object = {...fn(foo || {})}"#, + r#"const object = call({}, ...(foo || {}))"#, + r#"const object = {...(foo || {not: "empty"})}"#, + r#"const object = {...(foo || {...{}})}"#, + ]; + + let fail = vec![ + r#"const object = {...(foo || {})}"#, + r#"const object = {...(foo ?? {})}"#, + r#"const object = {...(foo ?? (( {} )))}"#, + r#"const object = {...((( foo )) ?? (( {} )))}"#, + r#"const object = {...(( (( foo )) ?? (( {} )) ))}"#, + r#"async ()=> ({...((await foo) || {})})"#, + r#"const object = {...(0 || {})}"#, + r#"const object = {...((-0) || {})}"#, + r#"const object = {...(.0 || {})}"#, + r#"const object = {...(0n || {})}"#, + r#"const object = {...(false || {})}"#, + r#"const object = {...(null || {})}"#, + r#"const object = {...(undefined || {})}"#, + r#"const object = {...((a && b) || {})}"#, + r#"const object = {...(NaN || {})}"#, + r#"const object = {...("" || {})}"#, + r#"const object = {...([] || {})}"#, + r#"const object = {...({} || {})}"#, + r#"const object = {...(foo || {}),}"#, + r#"const object = {...((foo ?? {}) || {})}"#, + r#"const object = {...((foo && {}) || {})}"#, + r#"const object = {...(foo && {} || {})}"#, + r#"const object = {...({...(foo || {})})}"#, + r#"function foo(a = {...(bar || {})}){}"#, + r#"const object = {...(document.all || {})}"#, + ]; + + Tester::new_without_config(NoUselessFallbackInSpread::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/rules/unicorn/throw_new_error.rs b/crates/oxc_linter/src/rules/unicorn/throw_new_error.rs index 686b8ed4f..4c08f3899 100644 --- a/crates/oxc_linter/src/rules/unicorn/throw_new_error.rs +++ b/crates/oxc_linter/src/rules/unicorn/throw_new_error.rs @@ -9,7 +9,12 @@ use oxc_macros::declare_oxc_lint; use oxc_span::Span; use regex::Regex; -use crate::{ast_util::outermost_paren, context::LintContext, rule::Rule, AstNode}; +use crate::{ + ast_util::{outermost_paren, outermost_paren_parent}, + context::LintContext, + rule::Rule, + AstNode, +}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-unicorn(throw-new-error): Require `new` when throwing an error.")] @@ -49,10 +54,7 @@ impl Rule for ThrowNewError { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::CallExpression(call_expr) = node.kind() else { return }; - let Some(outermost_paren_node) = ctx.nodes().parent_node(outermost_paren(node, ctx).id()) - else { - return; - }; + let Some(outermost_paren_node) = outermost_paren_parent(node, ctx) else { return }; let AstKind::ThrowStatement(_) = outermost_paren(outermost_paren_node, ctx).kind() else { return; diff --git a/crates/oxc_linter/src/snapshots/no_useless_fallback_in_spread.snap b/crates/oxc_linter/src/snapshots/no_useless_fallback_in_spread.snap new file mode 100644 index 000000000..698c53048 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_useless_fallback_in_spread.snap @@ -0,0 +1,180 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_useless_fallback_in_spread +--- + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(foo || {})} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(foo ?? {})} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(foo ?? (( {} )))} + · ──────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...((( foo )) ?? (( {} )))} + · ────────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(( (( foo )) ?? (( {} )) ))} + · ────────────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ async ()=> ({...((await foo) || {})}) + · ────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(0 || {})} + · ──────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...((-0) || {})} + · ─────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(.0 || {})} + · ───────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(0n || {})} + · ───────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(false || {})} + · ──────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(null || {})} + · ─────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(undefined || {})} + · ──────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...((a && b) || {})} + · ─────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(NaN || {})} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...("" || {})} + · ───────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...([] || {})} + · ───────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...({} || {})} + · ───────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(foo || {}),} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...((foo ?? {}) || {})} + · ────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...((foo && {}) || {})} + · ────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(foo && {} || {})} + · ──────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...({...(foo || {})})} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ function foo(a = {...(bar || {})}){} + · ────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + + ⚠ eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals + ╭─[no_useless_fallback_in_spread.tsx:1:1] + 1 │ const object = {...(document.all || {})} + · ─────────────────────── + ╰──── + help: Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback. + +