diff --git a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap index e8d03e6d82600..bab68f3051a3c 100644 --- a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap @@ -6,7 +6,7 @@ arguments: -c .oxlintrc.json working directory: fixtures/issue_11644 ---------- Found 0 warnings and 0 errors. -Finished in ms on 1 file with 166 rules using 1 threads. +Finished in ms on 1 file with 167 rules using 1 threads. ---------- CLI result: LintSucceeded ---------- diff --git a/crates/oxc_linter/src/config/settings/react.rs b/crates/oxc_linter/src/config/settings/react.rs index c1722cc59f9df..c59511ec17893 100644 --- a/crates/oxc_linter/src/config/settings/react.rs +++ b/crates/oxc_linter/src/config/settings/react.rs @@ -163,6 +163,17 @@ impl ReactVersion { pub fn patch(&self) -> u32 { self.patch } + + /// Checks if the React version supports `UNSAFE_` prefixed lifecycle methods. + /// + /// React 16.3 introduced the `UNSAFE_` prefixed lifecycle methods + /// (`UNSAFE_componentWillMount`, `UNSAFE_componentWillReceiveProps`, `UNSAFE_componentWillUpdate`). + /// + /// Returns `true` if this version is >= 16.3. + #[inline] + pub fn supports_unsafe_lifecycle_prefix(&self) -> bool { + self.major > 16 || (self.major == 16 && self.minor >= 3) + } } impl fmt::Display for ReactVersion { @@ -281,6 +292,33 @@ mod test { assert!(serde_json::from_str::(r#""18. 2.0""#).is_err()); } + #[test] + fn test_supports_unsafe_lifecycle_prefix() { + // Version 16.3.0 - should support UNSAFE_ prefix + let v16_3: ReactVersion = serde_json::from_str(r#""16.3.0""#).unwrap(); + assert!(v16_3.supports_unsafe_lifecycle_prefix()); + + // Version 16.4.0 - should support UNSAFE_ prefix + let v16_4: ReactVersion = serde_json::from_str(r#""16.4.0""#).unwrap(); + assert!(v16_4.supports_unsafe_lifecycle_prefix()); + + // Version 17.0.0 - should support UNSAFE_ prefix + let v17: ReactVersion = serde_json::from_str(r#""17.0.0""#).unwrap(); + assert!(v17.supports_unsafe_lifecycle_prefix()); + + // Version 16.2.0 - should NOT support UNSAFE_ prefix + let v16_2: ReactVersion = serde_json::from_str(r#""16.2.0""#).unwrap(); + assert!(!v16_2.supports_unsafe_lifecycle_prefix()); + + // Version 16.0.0 - should NOT support UNSAFE_ prefix + let v16_0: ReactVersion = serde_json::from_str(r#""16.0.0""#).unwrap(); + assert!(!v16_0.supports_unsafe_lifecycle_prefix()); + + // Version 15.0.0 - should NOT support UNSAFE_ prefix + let v15: ReactVersion = serde_json::from_str(r#""15.0.0""#).unwrap(); + assert!(!v15.supports_unsafe_lifecycle_prefix()); + } + #[test] fn test_version_regex() { let re = &*REACT_VERSION_REGEX; diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index f987e5aa769b4..362a0e1820abc 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -2504,6 +2504,12 @@ impl RuleRunner for crate::rules::react::no_unsafe::NoUnsafe { const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner for crate::rules::react::no_will_update_set_state::NoWillUpdateSetState { + const NODE_TYPES: Option<&AstTypesBitset> = + Some(&AstTypesBitset::from_types(&[AstType::CallExpression])); + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; +} + impl RuleRunner for crate::rules::react::only_export_components::OnlyExportComponents { const NODE_TYPES: Option<&AstTypesBitset> = None; const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::RunOnce; diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 073302ab5a7e8..0210cbd00fc30 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -394,6 +394,7 @@ pub(crate) mod react { pub mod no_unescaped_entities; pub mod no_unknown_property; pub mod no_unsafe; + pub mod no_will_update_set_state; pub mod only_export_components; pub mod prefer_es6_class; pub mod react_in_jsx_scope; @@ -1103,6 +1104,7 @@ oxc_macros::declare_all_lint_rules! { react::no_unescaped_entities, react::no_unknown_property, react::no_unsafe, + react::no_will_update_set_state, react::only_export_components, react::prefer_es6_class, react::react_in_jsx_scope, diff --git a/crates/oxc_linter/src/rules/react/no_unsafe.rs b/crates/oxc_linter/src/rules/react/no_unsafe.rs index e1edd0318935a..3d92bb805825a 100644 --- a/crates/oxc_linter/src/rules/react/no_unsafe.rs +++ b/crates/oxc_linter/src/rules/react/no_unsafe.rs @@ -99,20 +99,16 @@ impl Rule for NoUnsafe { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { AstKind::MethodDefinition(method_def) => { - let react_version = ctx.settings().react.version.as_ref(); - if let Some(name) = method_def.key.static_name() - && is_unsafe_method(name.as_ref(), self.0.check_aliases, react_version) + && is_unsafe_method(name.as_ref(), self.0.check_aliases, ctx) && get_parent_component(node, ctx).is_some() { ctx.diagnostic(no_unsafe_diagnostic(name.as_ref(), method_def.key.span())); } } AstKind::ObjectProperty(obj_prop) => { - let react_version = ctx.settings().react.version.as_ref(); - if let Some(name) = obj_prop.key.static_name() - && is_unsafe_method(name.as_ref(), self.0.check_aliases, react_version) + && is_unsafe_method(name.as_ref(), self.0.check_aliases, ctx) { for ancestor in ctx.nodes().ancestors(node.id()) { if is_es5_component(ancestor) { @@ -135,10 +131,13 @@ impl Rule for NoUnsafe { } /// Check if a method name is an unsafe lifecycle method -fn is_unsafe_method(name: &str, check_aliases: bool, react_version: Option<&ReactVersion>) -> bool { - // React 16.3 introduced the UNSAFE_ prefixed lifecycle methods - let check_unsafe_prefix = - react_version.is_none_or(|v| v.major() > 16 || (v.major() == 16 && v.minor() >= 3)); +fn is_unsafe_method(name: &str, check_aliases: bool, ctx: &LintContext) -> bool { + let check_unsafe_prefix = ctx + .settings() + .react + .version + .as_ref() + .is_none_or(ReactVersion::supports_unsafe_lifecycle_prefix); match name { "UNSAFE_componentWillMount" diff --git a/crates/oxc_linter/src/rules/react/no_will_update_set_state.rs b/crates/oxc_linter/src/rules/react/no_will_update_set_state.rs new file mode 100644 index 0000000000000..f23cff7904f7f --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_will_update_set_state.rs @@ -0,0 +1,414 @@ +use oxc_ast::{AstKind, ast::Expression}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::{ + AstNode, + config::ReactVersion, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, + utils::{is_es5_component, is_es6_component}, +}; + +fn no_will_update_set_state_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Do not use `setState` in `componentWillUpdate`.") + .with_help("Updating state during the update step can lead to indeterminate component state and is not allowed.") + .with_label(span) +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub enum NoWillUpdateSetStateConfig { + #[default] + #[serde(skip)] + Allowed, + DisallowInFunc, +} + +#[derive(Debug, Default, Clone, Deserialize)] +pub struct NoWillUpdateSetState(NoWillUpdateSetStateConfig); + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows using `setState` in the `componentWillUpdate` lifecycle method. + /// + /// ### Why is this bad? + /// + /// Updating the state during the component update step can lead to indeterminate component state and is not allowed. + /// This can cause unexpected behavior and bugs in your React application. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```jsx + /// var Hello = createReactClass({ + /// componentWillUpdate: function() { + /// this.setState({ + /// name: this.props.name.toUpperCase() + /// }); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```jsx + /// var Hello = createReactClass({ + /// componentWillUpdate: function() { + /// this.props.prepareHandler(); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// ``` + NoWillUpdateSetState, + react, + correctness, + config = NoWillUpdateSetStateConfig, +); + +impl Rule for NoWillUpdateSetState { + fn from_configuration(value: serde_json::Value) -> Result { + Ok(serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner()) + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::CallExpression(call_expr) = node.kind() else { return }; + + let Some(member_expr) = call_expr.callee.as_member_expression() else { return }; + + if !matches!(member_expr.object(), Expression::ThisExpression(_)) + || member_expr.static_property_name().is_none_or(|name| name != "setState") + { + return; + } + + let check_unsafe_prefix = ctx + .settings() + .react + .version + .as_ref() + .is_none_or(ReactVersion::supports_unsafe_lifecycle_prefix); + + let mut function_count = 0; + let mut found_component_will_update = false; + let mut in_component = false; + + for ancestor in ctx.nodes().ancestors(node.id()).skip(1) { + if found_component_will_update { + if is_es5_component(ancestor) || is_es6_component(ancestor) { + in_component = true; + break; + } + } else { + if matches!( + ancestor.kind(), + AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) + ) { + function_count += 1; + } + + let is_component_will_update = match ancestor.kind() { + AstKind::ObjectProperty(prop) => prop.key.static_name().is_some_and(|key| { + if key == "UNSAFE_componentWillUpdate" && !check_unsafe_prefix { + return false; + } + key == "componentWillUpdate" || key == "UNSAFE_componentWillUpdate" + }), + AstKind::MethodDefinition(method) => { + method.key.static_name().is_some_and(|name| { + if name == "UNSAFE_componentWillUpdate" && !check_unsafe_prefix { + return false; + } + name == "componentWillUpdate" || name == "UNSAFE_componentWillUpdate" + }) + } + AstKind::PropertyDefinition(prop) => { + prop.key.static_name().is_some_and(|name| { + if name == "UNSAFE_componentWillUpdate" && !check_unsafe_prefix { + return false; + } + name == "componentWillUpdate" || name == "UNSAFE_componentWillUpdate" + }) + } + _ => false, + }; + + if is_component_will_update { + found_component_will_update = true; + } + } + } + + if !found_component_will_update || !in_component { + return; + } + + let in_nested_function = function_count > 1; + + if in_nested_function && !matches!(self.0, NoWillUpdateSetStateConfig::DisallowInFunc) { + return; + } + + ctx.diagnostic(no_will_update_set_state_diagnostic(call_expr.callee.span())); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + " + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() {} + }); + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + someNonMemberFunction(arg); + this.someHandler = this.setState; + } + }); + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + someClass.onSomeEvent(function(data) { + this.setState({ + data: data + }); + }) + } + }); + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + function handleEvent(data) { + this.setState({ + data: data + }); + } + someClass.onSomeEvent(handleEvent) + } + }); + ", + None, + None, + ), + ( + " + class Hello extends React.Component { + UNSAFE_componentWillUpdate() { + this.setState({ + data: data + }); + } + } + ", + None, + Some(serde_json::json!({ "settings": { "react": { "version": "16.2.0" } } })), + ), + ]; + + let fail = vec![ + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + this.setState({ + data: data + }); + } + }); + ", + None, + None, + ), + ( + " + class Hello extends React.Component { + componentWillUpdate() { + this.setState({ + data: data + }); + } + } + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + this.setState({ + data: data + }); + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + class Hello extends React.Component { + componentWillUpdate() { + this.setState({ + data: data + }); + } + } + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + someClass.onSomeEvent(function(data) { + this.setState({ + data: data + }); + }) + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + class Hello extends React.Component { + componentWillUpdate() { + someClass.onSomeEvent(function(data) { + this.setState({ + data: data + }); + }) + } + } + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + if (true) { + this.setState({ + data: data + }); + } + } + }); + ", + None, + None, + ), + ( + " + class Hello extends React.Component { + componentWillUpdate() { + if (true) { + this.setState({ + data: data + }); + } + } + } + ", + None, + None, + ), + ( + " + var Hello = createReactClass({ + componentWillUpdate: function() { + someClass.onSomeEvent((data) => this.setState({data: data})); + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + class Hello extends React.Component { + componentWillUpdate() { + someClass.onSomeEvent((data) => this.setState({data: data})); + } + } + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + class Hello extends React.Component { + UNSAFE_componentWillUpdate() { + this.setState({ + data: data + }); + } + } + ", + None, + Some(serde_json::json!({ "settings": { "react": { "version": "16.3.0" } } })), + ), + ( + " + var Hello = createReactClass({ + UNSAFE_componentWillUpdate: function() { + this.setState({ + data: data + }); + } + }); + ", + None, + Some(serde_json::json!({ "settings": { "react": { "version": "16.3.0" } } })), + ), + ]; + + Tester::new(NoWillUpdateSetState::NAME, NoWillUpdateSetState::PLUGIN, pass, fail) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/react_no_will_update_set_state.snap b/crates/oxc_linter/src/snapshots/react_no_will_update_set_state.snap new file mode 100644 index 0000000000000..efd074b5c4b5b --- /dev/null +++ b/crates/oxc_linter/src/snapshots/react_no_will_update_set_state.snap @@ -0,0 +1,110 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ componentWillUpdate: function() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ componentWillUpdate() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ componentWillUpdate: function() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ componentWillUpdate() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:5:18] + 4 │ someClass.onSomeEvent(function(data) { + 5 │ this.setState({ + · ───────────── + 6 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:5:18] + 4 │ someClass.onSomeEvent(function(data) { + 5 │ this.setState({ + · ───────────── + 6 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:5:18] + 4 │ if (true) { + 5 │ this.setState({ + · ───────────── + 6 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:5:18] + 4 │ if (true) { + 5 │ this.setState({ + · ───────────── + 6 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:48] + 3 │ componentWillUpdate: function() { + 4 │ someClass.onSomeEvent((data) => this.setState({data: data})); + · ───────────── + 5 │ } + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:48] + 3 │ componentWillUpdate() { + 4 │ someClass.onSomeEvent((data) => this.setState({data: data})); + · ───────────── + 5 │ } + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ UNSAFE_componentWillUpdate() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed. + + ⚠ eslint-plugin-react(no-will-update-set-state): Do not use `setState` in `componentWillUpdate`. + ╭─[no_will_update_set_state.tsx:4:16] + 3 │ UNSAFE_componentWillUpdate: function() { + 4 │ this.setState({ + · ───────────── + 5 │ data: data + ╰──── + help: Updating state during the update step can lead to indeterminate component state and is not allowed.