diff --git a/crates/oxc_linter/src/rules/eslint/prefer_destructuring.rs b/crates/oxc_linter/src/rules/eslint/prefer_destructuring.rs index 28e09799509f2..f06fbb170696e 100644 --- a/crates/oxc_linter/src/rules/eslint/prefer_destructuring.rs +++ b/crates/oxc_linter/src/rules/eslint/prefer_destructuring.rs @@ -86,7 +86,7 @@ declare_oxc_lint!( PreferDestructuring, eslint, style, - pending, + fix, config = PreferDestructuring, ); @@ -161,7 +161,20 @@ impl Rule for PreferDestructuring { && get_target_name(&assign_expr.left) .is_some_and(|v| v == string_literal.value) { - ctx.diagnostic(prefer_object_destructuring(assign_expr.span)); + ctx.diagnostic_with_fix( + prefer_object_destructuring(assign_expr.span), + |fixer| { + generate_fix( + &fixer, + string_literal.span.shrink(1), + get_object_span_without_redundant_parentheses( + &comp_expr.object, + ), + assign_expr.span, + true, + ) + }, + ); } } } @@ -171,7 +184,21 @@ impl Rule for PreferDestructuring { if get_target_name(&assign_expr.left) .is_some_and(|name| name == static_expr.property.name.as_str()) { - ctx.diagnostic(prefer_object_destructuring(assign_expr.span)); + // Safe autofix for assignments: foo = object.foo; -> ({ foo } = object); + ctx.diagnostic_with_fix( + prefer_object_destructuring(assign_expr.span), + |fixer| { + generate_fix( + &fixer, + static_expr.property.span, + get_object_span_without_redundant_parentheses( + &static_expr.object, + ), + assign_expr.span, + true, + ) + }, + ); } } _ => {} @@ -210,7 +237,20 @@ impl Rule for PreferDestructuring { && self.variable_declarator.object && name.is_some_and(|v| v == string_literal.value) { - ctx.diagnostic(prefer_object_destructuring(init.span())); + ctx.diagnostic_with_fix( + prefer_object_destructuring(init.span()), + |fixer| { + generate_fix( + &fixer, + string_literal.span.shrink(1), + get_object_span_without_redundant_parentheses( + &comp_expr.object, + ), + declarator.span(), + false, + ) + }, + ); } } } @@ -221,7 +261,20 @@ impl Rule for PreferDestructuring { ctx.diagnostic(prefer_object_destructuring(right.span())); } if name.is_some_and(|name| name == static_expr.property.name.as_str()) { - ctx.diagnostic(prefer_object_destructuring(init.span())); + ctx.diagnostic_with_fix( + prefer_object_destructuring(init.span()), + |fixer| { + generate_fix( + &fixer, + static_expr.property.span, + get_object_span_without_redundant_parentheses( + &static_expr.object, + ), + declarator.span(), + false, + ) + }, + ); } } _ => {} @@ -249,6 +302,41 @@ fn check_expr(expr: &MemberExpression) -> bool { true } +/// Returns the span of the object expression, stripping redundant parentheses for expressions +/// where they are unnecessary in the destructuring context. +/// +/// For example: `(bar[baz]).foo` -> uses span of `bar[baz]` (without parens) +/// But: `(a, b).foo` -> uses span of `(a, b)` (keeps parens, comma operator needs them) +fn get_object_span_without_redundant_parentheses(object: &Expression) -> Span { + match object.without_parentheses() { + Expression::CallExpression(_) + | Expression::Identifier(_) + | Expression::StaticMemberExpression(_) + | Expression::ComputedMemberExpression(_) + | Expression::ThisExpression(_) => object.without_parentheses().span(), + _ => object.span(), + } +} + +/// Generate the fix for object destructuring. +/// `is_assignment` determines whether to wrap in parentheses for assignment expressions. +fn generate_fix( + fixer: &crate::fixer::RuleFixer<'_, '_>, + prop_span: Span, + object_span: Span, + replacement_span: Span, + is_assignment: bool, +) -> crate::fixer::RuleFix { + let prop = fixer.source_range(prop_span); + let object_text = fixer.source_range(object_span); + let replacement = if is_assignment { + format!("({{ {prop} }} = {object_text})") + } else { + format!("{{{prop}}} = {object_text}") + }; + fixer.replace(replacement_span, replacement) +} + #[test] fn test() { use crate::tester::Tester; @@ -572,44 +660,47 @@ fn test() { ("var foo = object.foo, /* comment */ a;", None), ]; - // pending - // let fix = vec![ - // ("var foo = object.foo;", "var {foo} = object;", None), - // ("var foo = (a, b).foo;", "var {foo} = (a, b);", None), - // ("var length = (() => {}).length;", "var {length} = () => {};", None), - // ("var foo = (a = b).foo;", "var {foo} = a = b;", None), - // ("var foo = (a || b).foo;", "var {foo} = a || b;", None), - // ("var foo = (f()).foo;", "var {foo} = f();", None), - // ("var foo = object.bar.foo;", "var {foo} = object.bar;", None), - // ( - // "class Foo extends Bar { static foo() {var bar = super.foo.bar} }", - // "class Foo extends Bar { static foo() {var {bar} = super.foo} }", - // None, - // ), - // ("var /* comment */ foo = object.foo;", "var /* comment */ {foo} = object;", None), - // ("var a, /* comment */foo = object.foo;", "var a, /* comment */{foo} = object;", None), - // ("var foo = bar(/* comment */).foo;", "var {foo} = bar(/* comment */);", None), - // ("var foo = bar/* comment */.baz.foo;", "var {foo} = bar/* comment */.baz;", None), - // ( - // "var foo = bar[// comment - // baz].foo;", - // "var {foo} = bar[// comment - // baz];", - // None, - // ), - // ("var foo = object.foo/* comment */;", "var {foo} = object/* comment */;", None), - // ("var foo = object.foo// comment", "var {foo} = object// comment", None), - // ("var foo = object.foo/* comment */, a;", "var {foo} = object/* comment */, a;", None), - // ( - // "var foo = object.foo// comment - // , a;", - // "var {foo} = object// comment - // , a;", - // None, - // ), - // ("var foo = object.foo, /* comment */ a;", "var {foo} = object, /* comment */ a;", None), - // ]; + let fix: Vec<(&str, &str, Option)> = vec![ + ("var foo = object.foo;", "var {foo} = object;", None), + ("var foo = (a, b).foo;", "var {foo} = (a, b);", None), + // ("var length = (() => {}).length;", "var {length} = () => {};", None), + // ("var foo = (a = b).foo;", "var {foo} = a = b;", None), + // ("var foo = (a || b).foo;", "var {foo} = a || b;", None), + ("var foo = (f()).foo;", "var {foo} = f();", None), + ("var foo = object.bar.foo;", "var {foo} = object.bar;", None), + ( + "class Foo extends Bar { static foo() {var bar = super.foo.bar} }", + "class Foo extends Bar { static foo() {var {bar} = super.foo} }", + None, + ), + ("var /* comment */ foo = object.foo;", "var /* comment */ {foo} = object;", None), + ("var a, /* comment */foo = object.foo;", "var a, /* comment */{foo} = object;", None), + ("var foo = bar(/* comment */).foo;", "var {foo} = bar(/* comment */);", None), + ("var foo = bar/* comment */.baz.foo;", "var {foo} = bar/* comment */.baz;", None), + ( + "var foo = bar[// comment + baz].foo;", + "var {foo} = bar[// comment + baz];", + None, + ), + ("var foo = (bar[baz]).foo;", "var {foo} = bar[baz];", None), + ("var foo = object.foo/* comment */;", "var {foo} = object/* comment */;", None), + ("var foo = object.foo// comment", "var {foo} = object// comment", None), + ("var foo = object.foo/* comment */, a;", "var {foo} = object/* comment */, a;", None), + ( + "var foo = object.foo// comment + , a;", + "var {foo} = object// comment + , a;", + None, + ), + ("var foo = object.foo, /* comment */ a;", "var {foo} = object, /* comment */ a;", None), + ("var foo = object['foo'];", "var {foo} = object;", None), + ("foo = object.foo;", "({ foo } = object);", None), + ]; Tester::new(PreferDestructuring::NAME, PreferDestructuring::PLUGIN, pass, fail) + .expect_fix(fix) .test_and_snapshot(); }