Skip to content

Commit

Permalink
feat(linter): overhaul unicorn/no-useless-spread (#4791)
Browse files Browse the repository at this point in the history
I got tired of seeing useless spreads on ternaries and `arr.reduce()` within my company's internal codebase so I overhauled this rule.

## Changes
- add fixer for object spreads
  ```js
  const before = { a, ...{ b, c }, d }
  const after = { a,  b, c, d } // fixer does not dedupe spaces before `b`
  ```
- recursively check for useless clones on complex expressions. This rule now catches and auto-fixes the following cases:
   ```js
  // ternaries when both branches create a new array or object
   const obj = { ...(foo ? { a: 1 } : { b: 2 }) }
  // recursive, so this can support complex cases
  const arr = [ ...(foo ? a.map(fn) : bar ? Array.from(iter) : await Promise.all(bar)) ]
  // reduce functions where the initial accumulator creates a new object or array
   const obj = { ...(arr.reduce(fn, {}) }
  ```
  • Loading branch information
DonIsaac committed Aug 10, 2024
1 parent 5992b75 commit b3c3125
Show file tree
Hide file tree
Showing 5 changed files with 659 additions and 187 deletions.
18 changes: 4 additions & 14 deletions crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,20 +233,10 @@ 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())
ctx.nodes()
.iter_parents(node.id())
.skip(1)
.find(|parent| !matches!(parent.kind(), AstKind::ParenthesizedExpression(_)))
}

pub fn get_declaration_of_variable<'a, 'b>(
Expand Down
232 changes: 232 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/no_useless_spread/const_eval.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
use oxc_ast::ast::{
match_expression, Argument, CallExpression, ConditionalExpression, Expression, NewExpression,
};

use crate::ast_util::{is_method_call, is_new_expression};

#[derive(Debug, Clone)]
pub(super) enum ValueHint {
NewObject,
NewArray,
Promise(Box<ValueHint>),
Unknown,
}

impl ValueHint {
pub fn r#await(self) -> Self {
match self {
Self::Promise(inner) => *inner,
_ => self,
}
}

#[inline]
pub fn is_object(&self) -> bool {
matches!(self, Self::NewObject)
}

#[inline]
pub fn is_array(&self) -> bool {
matches!(self, Self::NewArray)
}
}

impl std::ops::BitAnd for ValueHint {
type Output = Self;
fn bitand(self, rhs: Self) -> Self::Output {
match (self, rhs) {
(Self::NewArray, Self::NewArray) => Self::NewArray,
(Self::NewObject, Self::NewObject) => Self::NewObject,
_ => Self::Unknown,
}
}
}
pub(super) trait ConstEval {
fn const_eval(&self) -> ValueHint;
}

impl<'a> ConstEval for Expression<'a> {
fn const_eval(&self) -> ValueHint {
match self.get_inner_expression() {
Self::ArrayExpression(_) => ValueHint::NewArray,
Self::ObjectExpression(_) => ValueHint::NewObject,
Self::AwaitExpression(expr) => expr.argument.const_eval().r#await(),
Self::SequenceExpression(expr) => {
expr.expressions.last().map_or(ValueHint::Unknown, ConstEval::const_eval)
}
Self::ConditionalExpression(cond) => cond.const_eval(),
Self::CallExpression(call) => call.const_eval(),
Self::NewExpression(new) => new.const_eval(),
_ => ValueHint::Unknown,
}
}
}

impl<'a> ConstEval for ConditionalExpression<'a> {
fn const_eval(&self) -> ValueHint {
self.consequent.const_eval() & self.alternate.const_eval()
}
}

impl<'a> ConstEval for Argument<'a> {
fn const_eval(&self) -> ValueHint {
match self {
// using a spread as an initial accumulator value creates a new
// object or array
Self::SpreadElement(spread) => spread.argument.const_eval(),
expr @ match_expression!(Argument) => expr.as_expression().unwrap().const_eval(),
}
}
}

