diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index 46a7390ee..519c759be 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -2,12 +2,12 @@ use oxc_ast::{ast::Expression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_semantic::AstNodeId; +use oxc_semantic::{ + pg::neighbors_filtered_by_edge_weight, AssignmentValue, BasicBlockElement, EdgeType, Register, +}; use oxc_span::{GetSpan, Span}; -use rustc_hash::FxHashSet; use crate::{ - ast_util::get_enclosing_function, context::LintContext, rule::Rule, utils::{is_es5_component, is_es6_component}, @@ -49,51 +49,85 @@ declare_oxc_lint!( ); impl Rule for RequireRenderReturn { - fn run_once(&self, ctx: &LintContext) { - let mut render_fn_set: FxHashSet = FxHashSet::default(); - - for node in ctx.nodes().iter() { - if is_render_fn(node) { - render_fn_set.insert(node.id()); - } - - let is_return_detected = match node.kind() { - AstKind::ReturnStatement(_) => true, - AstKind::ArrowFunctionExpression(arrow) => arrow.expression, - _ => false, - }; - - if is_return_detected { - let Some(function) = get_enclosing_function(node, ctx) else { - continue; - }; - let Some(fn_parent_node) = ctx.nodes().parent_node(function.id()) else { - continue; - }; - - if matches!( - fn_parent_node.kind(), - AstKind::MethodDefinition(_) - | AstKind::PropertyDefinition(_) - | AstKind::ObjectProperty(_) - ) { - render_fn_set.remove(&fn_parent_node.id()); - } - } + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if !matches!(node.kind(), AstKind::ArrowFunctionExpression(_) | AstKind::Function(_)) { + return; + } + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return; + }; + if !is_render_fn(parent) { + return; + } + if !is_in_es6_component(parent, ctx) && !is_in_es5_component(parent, ctx) { + return; } - for node_id in render_fn_set { - let render_fn_node = ctx.nodes().get_node(node_id); - if is_in_es6_component(render_fn_node, ctx) { - diagnostic_on_render_fn(node_id, ctx); - return; - } else if is_in_es5_component(render_fn_node, ctx) { - diagnostic_on_render_fn(node_id, ctx); - } + if !contains_return_statement(node, ctx) { + match parent.kind() { + AstKind::MethodDefinition(method) => { + ctx.diagnostic(require_render_return_diagnostic(method.key.span())); + } + AstKind::PropertyDefinition(property) => { + ctx.diagnostic(require_render_return_diagnostic(property.key.span())); + } + AstKind::ObjectProperty(property) => { + ctx.diagnostic(require_render_return_diagnostic(property.key.span())); + } + _ => {} + }; } } } +#[derive(Clone, Copy, Debug, Default, PartialEq)] +enum FoundReturn { + #[default] + No, + Yes, +} + +const KEEP_WALKING_ON_THIS_PATH: bool = true; +const STOP_WALKING_ON_THIS_PATH: bool = false; + +fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { + let cfg = ctx.semantic().cfg(); + let state = neighbors_filtered_by_edge_weight( + &cfg.graph, + node.cfg_ix(), + &|edge| match edge { + // We only care about normal edges having a return statement. + EdgeType::Normal => None, + // For these two type, we flag it as not found. + EdgeType::NewFunction | EdgeType::Backedge => Some(FoundReturn::No), + }, + &mut |basic_block_id, _state_going_into_this_rule| { + // If its an arrow function with an expression, marked as founded and stop walking. + if let AstKind::ArrowFunctionExpression(arrow_expr) = node.kind() { + if arrow_expr.expression { + return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH); + } + } + + for entry in cfg.basic_block_by_index(*basic_block_id) { + if let BasicBlockElement::Assignment(to_reg, val) = entry { + if matches!(to_reg, Register::Return) + && matches!(val, AssignmentValue::NotImplicitUndefined) + { + return (FoundReturn::Yes, STOP_WALKING_ON_THIS_PATH); + } + } else { + // We don't care about other types of instructions. + } + } + + (FoundReturn::No, KEEP_WALKING_ON_THIS_PATH) + }, + ); + + state.iter().any(|&state| state == FoundReturn::Yes) +} + const RENDER_METHOD_NAME: &str = "render"; fn is_render_fn(node: &AstNode) -> bool { @@ -122,19 +156,6 @@ fn is_render_fn(node: &AstNode) -> bool { false } -fn diagnostic_on_render_fn(node_id: AstNodeId, ctx: &LintContext) { - let span = match ctx.nodes().get_node(node_id).kind() { - AstKind::MethodDefinition(method) => Some(method.key.span()), - AstKind::PropertyDefinition(property) => Some(property.key.span()), - AstKind::ObjectProperty(property) => Some(property.key.span()), - _ => None, - }; - - if let Some(span) = span { - ctx.diagnostic(require_render_return_diagnostic(span)); - } -} - fn is_in_es5_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool { let Some(ancestors_0) = ctx.nodes().parent_node(node.id()) else { return false }; if !matches!(ancestors_0.kind(), AstKind::ObjectExpression(_)) { @@ -246,6 +267,15 @@ fn test() { render } ", + r" + class Foo extends Component { + render = () => { + if (true) { + return
Hello
; + } + } + } + ", ]; let fail = vec![ @@ -276,6 +306,22 @@ fn test() { } } ", + r" + class Hello extends React.Component { + render() { + function foo() { + return
Hello {this.props.name}
; + } + } + } + ", + r" + class Hello extends React.Component { + render() { + return + } + } + ", ]; Tester::new(RequireRenderReturn::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/require_render_return.snap b/crates/oxc_linter/src/snapshots/require_render_return.snap index 65cd1f1ab..76e781be2 100644 --- a/crates/oxc_linter/src/snapshots/require_render_return.snap +++ b/crates/oxc_linter/src/snapshots/require_render_return.snap @@ -37,3 +37,21 @@ expression: require_render_return 4 │
Hello {this.props.name}
╰──── help: When writing the `render` method in a component it is easy to forget to return the JSX content. This rule will warn if the return statement is missing. + + ⚠ eslint-plugin-react(require-render-return): Your render method should have a return statement + ╭─[require_render_return.tsx:3:15] + 2 │ class Hello extends React.Component { + 3 │ render() { + · ────── + 4 │ function foo() { + ╰──── + help: When writing the `render` method in a component it is easy to forget to return the JSX content. This rule will warn if the return statement is missing. + + ⚠ eslint-plugin-react(require-render-return): Your render method should have a return statement + ╭─[require_render_return.tsx:3:15] + 2 │ class Hello extends React.Component { + 3 │ render() { + · ────── + 4 │ return + ╰──── + help: When writing the `render` method in a component it is easy to forget to return the JSX content. This rule will warn if the return statement is missing. diff --git a/tasks/transform_conformance/Cargo.toml b/tasks/transform_conformance/Cargo.toml index e303a140f..22beb8121 100644 --- a/tasks/transform_conformance/Cargo.toml +++ b/tasks/transform_conformance/Cargo.toml @@ -29,6 +29,6 @@ oxc_transformer = { workspace = true } oxc_tasks_common = { workspace = true } oxc_diagnostics = { workspace = true } -walkdir = { workspace = true } -pico-args = { workspace = true } -indexmap = { workspace = true } +walkdir = { workspace = true } +pico-args = { workspace = true } +indexmap = { workspace = true }