From 4975440e1b6e87d0a164d6b46fdb945688ee2f64 Mon Sep 17 00:00:00 2001
From: Boshen <boshenc@gmail.com>
Date: Mon, 30 Oct 2023 17:23:11 +0800
Subject: [PATCH] fix(linter/no_render_return_value): fix false positive when
 nested inside an arrow expression (#1109)

---
 .../src/rules/react/no_render_return_value.rs | 20 +++++++++++--------
 .../src/snapshots/no_render_return_value.snap |  7 +++++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/crates/oxc_linter/src/rules/react/no_render_return_value.rs b/crates/oxc_linter/src/rules/react/no_render_return_value.rs
index f6c039a62..b50ea1049 100644
--- a/crates/oxc_linter/src/rules/react/no_render_return_value.rs
+++ b/crates/oxc_linter/src/rules/react/no_render_return_value.rs
@@ -11,7 +11,7 @@ use crate::{context::LintContext, rule::Rule, AstNode};
 
 #[derive(Debug, Error, Diagnostic)]
 #[error("eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.")]
-#[diagnostic(severity(warning))]
+#[diagnostic(severity(warning), help("Using the return value is a legacy feature."))]
 struct NoRenderReturnValueDiagnostic(#[label] pub Span);
 
 #[derive(Debug, Default, Clone)]
@@ -64,16 +64,18 @@ impl Rule for NoRenderReturnValue {
                             .contains(ScopeFlags::Arrow);
 
                         if is_arrow_function {
-                            ctx.nodes().ancestors(parent_node.id()).skip(1).find(|node_id| {
-                                let parent_node = ctx.nodes().get_node(*node_id);
-                                matches!(parent_node.kind(), AstKind::ArrowExpression(_))
-                                    .then(|| {
+                            for node_id in ctx.nodes().ancestors(parent_node.id()).skip(1) {
+                                let node = ctx.nodes().get_node(node_id);
+                                if let AstKind::ArrowExpression(e) = node.kind() {
+                                    if e.expression {
                                         ctx.diagnostic(NoRenderReturnValueDiagnostic(
                                             ident.span.merge(&property_span),
                                         ));
-                                    })
-                                    .is_some()
-                            });
+                                    } else {
+                                        break;
+                                    }
+                                }
+                            }
                         }
                     }
                 }
@@ -101,6 +103,8 @@ fn test() {
         ("var foo = React.render(<div />, root);", None),
         ("var foo = render(<div />, root)", None),
         ("var foo = ReactDom.renderder(<div />, root)", None),
+        ("var foo = ReactDom.renderder(<div />, root)", None),
+        ("export const foo = () => ({ destroy: ({ dom }) => { ReactDOM.unmountComponentAtNode(dom); } });", None),
     ];
 
     let fail = vec![
diff --git a/crates/oxc_linter/src/snapshots/no_render_return_value.snap b/crates/oxc_linter/src/snapshots/no_render_return_value.snap
index 802adfd08..ec4a0691a 100644
--- a/crates/oxc_linter/src/snapshots/no_render_return_value.snap
+++ b/crates/oxc_linter/src/snapshots/no_render_return_value.snap
@@ -7,6 +7,7 @@ expression: no_render_return_value
  1 │ var Hello = ReactDOM.render(<div />, document.body);
    ·             ───────────────
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:2:1]
@@ -15,6 +16,7 @@ expression: no_render_return_value
    ·                             ───────────────
  4 │                     };
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:2:1]
@@ -23,29 +25,34 @@ expression: no_render_return_value
    ·                              ───────────────
  4 │                     }
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:1:1]
  1 │ var render = (a, b) => ReactDOM.render(a, b)
    ·                        ───────────────
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:1:1]
  1 │ this.o = ReactDOM.render(<div />, document.body);
    ·          ───────────────
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:1:1]
  1 │ var v; v = ReactDOM.render(<div />, document.body);
    ·            ───────────────
    ╰────
+  help: Using the return value is a legacy feature.
 
   ⚠ eslint-plugin-react(no-render-return-value): Do not depend on the return value from ReactDOM.render.
    ╭─[no_render_return_value.tsx:1:1]
  1 │ var inst = ReactDOM.render(<div />, document.body);
    ·            ───────────────
    ╰────
+  help: Using the return value is a legacy feature.