impl<'a> ConstEval for NewExpression<'a> {
fn const_eval(&self) -> ValueHint {
if is_new_array(self) || is_new_map_or_set(self) || is_new_typed_array(self) {
ValueHint::NewArray
} else if is_new_object(self) {
ValueHint::NewObject
} else {
ValueHint::Unknown
}
}
}

fn is_new_array(new_expr: &NewExpression) -> bool {
is_new_expression(new_expr, &["Array"], None, None)
}

/// Matches `new {Set,WeakSet,Map,WeakMap}(iterable?)`
fn is_new_map_or_set(new_expr: &NewExpression) -> bool {
is_new_expression(new_expr, &["Map", "WeakMap", "Set", "WeakSet"], None, Some(1))
}

/// Matches `new Object()` with any number of args.
fn is_new_object(new_expr: &NewExpression) -> bool {
is_new_expression(new_expr, &["Object"], None, None)
}

/// Matches `new <TypedArray>(a, [other args])` with >= 1 arg
pub fn is_new_typed_array(new_expr: &NewExpression) -> bool {
is_new_expression(
new_expr,
&[
"Int8Array",
"Uint8Array",
"Uint8ClampedArray",
"Int16Array",
"Uint16Array",
"Int32Array",
"Uint32Array",
"Float32Array",
"Float64Array",
"BigInt64Array",
"BigUint64Array",
],
Some(1),
None,
)
}

impl<'a> ConstEval for CallExpression<'a> {
fn const_eval(&self) -> ValueHint {
if is_array_from(self)
|| is_split_method(self)
|| is_array_factory(self)
|| is_functional_array_method(self)
|| is_array_producing_obj_method(self)
{
ValueHint::NewArray
} else if is_array_reduce(self) {
self.arguments[1].const_eval()
} else if is_promise_array_method(self) {
ValueHint::Promise(Box::new(ValueHint::NewArray))
} else if is_obj_factory(self) {
ValueHint::NewObject
} else {
// TODO: check initial value for arr.reduce() accumulators
ValueHint::Unknown
}
}
}

/// - `Array.from(x)`
/// - `Int8Array.from(x)`
/// - plus all other typed arrays
pub fn is_array_from(call_expr: &CallExpression) -> bool {
is_method_call(
call_expr,
Some(&[
"Array",
"Int8Array",
"Uint8Array",
"Uint8ClampedArray",
"Int16Array",
"Uint16Array",
"Int32Array",
"Uint32Array",
"Float32Array",
"Float64Array",
"BigInt64Array",
"BigUint64Array",
]),
Some(&["from"]),
Some(1),
Some(1),
)
}
/// `<expr>.{concat,map,filter,...}`
fn is_functional_array_method(call_expr: &CallExpression) -> bool {
is_method_call(
call_expr,
None,
Some(&[
"concat",
"copyWithin",
"filter",
"flat",
"flatMap",
"map",
"slice",
"splice",
"toReversed",
"toSorted",
"toSpliced",
"with",
]),
None,
None,
)
}

/// Matches `<expr>.reduce(a, b)`, which usually looks like
/// ```ts
/// arr.reduce(reducerRn, initialAccumulator)
/// ```
fn is_array_reduce(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, None, Some(&["reduce"]), Some(2), Some(2))
}

/// Matches `<expr>.split(...)`, which usually is `String.prototype.split(pattern)`
fn is_split_method(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, None, Some(&["split"]), None, None)
}

/// Matches `Object.{fromEntries,create}(x)`
fn is_obj_factory(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, Some(&["Object"]), Some(&["fromEntries", "create"]), Some(1), Some(1))
}

/// Matches `Object.{keys,values,entries}(...)`
fn is_array_producing_obj_method(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, Some(&["Object"]), Some(&["keys", "values", "entries"]), None, None)
}

/// Matches `Array.{from,of}(...)`
fn is_array_factory(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, Some(&["Array"]), Some(&["from", "of"]), None, None)
}

/// Matches `Promise.{all,allSettled}(x)`
fn is_promise_array_method(call_expr: &CallExpression) -> bool {
is_method_call(call_expr, Some(&["Promise"]), Some(&["all", "allSettled"]), Some(1), Some(1))
}
Loading

0 comments on commit b3c3125

Please sign in to comment.