feat(linter/react-perf): handle new objects and arrays in prop assignment patterns

crates/oxc_ast/src/ast_impl/js.rs

@@ -304,6 +304,11 @@ impl<'a> IdentifierReference<'a> {
             reference_flag: ReferenceFlag::Read,
         }
     }
+
+    #[inline]
+    pub fn reference_id(&self) -> Option<ReferenceId> {
+        self.reference_id.get()
+    }
 }
 
 impl<'a> Hash for BindingIdentifier<'a> {

crates/oxc_linter/src/rules/react_perf/jsx_no_jsx_as_prop.rs

@@ -1,18 +1,8 @@
-use oxc_ast::{
-    ast::{Expression, JSXAttributeValue, JSXElement},
-    AstKind,
-};
-use oxc_diagnostics::OxcDiagnostic;
+use crate::utils::ReactPerfRule;
+use oxc_ast::{ast::Expression, AstKind};
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
-
-use crate::{context::LintContext, rule::Rule, utils::get_prop_value, AstNode};
-
-fn jsx_no_jsx_as_prop_diagnostic(span0: Span) -> OxcDiagnostic {
-    OxcDiagnostic::warn("JSX attribute values should not contain other JSX.")
-        .with_help(r"simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).")
-        .with_label(span0)
-}
+use oxc_semantic::SymbolId;
+use oxc_span::{GetSpan, Span};
 
 #[derive(Debug, Default, Clone)]
 pub struct JsxNoJsxAsProp;
@@ -36,34 +26,23 @@ declare_oxc_lint!(
     perf
 );
 
-impl Rule for JsxNoJsxAsProp {
-    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
-        if node.scope_id() == ctx.scopes().root_scope_id() {
-            return;
-        }
-        if let AstKind::JSXElement(jsx_elem) = node.kind() {
-            check_jsx_element(jsx_elem, ctx);
-        }
-    }
+impl ReactPerfRule for JsxNoJsxAsProp {
+    const MESSAGE: &'static str = "JSX attribute values should not contain other JSX.";
 
-    fn should_run(&self, ctx: &LintContext) -> bool {
-        ctx.source_type().is_jsx()
+    fn check_for_violation_on_expr(&self, expr: &Expression<'_>) -> Option<Span> {
+        check_expression(expr)
     }
-}
 
-fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
-    for item in &jsx_elem.opening_element.attributes {
-        match get_prop_value(item) {
-            None => return,
-            Some(JSXAttributeValue::ExpressionContainer(container)) => {
-                if let Some(expr) = container.expression.as_expression() {
-                    if let Some(span) = check_expression(expr) {
-                        ctx.diagnostic(jsx_no_jsx_as_prop_diagnostic(span));
-                    }
-                }
-            }
-            _ => {}
+    fn check_for_violation_on_ast_kind(
+        &self,
+        kind: &AstKind<'_>,
+        _symbol_id: SymbolId,
+    ) -> Option<(/* decl */ Span, /* init */ Option<Span>)> {
+        let AstKind::VariableDeclarator(decl) = kind else {
+            return None;
         };
+        let init_span = decl.init.as_ref().and_then(check_expression)?;
+        Some((decl.id.span(), Some(init_span)))
     }
 }
 
@@ -91,13 +70,15 @@ fn test() {
         r"<Item jsx={this.props.jsx || <SubItem />} />",
         r"<Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />",
         r"<Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />",
+        r"const Icon = <svg />; const Foo = () => (<IconButton icon={Icon} />)",
     ];
 
     let fail = vec![
         r"const Foo = () => (<Item jsx={<SubItem />} />)",
         r"const Foo = () => (<Item jsx={this.props.jsx || <SubItem />} />)",
         r"const Foo = () => (<Item jsx={this.props.jsx ? this.props.jsx : <SubItem />} />)",
         r"const Foo = () => (<Item jsx={this.props.jsx || (this.props.component ? this.props.component : <SubItem />)} />)",
+        r"const Foo = () => { const Icon = <svg />; return (<IconButton icon={Icon} />) }",
     ];
 
     Tester::new(JsxNoJsxAsProp::NAME, pass, fail).with_react_perf_plugin(true).test_and_snapshot();

crates/oxc_linter/src/rules/react_perf/jsx_no_new_array_as_prop.rs

@@ -1,24 +1,13 @@
-use oxc_ast::{
-    ast::{Expression, JSXAttributeValue, JSXElement},
-    AstKind,
-};
-use oxc_diagnostics::OxcDiagnostic;
+use oxc_ast::{ast::Expression, AstKind};
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_semantic::SymbolId;
+use oxc_span::{GetSpan, Span};
 
 use crate::{
-    context::LintContext,
-    rule::Rule,
-    utils::{get_prop_value, is_constructor_matching_name},
-    AstNode,
+    ast_util::is_method_call,
+    utils::{find_initialized_binding, is_constructor_matching_name, ReactPerfRule},
 };
 
-fn jsx_no_new_array_as_prop_diagnostic(span0: Span) -> OxcDiagnostic {
-    OxcDiagnostic::warn("JSX attribute values should not contain Arrays created in the same scope.")
-        .with_help(r"simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).")
-        .with_label(span0)
-}
-
 #[derive(Debug, Default, Clone)]
 pub struct JsxNoNewArrayAsProp;
 
@@ -44,42 +33,49 @@ declare_oxc_lint!(
     perf
 );
 
-impl Rule for JsxNoNewArrayAsProp {
-    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
-        if node.scope_id() == ctx.scopes().root_scope_id() {
-            return;
-        }
-        if let AstKind::JSXElement(jsx_elem) = node.kind() {
-            check_jsx_element(jsx_elem, ctx);
-        }
-    }
+impl ReactPerfRule for JsxNoNewArrayAsProp {
+    const MESSAGE: &'static str =
+        "JSX attribute values should not contain Arrays created in the same scope.";
 
-    fn should_run(&self, ctx: &LintContext) -> bool {
-        ctx.source_type().is_jsx()
+    fn check_for_violation_on_expr(&self, expr: &Expression<'_>) -> Option<Span> {
+        check_expression(expr)
     }
-}
 
-fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
-    for item in &jsx_elem.opening_element.attributes {
-        match get_prop_value(item) {
-            None => return,
-            Some(JSXAttributeValue::ExpressionContainer(container)) => {
-                if let Some(expr) = container.expression.as_expression() {
-                    if let Some(span) = check_expression(expr) {
-                        ctx.diagnostic(jsx_no_new_array_as_prop_diagnostic(span));
-                    }
+    fn check_for_violation_on_ast_kind(
+        &self,
+        kind: &AstKind<'_>,
+        symbol_id: SymbolId,
+    ) -> Option<(/* decl */ Span, /* init */ Option<Span>)> {
+        match kind {
+            AstKind::VariableDeclarator(decl) => {
+                if let Some(init_span) = decl.init.as_ref().and_then(check_expression) {
+                    return Some((decl.id.span(), Some(init_span)));
                 }
+                None
+            }
+            AstKind::FormalParameter(param) => {
+                let (id, init) = find_initialized_binding(&param.pattern, symbol_id)?;
+                let init_span = check_expression(init)?;
+                Some((id.span(), Some(init_span)))
             }
-            _ => {}
-        };
+            _ => None,
+        }
     }
 }
 
 fn check_expression(expr: &Expression) -> Option<Span> {
     match expr.without_parenthesized() {
         Expression::ArrayExpression(expr) => Some(expr.span),
         Expression::CallExpression(expr) => {
-            if is_constructor_matching_name(&expr.callee, "Array") {
+            if is_constructor_matching_name(&expr.callee, "Array")
+                || is_method_call(
+                    expr.as_ref(),
+                    None,
+                    Some(&["concat", "map", "filter"]),
+                    Some(1),
+                    Some(1),
+                )
+            {
                 Some(expr.span)
             } else {
                 None
@@ -108,22 +104,29 @@ fn test() {
 
     let pass = vec![
         r"<Item list={this.props.list} />",
-        r"const Foo = () => <Item list={this.props.list} />",
         r"<Item list={[]} />",
         r"<Item list={new Array()} />",
         r"<Item list={Array()} />",
         r"<Item list={this.props.list || []} />",
         r"<Item list={this.props.list ? this.props.list : []} />",
         r"<Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />",
+        r"const Foo = () => <Item list={this.props.list} />",
+        r"const x = []; const Foo = () => <Item list={x} />",
+        r"const DEFAULT_X = []; const Foo = ({ x = DEFAULT_X }) => <Item list={x} />",
     ];
 
     let fail = vec![
         r"const Foo = () => (<Item list={[]} />)",
         r"const Foo = () => (<Item list={new Array()} />)",
         r"const Foo = () => (<Item list={Array()} />)",
+        r"const Foo = () => (<Item list={arr1.concat(arr2)} />)",
+        r"const Foo = () => (<Item list={arr1.filter(x => x > 0)} />)",
+        r"const Foo = () => (<Item list={arr1.map(x => x * x)} />)",
         r"const Foo = () => (<Item list={this.props.list || []} />)",
         r"const Foo = () => (<Item list={this.props.list ? this.props.list : []} />)",
         r"const Foo = () => (<Item list={this.props.list || (this.props.arr ? this.props.arr : [])} />)",
+        r"const Foo = () => { let x = []; return <Item list={x} /> }",
+        r"const Foo = ({ x = [] }) => <Item list={x} />",
     ];
 
     Tester::new(JsxNoNewArrayAsProp::NAME, pass, fail)

crates/oxc_linter/src/rules/react_perf/jsx_no_new_function_as_prop.rs

@@ -1,23 +1,12 @@
 use oxc_ast::{
-    ast::{Expression, JSXAttributeValue, JSXElement, MemberExpression},
+    ast::{Expression, MemberExpression},
     AstKind,
 };
-use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_semantic::SymbolId;
+use oxc_span::{GetSpan, Span};
 
-use crate::{
-    context::LintContext,
-    rule::Rule,
-    utils::{get_prop_value, is_constructor_matching_name},
-    AstNode,
-};
-
-fn jsx_no_new_function_as_prop_diagnostic(span0: Span) -> OxcDiagnostic {
-    OxcDiagnostic::warn("JSX attribute values should not contain functions created in the same scope.")
-        .with_help(r"simplify props or memoize props in the parent component (https://react.dev/reference/react/memo#my-component-rerenders-when-a-prop-is-an-object-or-array).")
-        .with_label(span0)
-}
+use crate::utils::{is_constructor_matching_name, ReactPerfRule};
 
 #[derive(Debug, Default, Clone)]
 pub struct JsxNoNewFunctionAsProp;
@@ -39,34 +28,31 @@ declare_oxc_lint!(
     perf
 );
 
-impl Rule for JsxNoNewFunctionAsProp {
-    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
-        if node.scope_id() == ctx.scopes().root_scope_id() {
-            return;
-        }
-        if let AstKind::JSXElement(jsx_elem) = node.kind() {
-            check_jsx_element(jsx_elem, ctx);
-        }
-    }
+impl ReactPerfRule for JsxNoNewFunctionAsProp {
+    const MESSAGE: &'static str =
+        "JSX attribute values should not contain functions created in the same scope.";
 
-    fn should_run(&self, ctx: &LintContext) -> bool {
-        ctx.source_type().is_jsx()
+    fn check_for_violation_on_expr(&self, expr: &Expression<'_>) -> Option<Span> {
+        check_expression(expr)
     }
-}
 
-fn check_jsx_element<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
-    for item in &jsx_elem.opening_element.attributes {
-        match get_prop_value(item) {
-            None => return,
-            Some(JSXAttributeValue::ExpressionContainer(container)) => {
-                if let Some(expr) = container.expression.as_expression() {
-                    if let Some(span) = check_expression(expr) {
-                        ctx.diagnostic(jsx_no_new_function_as_prop_diagnostic(span));
-                    }
-                }
+    fn check_for_violation_on_ast_kind(
+        &self,
+        kind: &AstKind<'_>,
+        _symbol_id: SymbolId,
+    ) -> Option<(/* decl */ Span, /* init */ Option<Span>)> {
+        match kind {
+            AstKind::VariableDeclarator(decl)
+                if decl.init.as_ref().and_then(check_expression).is_some() =>
+            {
+                // don't report init span, b/c thats usually an arrow
+                // function expression which gets quite large. It also
+                // doesn't add any value.
+                Some((decl.id.span(), None))
             }
-            _ => {}
-        };
+            AstKind::Function(f) => Some((f.id.as_ref().map_or(f.span, GetSpan::span), None)),
+            _ => None,
+        }
     }
 }
 
@@ -131,6 +117,24 @@ fn test() {
         r"<Item callback={this.props.callback ? this.props.callback : function() {}} />",
         r"<Item prop={this.props.callback || this.props.callback ? this.props.callback : function(){}} />",
         r"<Item prop={this.props.callback || (this.props.cb ? this.props.cb : function(){})} />",
+        r"
+        import { FC, useCallback } from 'react';
+        export const Foo: FC = props => {
+            const onClick = useCallback(
+                e => { props.onClick?.(e) },
+                [props.onClick]
+            );
+            return <button onClick={onClick} />
+        }",
+        r"
+        import React from 'react'
+        function onClick(e: React.MouseEvent) {
+            window.location.navigate(e.target.href)
+        }
+        export default function Foo() {
+            return <a onClick={onClick} />
+        }
+        ",
     ];
 
     let fail = vec![
@@ -143,6 +147,35 @@ fn test() {
         r"const Foo = () => (<Item callback={this.props.callback ? this.props.callback : function() {}} />)",
         r"const Foo = () => (<Item prop={this.props.callback || this.props.callback ? this.props.callback : function(){}} />)",
         r"const Foo = () => (<Item prop={this.props.callback || (this.props.cb ? this.props.cb : function(){})} />)",
+        r"
+        const Foo = ({ onClick }) => {
+            const _onClick = onClick.bind(this)
+            return <button onClick={_onClick} />
+        }",
+        r"
+        const Foo = () => {
+            function onClick(e) {
+                window.location.navigate(e.target.href)
+            }
+            return <a onClick={onClick} />
+        }
+        ",
+        r"
+        const Foo = () => {
+            const onClick = (e) => {
+                window.location.navigate(e.target.href)
+            }
+            return <a onClick={onClick} />
+        }
+        ",
+        r"
+        const Foo = () => {
+            const onClick = function (e) {
+                window.location.navigate(e.target.href)
+            }
+            return <a onClick={onClick} />
+        }
+        ",
     ];
 
     Tester::new(JsxNoNewFunctionAsProp::NAME, pass, fail)