Skip to content

Commit

Permalink
feat(linter): eslint-plugin-react jsx-props-no-spread-multi (#4866)
Browse files Browse the repository at this point in the history
  • Loading branch information
keita-hino and DonIsaac authored Aug 14, 2024
1 parent 117ae36 commit 93ae1c7
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 0 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 @@ -227,6 +227,7 @@ mod react {
pub mod jsx_no_target_blank;
pub mod jsx_no_undef;
pub mod jsx_no_useless_fragment;
pub mod jsx_props_no_spread_multi;
pub mod no_children_prop;
pub mod no_danger;
pub mod no_direct_mutation_state;
Expand Down Expand Up @@ -733,6 +734,7 @@ oxc_macros::declare_all_lint_rules! {
react::jsx_no_comment_textnodes,
react::jsx_no_duplicate_props,
react::jsx_no_useless_fragment,
react::jsx_props_no_spread_multi,
react::jsx_no_undef,
react::react_in_jsx_scope,
react::no_children_prop,
Expand Down
105 changes: 105 additions & 0 deletions crates/oxc_linter/src/rules/react/jsx_props_no_spread_multi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use rustc_hash::FxHashMap;

use oxc_ast::{ast::JSXAttributeItem, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{Atom, Span};

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

fn jsx_props_no_spread_multi_diagnostic(spans: Vec<Span>, prop_name: &str) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow JSX prop spreading the same identifier multiple times.")
.with_help(format!("Prop '{prop_name}' is spread multiple times."))
.with_labels(spans)
}

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

declare_oxc_lint!(
/// ### What it does
/// Enforces that any unique expression is only spread once.
///
/// ### Why is this bad?
/// Generally spreading the same expression twice is an indicator of a mistake since any attribute between the spreads may be overridden when the intent was not to.
/// Even when that is not the case this will lead to unnecessary computations being performed.
///
/// ### Example
/// ```jsx
/// // Bad
/// <App {...props} myAttr="1" {...props} />
///
/// // Good
/// <App myAttr="1" {...props} />
/// <App {...props} myAttr="1" />
/// ```
JsxPropsNoSpreadMulti,
correctness,
pending // TODO: add auto-fix to remove the first spread. Removing the second one would change program behavior.
);

impl Rule for JsxPropsNoSpreadMulti {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::JSXOpeningElement(jsx_opening_el) = node.kind() {
let spread_attrs =
jsx_opening_el.attributes.iter().filter_map(JSXAttributeItem::as_spread);

let mut identifier_names: FxHashMap<&Atom, Span> = FxHashMap::default();
let mut duplicate_spreads: FxHashMap<&Atom, Vec<Span>> = FxHashMap::default();

for spread_attr in spread_attrs {
if let Some(identifier_name) =
spread_attr.argument.get_identifier_reference().map(|arg| &arg.name)
{
identifier_names
.entry(identifier_name)
.and_modify(|first_span| {
duplicate_spreads
.entry(identifier_name)
.or_insert_with(|| vec![*first_span])
.push(spread_attr.span);
})
.or_insert(spread_attr.span);
}
}

for (identifier_name, spans) in duplicate_spreads {
ctx.diagnostic(jsx_props_no_spread_multi_diagnostic(spans, identifier_name));
}
}
}
}

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

let pass = vec![
"
const a = {};
<App {...a} />
",
"
const a = {};
const b = {};
<App {...a} {...b} />
",
];

let fail = vec![
"
const props = {};
<App {...props} {...props} />
",
r#"
const props = {};
<div {...props} a="a" {...props} />
"#,
"
const props = {};
<div {...props} {...props} {...props} />
",
];

Tester::new(JsxPropsNoSpreadMulti::NAME, pass, fail).test_and_snapshot();
}
29 changes: 29 additions & 0 deletions crates/oxc_linter/src/snapshots/jsx_props_no_spread_multi.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint-plugin-react(jsx-props-no-spread-multi): Disallow JSX prop spreading the same identifier multiple times.
╭─[jsx_props_no_spread_multi.tsx:3:16]
2const props = {};
3<App {...props} {...props} />
· ────────── ──────────
4
╰────
help: Prop 'props' is spread multiple times.

eslint-plugin-react(jsx-props-no-spread-multi): Disallow JSX prop spreading the same identifier multiple times.
╭─[jsx_props_no_spread_multi.tsx:3:16]
2const props = {};
3<div {...props} a="a" {...props} />
· ────────── ──────────
4
╰────
help: Prop 'props' is spread multiple times.

eslint-plugin-react(jsx-props-no-spread-multi): Disallow JSX prop spreading the same identifier multiple times.
╭─[jsx_props_no_spread_multi.tsx:3:16]
2const props = {};
3<div {...props} {...props} {...props} />
· ────────── ────────── ──────────
4
╰────
help: Prop 'props' is spread multiple times.

0 comments on commit 93ae1c7

Please sign in to comment.