From 40dbfae3c2d472e47f7733a35354d8cda1267b76 Mon Sep 17 00:00:00 2001 From: zhangrunzhao <51067792+zhangrunzhao@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:32:19 +0800 Subject: [PATCH] feat(linter): eslint-plugin-react: no-direct-mutation-state (#1892) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1022 Based on: - code: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/no-direct-mutation-state.js - doc: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-direct-mutation-state.md - test: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/tests/lib/rules/no-direct-mutation-state.js Co-authored-by: 张润钊 --- crates/oxc_linter/src/rules.rs | 2 + .../rules/react/no_direct_mutation_state.rs | 460 ++++++++++++++++++ .../snapshots/no_direct_mutation_state.snap | 159 ++++++ 3 files changed, 621 insertions(+) create mode 100644 crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs create mode 100644 crates/oxc_linter/src/snapshots/no_direct_mutation_state.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 02f80e5d98171..b02ea6e4dfbc5 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -147,6 +147,7 @@ mod react { pub mod jsx_no_useless_fragment; pub mod no_children_prop; pub mod no_dangerously_set_inner_html; + pub mod no_direct_mutation_state; pub mod no_find_dom_node; pub mod no_is_mounted; pub mod no_render_return_value; @@ -483,6 +484,7 @@ oxc_macros::declare_all_lint_rules! { react::react_in_jsx_scope, react::no_children_prop, react::no_dangerously_set_inner_html, + react::no_direct_mutation_state, react::no_find_dom_node, react::no_render_return_value, react::no_string_refs, diff --git a/crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs b/crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs new file mode 100644 index 0000000000000..83147353600be --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs @@ -0,0 +1,460 @@ +use oxc_ast::{ + ast::{ + AssignmentTarget, Expression, MemberExpression, MethodDefinitionKind, + SimpleAssignmentTarget, StaticMemberExpression, + }, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{is_es5_component, is_es6_component}, + AstNode, +}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly.")] +#[diagnostic( + severity(warning), + help("calling setState() afterwards may replace the mutation you made.") +)] +struct NoDirectMutationStateDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoDirectMutationState; + +// code: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/no-direct-mutation-state.js +// doc: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-direct-mutation-state.md +// test: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/tests/lib/rules/no-direct-mutation-state.js + +declare_oxc_lint!( + /// ### What it does + /// The restriction coder cannot directly change the value of this.state + /// + /// ### Why is this bad? + /// calling setState() afterwards may replace the mutation you made + /// + /// ### Example + /// ```javascript + /// // error + /// var Hello = createReactClass({ + /// componentDidMount: function() { + /// this.state.name = this.props.name.toUpperCase(); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// + /// class Hello extends React.Component { + /// constructor(props) { + /// super(props) + /// + /// doSomethingAsync(() => { + /// this.state = 'bad'; + /// }); + /// } + /// } + /// + /// // success + /// var Hello = createReactClass({ + /// componentDidMount: function() { + /// this.setState({ + /// name: this.props.name.toUpperCase(); + /// }); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// + /// class Hello extends React.Component { + /// constructor(props) { + /// super(props) + /// + /// this.state = { + /// foo: 'bar', + /// } + /// } + /// } + /// ``` + NoDirectMutationState, + correctness +); + +// check current node is this.state.xx +fn is_state_member_expression(expression: &StaticMemberExpression<'_>) -> bool { + if let Expression::ThisExpression(_) = &expression.object { + return expression.property.name == "state"; + } + + false +} + +// get the top iterator +// example: this.state.a.b.c.d => this.state +fn get_outer_member_expression<'a, 'b>( + assignment: &'b SimpleAssignmentTarget<'a>, +) -> Option<&'b StaticMemberExpression<'a>> { + if let SimpleAssignmentTarget::MemberAssignmentTarget(member_expr) = assignment { + match &member_expr.0 { + MemberExpression::StaticMemberExpression(expr) => { + let mut node = expr; + loop { + if node.object.is_null() { + return Some(node); + } + + if let Some(object) = get_static_member_expression_obj(&node.object) { + if !object.property.name.is_empty() { + node = object; + + continue; + } + } + + return Some(node); + } + } + MemberExpression::PrivateFieldExpression(_) + | MemberExpression::ComputedMemberExpression(_) => {} + } + } + + None +} + +// Because node.object is of type &Expression<'_> +// We need a function to get static_member_expression +fn get_static_member_expression_obj<'a, 'b>( + expression: &'b Expression<'a>, +) -> Option<&'b StaticMemberExpression<'a>> { + match expression { + Expression::MemberExpression(member_expr) => match &member_expr.0 { + MemberExpression::StaticMemberExpression(expr) => Some(expr), + _ => None, + }, + _ => None, + } +} + +fn should_ignore_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool { + let mut is_constructor: bool = false; + let mut is_call_expression_node: bool = false; + let mut is_component: bool = false; + + for parent in ctx.nodes().iter_parents(node.id()) { + if let AstKind::MethodDefinition(method_def) = parent.kind() { + if method_def.kind == MethodDefinitionKind::Constructor { + is_constructor = true; + } + } + + if let AstKind::CallExpression(_) = parent.kind() { + is_call_expression_node = true; + } + + if is_es6_component(parent) || is_es5_component(parent) { + is_component = true; + } + } + + is_constructor && !is_call_expression_node || !is_component +} + +impl Rule for NoDirectMutationState { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::AssignmentExpression(assignment_expr) => { + if should_ignore_component(node, ctx) { + return; + } + + if let AssignmentTarget::SimpleAssignmentTarget(assignment) = &assignment_expr.left + { + if let Some(outer_member_expression) = get_outer_member_expression(assignment) { + if is_state_member_expression(outer_member_expression) { + ctx.diagnostic(NoDirectMutationStateDiagnostic( + assignment_expr.left.span(), + )); + } + } + } + } + + AstKind::UpdateExpression(update_expr) => { + if should_ignore_component(node, ctx) { + return; + } + + if let Some(outer_member_expression) = + get_outer_member_expression(&update_expr.argument) + { + if is_state_member_expression(outer_member_expression) { + ctx.diagnostic(NoDirectMutationStateDiagnostic(update_expr.span)); + } + } + } + + _ => {} + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + "var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + });", + None, + ), + ( + " + var Hello = createReactClass({ + render: function() { + var obj = {state: {}}; + obj.state.name = 'foo'; + return
Hello {obj.state.name}
; + } + }); + ", + None, + ), + ( + " + var Hello = 'foo'; + module.exports = {}; + ", + None, + ), + ( + " + class Hello { + getFoo() { + this.state.foo = 'bar' + return this.state.foo; + } + } + ", + None, + ), + ( + " + class Hello extends React.Component { + constructor() { + this.state.foo = 'bar' + } + } + ", + None, + ), + ( + " + class Hello extends React.Component { + constructor() { + this.state.foo = 1; + } + } + ", + None, + ), + ( + " + class OneComponent extends Component { + constructor() { + super(); + class AnotherComponent extends Component { + constructor() { + super(); + } + } + this.state = {}; + } + } + ", + None, + ), + ]; + + let fail = vec![ + ( + r#" + var Hello = createReactClass({ + + componentWillMount() { + this.state.foo = "Chicken, you're so beautiful" + }, + + render: function() { + this.state.foo = "Chicken, you're so beautiful" + return
Hello{this.props.name}
; + } + }); + + var Hello2 = createReactClass({ + render: () => { + this.state.foo = "Chicken, you're so beautiful" + return
Hello {this.props.name}
; + } + }); + "#, + None, + ), + ( + " + var Hello = createReactClass({ + render: function() { + this.state.foo++; + return
Hello {this.props.name}
; + } + }); + ", + None, + ), + ( + r#" + var Hello = createReactClass({ + render: function() { + this.state.person.name= "bar" + return
Hello {this.props.name}
; + } + }); + "#, + None, + ), + ( + r#" + var Hello = createReactClass({ + render: function() { + this.state.person.name.first = "bar" + return
Hello
; + } + }); + "#, + None, + ), + ( + r#" + var Hello = createReactClass({ + render: function() { + this.state.person.name.first = "bar" + this.state.person.name.last = "baz" + return
Hello
; + } + }); + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + constructor() { + someFn() + } + someFn() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + constructor(props) { + super(props) + doSomethingAsync(() => { + this.state = "bad"; + }); + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentWillMount() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentDidMount() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentWillReceiveProps() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + shouldComponentUpdate() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentWillUpdate() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentDidUpdate() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ( + r#" + class Hello extends React.Component { + componentWillUnmount() { + this.state.foo = "bar" + } + } + "#, + None, + ), + ]; + + Tester::new(NoDirectMutationState::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_direct_mutation_state.snap b/crates/oxc_linter/src/snapshots/no_direct_mutation_state.snap new file mode 100644 index 0000000000000..dd3db8454f887 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_direct_mutation_state.snap @@ -0,0 +1,159 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 144 +expression: no_direct_mutation_state +--- + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:4:1] + 4 │ componentWillMount() { + 5 │ this.state.foo = "Chicken, you're so beautiful" + · ────────────── + 6 │ }, + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:8:1] + 8 │ render: function() { + 9 │ this.state.foo = "Chicken, you're so beautiful" + · ────────────── + 10 │ return
Hello{this.props.name}
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:15:1] + 15 │ render: () => { + 16 │ this.state.foo = "Chicken, you're so beautiful" + · ────────────── + 17 │ return
Hello {this.props.name}
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ render: function() { + 4 │ this.state.foo++; + · ──────────────── + 5 │ return
Hello {this.props.name}
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ render: function() { + 4 │ this.state.person.name= "bar" + · ────────────────────── + 5 │ return
Hello {this.props.name}
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ render: function() { + 4 │ this.state.person.name.first = "bar" + · ──────────────────────────── + 5 │ return
Hello
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ render: function() { + 4 │ this.state.person.name.first = "bar" + · ──────────────────────────── + 5 │ this.state.person.name.last = "baz" + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:4:1] + 4 │ this.state.person.name.first = "bar" + 5 │ this.state.person.name.last = "baz" + · ─────────────────────────── + 6 │ return
Hello
; + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:6:1] + 6 │ someFn() { + 7 │ this.state.foo = "bar" + · ────────────── + 8 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:5:1] + 5 │ doSomethingAsync(() => { + 6 │ this.state = "bad"; + · ────────── + 7 │ }); + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentWillMount() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentDidMount() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentWillReceiveProps() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ shouldComponentUpdate() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentWillUpdate() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentDidUpdate() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + + ⚠ eslint-plugin-react(no-direct-mutation-state): never mutate this.state directly. + ╭─[no_direct_mutation_state.tsx:3:1] + 3 │ componentWillUnmount() { + 4 │ this.state.foo = "bar" + · ────────────── + 5 │ } + ╰──── + help: calling setState() afterwards may replace the mutation you made. + +