Skip to content

Commit

Permalink
feat(es/lints): Add prefer-object-spread rule (#8696)
Browse files Browse the repository at this point in the history
  • Loading branch information
ArturAralin authored Mar 11, 2024
1 parent a3d877e commit aa9297b
Show file tree
Hide file tree
Showing 6 changed files with 343 additions and 0 deletions.
10 changes: 10 additions & 0 deletions crates/swc/tests/errors/lints/prefer-object-spread/default/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"jsc": {
"target": "ES2018",
"lints": {
"preferObjectSpread": [
"error"
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
Object.assign({}, foo);

Object.assign({}, { foo: 'bar' });

Object.assign({}, baz, { foo: 'bar' });

Object.assign({}, { foo: 'bar', baz: 'foo' });

Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz))

Object.assign({});

Object['assign']({});

Object[`assign`]({});

// Valid

Object.assign(foo, { bar: baz });
Object.assign();
let a = Object.assign(a, b);
Object.assign(...foo);
Object.assign(foo, { bar: baz });
Object.assign({}, ...objects);
() => {
const Object = {};
Object.assign({}, foo);
};
Object.assign({ get a() {} }, {});
Object.assign({ set a(val) {} }, {});
Object.assign({ get a() {} }, foo);
Object.assign({ set a(val) {} }, foo);
Object.assign({ foo: 'bar', get a() {}, baz: 'quux' }, quuux);
Object.assign({ foo: 'bar', set a(val) {} }, { baz: 'quux' });
Object.assign({}, { get a() {} });
Object.assign({}, { set a(val) {} });
Object.assign({}, { foo: 'bar', get a() {} }, {});
Object.assign({ foo }, bar, {}, { baz: 'quux', set a(val) {}, quuux }, {});
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[1:1]
1 | Object.assign({}, foo);
: ^^^^^^^^^^^^^^^^^^^^^^
2 |
3 | Object.assign({}, { foo: 'bar' });
`----

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[1:1]
1 | Object.assign({}, foo);
2 |
3 | Object.assign({}, { foo: 'bar' });
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4 |
5 | Object.assign({}, baz, { foo: 'bar' });
`----

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[2:1]
2 |
3 | Object.assign({}, { foo: 'bar' });
4 |
5 | Object.assign({}, baz, { foo: 'bar' });
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6 |
7 | Object.assign({}, { foo: 'bar', baz: 'foo' });
`----

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[4:1]
4 |
5 | Object.assign({}, baz, { foo: 'bar' });
6 |
7 | Object.assign({}, { foo: 'bar', baz: 'foo' });
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 |
9 | Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz))
`----

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[6:1]
6 |
7 | Object.assign({}, { foo: 'bar', baz: 'foo' });
8 |
9 | Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz))
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
10 |
11 | Object.assign({});
`----

x "Use an object spread instead of `Object.assign` eg: `{ ...foo }`"
,-[6:1]
6 |
7 | Object.assign({}, { foo: 'bar', baz: 'foo' });
8 |
9 | Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz))
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
10 |
11 | Object.assign({});
`----

x "Use an object literal instead of `Object.assign`. eg: `{ foo: bar }`"
,-[8:1]
8 |
9 | Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz))
10 |
11 | Object.assign({});
: ^^^^^^^^^^^^^^^^^
12 |
13 | Object['assign']({});
`----

x "Use an object literal instead of `Object.assign`. eg: `{ foo: bar }`"
,-[10:1]
10 |
11 | Object.assign({});
12 |
13 | Object['assign']({});
: ^^^^^^^^^^^^^^^^^^^^
14 |
15 | Object[`assign`]({});
`----

