diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index cc26afb444cc1..521e02bce716a 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -2328,6 +2328,13 @@ impl RuleRunner for crate::rules::react::no_namespace::NoNamespace { const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner + for crate::rules::react::no_redundant_should_component_update::NoRedundantShouldComponentUpdate +{ + const NODE_TYPES: Option<&AstTypesBitset> = None; + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; +} + impl RuleRunner for crate::rules::react::no_render_return_value::NoRenderReturnValue { const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[AstType::CallExpression])); diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 7e80f946cb25b..dbfeb75e2ac3e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -370,6 +370,7 @@ pub(crate) mod react { pub mod no_find_dom_node; pub mod no_is_mounted; pub mod no_namespace; + pub mod no_redundant_should_component_update; pub mod no_render_return_value; pub mod no_set_state; pub mod no_string_refs; @@ -1047,6 +1048,7 @@ oxc_macros::declare_all_lint_rules! { react::no_direct_mutation_state, react::no_find_dom_node, react::no_is_mounted, + react::no_redundant_should_component_update, react::no_render_return_value, react::no_set_state, react::no_string_refs, diff --git a/crates/oxc_linter/src/rules/react/no_redundant_should_component_update.rs b/crates/oxc_linter/src/rules/react/no_redundant_should_component_update.rs new file mode 100644 index 0000000000000..930c41c87f00e --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_redundant_should_component_update.rs @@ -0,0 +1,243 @@ +use oxc_ast::{ + AstKind, + ast::{Class, Expression}, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{AstNode, context::LintContext, rule::Rule}; + +fn no_redundant_should_component_update_diagnostic( + span: Span, + component_name: &str, +) -> OxcDiagnostic { + let msg = format!( + "{component_name} does not need shouldComponentUpdate when extending React.PureComponent." + ); + OxcDiagnostic::warn(msg).with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoRedundantShouldComponentUpdate; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow usage of shouldComponentUpdate when extending React.PureComponent + /// + /// ### Why is this bad? + /// + /// React.PureComponent automatically implements shouldComponentUpdate with a shallow prop and state comparison. + /// Defining shouldComponentUpdate in a class that extends PureComponent is redundant and defeats the purpose + /// of using PureComponent. If you need custom comparison logic, extend React.Component instead. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```jsx + /// class Foo extends React.PureComponent { + /// shouldComponentUpdate() { + /// // do check + /// } + /// + /// render() { + /// return
Radical!
+ /// } + ///} + /// + ///function Bar() { + /// return class Baz extends React.PureComponent { + /// shouldComponentUpdate() { + /// // do check + /// } + /// + /// render() { + /// return
Groovy!
+ /// } + /// } + ///} + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```jsx + /// class Foo extends React.Component { + /// shouldComponentUpdate() { + /// // do check + /// } + /// + /// render() { + /// return
Radical!
+ /// } + /// } + /// + /// function Bar() { + /// return class Baz extends React.Component { + /// shouldComponentUpdate() { + /// // do check + /// } + /// + /// render() { + /// return
Groovy!
+ /// } + /// } + /// } + /// + /// class Qux extends React.PureComponent { + /// render() { + /// return
Tubular!
+ /// } + /// } + /// ``` + NoRedundantShouldComponentUpdate, + react, + style, +); + +impl Rule for NoRedundantShouldComponentUpdate { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let Some(class_node) = is_react_pure_component(node) else { + return; + }; + + if has_should_component_update(class_node) { + let component_name = class_node + .name() + .or_else(|| { + // e.g. var Foo = class extends PureComponent + let parent = ctx.nodes().parent_node(node.id()); + if let AstKind::VariableDeclarator(declarator) = parent.kind() { + declarator.id.get_identifier_name() + } else { + None + } + }) + .map_or("", |name| name.as_str()); + + ctx.diagnostic(no_redundant_should_component_update_diagnostic( + class_node.span, + component_name, + )); + } + } +} + +fn has_should_component_update(class: &Class<'_>) -> bool { + class + .body + .body + .iter() + .any(|prop| prop.static_name().is_some_and(|name| name == "shouldComponentUpdate")) +} + +/// Checks if class is React.PureComponent and returns this class if true +fn is_react_pure_component<'a>(node: &'a AstNode) -> Option<&'a Class<'a>> { + let AstKind::Class(class_expr) = node.kind() else { + return None; + }; + if let Some(super_class) = &class_expr.super_class { + if let Some(member_expr) = super_class.as_member_expression() + && let Expression::Identifier(ident) = member_expr.object() + { + let is_pure = ident.name == "React" + && member_expr.static_property_name().is_some_and(|name| name == "PureComponent"); + + return if is_pure { Some(class_expr) } else { None }; + } + + if let Some(ident_reference) = super_class.get_identifier_reference() { + let is_pure = ident_reference.name == "PureComponent"; + return if is_pure { Some(class_expr) } else { None }; + } + } + + None +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + " + class Foo extends React.Component { + shouldComponentUpdate() { + return true; + } + } + ", + " + class Foo extends React.Component { + shouldComponentUpdate = () => { + return true; + } + } + ", + " + function Foo() { + return class Bar extends React.Component { + shouldComponentUpdate() { + return true; + } + }; + } + ", + ]; + + let fail = vec![ + " + class Foo extends React.PureComponent { + shouldComponentUpdate() { + return true; + } + } + ", + " + class Foo extends PureComponent { + shouldComponentUpdate() { + return true; + } + } + ", + " + class Foo extends React.PureComponent { + shouldComponentUpdate = () => { + return true; + } + } + ", + " + function Foo() { + return class Bar extends React.PureComponent { + shouldComponentUpdate() { + return true; + } + }; + } + ", + " + function Foo() { + return class Bar extends PureComponent { + shouldComponentUpdate() { + return true; + } + }; + } + ", + " + var Foo = class extends PureComponent { + shouldComponentUpdate() { + return true; + } + } + ", + ]; + + Tester::new( + NoRedundantShouldComponentUpdate::NAME, + NoRedundantShouldComponentUpdate::PLUGIN, + pass, + fail, + ) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/react_no_redundant_should_component_update.snap b/crates/oxc_linter/src/snapshots/react_no_redundant_should_component_update.snap new file mode 100644 index 0000000000000..634a1d42c50fa --- /dev/null +++ b/crates/oxc_linter/src/snapshots/react_no_redundant_should_component_update.snap @@ -0,0 +1,68 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:2:12] + 1 │ + 2 │ ╭─▶ class Foo extends React.PureComponent { + 3 │ │ shouldComponentUpdate() { + 4 │ │ return true; + 5 │ │ } + 6 │ ╰─▶ } + 7 │ + ╰──── + + ⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:2:12] + 1 │ + 2 │ ╭─▶ class Foo extends PureComponent { + 3 │ │ shouldComponentUpdate() { + 4 │ │ return true; + 5 │ │ } + 6 │ ╰─▶ } + 7 │ + ╰──── + + ⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:2:12] + 1 │ + 2 │ ╭─▶ class Foo extends React.PureComponent { + 3 │ │ shouldComponentUpdate = () => { + 4 │ │ return true; + 5 │ │ } + 6 │ ╰─▶ } + 7 │ + ╰──── + + ⚠ eslint-plugin-react(no-redundant-should-component-update): Bar does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:3:21] + 2 │ function Foo() { + 3 │ ╭─▶ return class Bar extends React.PureComponent { + 4 │ │ shouldComponentUpdate() { + 5 │ │ return true; + 6 │ │ } + 7 │ ╰─▶ }; + 8 │ } + ╰──── + + ⚠ eslint-plugin-react(no-redundant-should-component-update): Bar does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:3:21] + 2 │ function Foo() { + 3 │ ╭─▶ return class Bar extends PureComponent { + 4 │ │ shouldComponentUpdate() { + 5 │ │ return true; + 6 │ │ } + 7 │ ╰─▶ }; + 8 │ } + ╰──── + + ⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent. + ╭─[no_redundant_should_component_update.tsx:2:22] + 1 │ + 2 │ ╭─▶ var Foo = class extends PureComponent { + 3 │ │ shouldComponentUpdate() { + 4 │ │ return true; + 5 │ │ } + 6 │ ╰─▶ } + 7 │ + ╰────