x "Use an object literal instead of `Object.assign`. eg: `{ foo: bar }`"
,-[12:1]
12 |
13 | Object['assign']({});
14 |
15 | Object[`assign`]({});
: ^^^^^^^^^^^^^^^^^^^^
16 |
17 | // Valid
`----
4 changes: 4 additions & 0 deletions crates/swc_ecma_lints/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,8 @@ pub struct LintConfig {
pub no_prototype_builtins: RuleConfig<()>,
#[serde(default, alias = "noNewObject")]
pub no_new_object: RuleConfig<()>,

#[cfg(feature = "non_critical_lints")]
#[serde(default, alias = "preferObjectSpread")]
pub prefer_object_spread: RuleConfig<()>,
}
7 changes: 7 additions & 0 deletions crates/swc_ecma_lints/src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub(crate) mod non_critical_lints {
pub mod no_use_before_define;
pub mod no_var;
pub mod prefer_const;
pub mod prefer_object_spread;
pub mod prefer_regex_literals;
pub mod quotes;
pub mod radix;
Expand Down Expand Up @@ -205,6 +206,12 @@ pub fn all(lint_params: LintParams) -> Vec<Box<dyn Rule>> {
unresolved_ctxt,
&lint_config.no_new_object,
));

rules.extend(prefer_object_spread::prefer_object_spread(
&lint_config.prefer_object_spread,
unresolved_ctxt,
es_version,
));
}

rules
Expand Down
189 changes: 189 additions & 0 deletions crates/swc_ecma_lints/src/rules/prefer_object_spread.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
use swc_common::{errors::HANDLER, Span, SyntaxContext};
use swc_ecma_ast::*;
use swc_ecma_utils::ExprExt;
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};

use crate::{
config::{LintRuleReaction, RuleConfig},
rule::{visitor_rule, Rule},
};

const USE_SPREAD_MESSAGE: &str =
r#""Use an object spread instead of `Object.assign` eg: `{ ...foo }`""#;

const USE_LITERAL_MESSAGE: &str =
r#""Use an object literal instead of `Object.assign`. eg: `{ foo: bar }`""#;

pub fn prefer_object_spread(
config: &RuleConfig<()>,
unresolved_ctxt: SyntaxContext,
es_version: EsVersion,
) -> Option<Box<dyn Rule>> {
if es_version < EsVersion::Es2018 {
return None;
}

let rule_reaction = config.get_rule_reaction();

match rule_reaction {
LintRuleReaction::Off => None,
_ => Some(visitor_rule(PreferObjectSpread::new(
rule_reaction,
unresolved_ctxt,
))),
}
}

#[derive(Debug, Default)]
struct PreferObjectSpread {
expected_reaction: LintRuleReaction,
unresolved_ctxt: SyntaxContext,
}

#[derive(Debug)]
enum ArgType {
EmptyLiteralObject,
LiteralObjectWithFields,
LiteralObjectWithGetterOrSetter,
Ident,
Spread,
Other,
}

impl PreferObjectSpread {
fn new(expected_reaction: LintRuleReaction, unresolved_ctxt: SyntaxContext) -> Self {
Self {
expected_reaction,
unresolved_ctxt,
}
}

fn emit_report(&self, span: Span, message: &str) {
HANDLER.with(|handler| match self.expected_reaction {
LintRuleReaction::Error => {
handler.struct_span_err(span, message).emit();
}
LintRuleReaction::Warning => {
handler.struct_span_warn(span, message).emit();
}
_ => {}
});
}

fn recognize_expr_arg(expr: &Expr) -> ArgType {
match expr {
Expr::Object(obj) => {
if obj.props.is_empty() {
ArgType::EmptyLiteralObject
} else {
let has_getter_or_setter = obj.props.iter().any(|prop| {
if let Some(prop) = prop.as_prop() {
return matches!(prop.as_ref(), Prop::Setter(_) | Prop::Getter(_));
}

false
});

if has_getter_or_setter {
ArgType::LiteralObjectWithGetterOrSetter
} else {
ArgType::LiteralObjectWithFields
}
}
}
Expr::Ident(_) => ArgType::Ident,
Expr::Paren(paren) => Self::recognize_expr_arg(&paren.expr),
Expr::Seq(seq) => {
let last = seq.exprs.last().unwrap();

Self::recognize_expr_arg(last)
}
_ => ArgType::Other,
}
}

fn recognize_arg(expr_or_spread: &ExprOrSpread) -> ArgType {
if expr_or_spread.spread.is_some() {
return ArgType::Spread;
}

Self::recognize_expr_arg(&expr_or_spread.expr)
}

fn is_global_object(&self, expr: &Expr) -> bool {
if let Expr::Ident(ident) = expr {
return ident.sym == "Object" && ident.span.ctxt == self.unresolved_ctxt;
}

false
}

fn is_method_assign(&self, mem_prop: &MemberProp) -> bool {
match mem_prop {
MemberProp::Ident(ident) => ident.sym == "assign",
MemberProp::Computed(computed) => match computed.expr.as_ref() {
Expr::Lit(Lit::Str(lit_str)) => lit_str.value == "assign",
Expr::Tpl(tlp) => {
tlp.exprs.is_empty() && tlp.quasis.len() == 1 && tlp.quasis[0].raw == "assign"
}
_ => false,
},
_ => false,
}
}

fn is_object_assign_call(&self, call_expr: &CallExpr) -> bool {
if let Some(callee) = call_expr.callee.as_expr() {
if let Some(MemberExpr { obj, prop, .. }) = callee.as_member() {
return self.is_global_object(obj.as_expr()) && self.is_method_assign(prop);
}
}

false
}

fn check(&mut self, call_expr: &CallExpr) {
if call_expr.args.is_empty() {
return;
}

if !self.is_object_assign_call(call_expr) {
return;
}

let arg: ArgType = Self::recognize_arg(&call_expr.args[0]);

match (call_expr.args.len(), &arg) {
(1, ArgType::EmptyLiteralObject)
| (1, ArgType::LiteralObjectWithFields)
| (1, ArgType::LiteralObjectWithGetterOrSetter) => {
self.emit_report(call_expr.span, USE_LITERAL_MESSAGE);
}
(_, ArgType::EmptyLiteralObject) | (_, ArgType::LiteralObjectWithFields) => {
let has_spread_or_getter_setter = call_expr.args[1..].iter().any(|prop| {
matches!(
Self::recognize_arg(prop),
ArgType::Spread | ArgType::LiteralObjectWithGetterOrSetter
)
});

if has_spread_or_getter_setter {
return;
}

self.emit_report(call_expr.span, USE_SPREAD_MESSAGE);
}
_ => {}
}
}
}

impl Visit for PreferObjectSpread {
noop_visit_type!();

fn visit_call_expr(&mut self, call_expr: &CallExpr) {
self.check(call_expr);

call_expr.visit_children_with(self);
}
}

0 comments on commit aa9297b

Please sign in to comment.