Skip to content

Commit

Permalink
fix(linter/no-direct-mutation-state): false positive when class is de…
Browse files Browse the repository at this point in the history 
…clared inside a `CallExpression` (#3294)

fixes #3290
  • Loading branch information
@Boshen
Boshen committed May 15, 2024
1 parent 8ff1ffb commit e12323f
Showing 1 changed file with 80 additions and 127 deletions.
207 changes: 80 additions & 127 deletions crates/oxc_linter/src/rules/react/no_direct_mutation_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,44 @@ declare_oxc_lint!(
correctness
);

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 Some(assignment) = assignment_expr.left.as_simple_assignment_target() {
if let Some(outer_member_expression) = get_outer_member_expression(assignment) {
if is_state_member_expression(outer_member_expression) {
ctx.diagnostic(no_direct_mutation_state_diagnostic(
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(no_direct_mutation_state_diagnostic(update_expr.span));
}
}
}

_ => {}
}
}
}

// check current node is this.state.xx
fn is_state_member_expression(expression: &StaticMemberExpression<'_>) -> bool {
if let Expression::ThisExpression(_) = &expression.object {
Expand Down Expand Up @@ -133,9 +171,9 @@ fn get_static_member_expression_obj<'a, 'b>(
}

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;
let mut is_constructor = false;
let mut is_call_expression = false;
let mut is_component = false;

for parent in ctx.nodes().iter_parents(node.id()) {
if let AstKind::MethodDefinition(method_def) = parent.kind() {
Expand All @@ -144,71 +182,33 @@ fn should_ignore_component<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'
}
}

if let AstKind::CallExpression(_) = parent.kind() {
is_call_expression_node = true;
if matches!(parent.kind(), AstKind::CallExpression(_)) {
is_call_expression = 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 Some(assignment) = assignment_expr.left.as_simple_assignment_target() {
if let Some(outer_member_expression) = get_outer_member_expression(assignment) {
if is_state_member_expression(outer_member_expression) {
ctx.diagnostic(no_direct_mutation_state_diagnostic(
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(no_direct_mutation_state_diagnostic(update_expr.span));
}
}
}

_ => {}
if matches!(parent.kind(), AstKind::Class(_)) {
break;
}
}

(is_constructor && !is_call_expression) || !is_component
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
(
"var Hello = createReactClass({
"var Hello = createReactClass({
render: function() {
return <div>Hello {this.props.name}</div>;
}
});",
None,
),
(
"
"
var Hello = createReactClass({
render: function() {
var obj = {state: {}};
Expand All @@ -217,48 +217,33 @@ fn test() {
}
});
",
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();
Expand All @@ -271,13 +256,22 @@ fn test() {
}
}
",
None,
),
"
describe('Component spec', () => {
it('should apply default props on rerender', () => {
class Outer extends Component {
constructor() {
super();
this.state = { i: 1 };
}
}
});
});
",
];

let fail = vec![
(
r#"
r#"
var Hello = createReactClass({
componentWillMount() {
Expand All @@ -297,43 +291,31 @@ fn test() {
}
});
"#,
None,
),
(
"
"
var Hello = createReactClass({
render: function() {
this.state.foo++;
return <div>Hello {this.props.name}</div>;
}
});
",
None,
),
(
r#"
r#"
var Hello = createReactClass({
render: function() {
this.state.person.name= "bar"
return <div>Hello {this.props.name}</div>;
}
});
"#,
None,
),
(
r#"
r#"
var Hello = createReactClass({
render: function() {
this.state.person.name.first = "bar"
return <div>Hello</div>;
}
});
"#,
None,
),
(
r#"
r#"
var Hello = createReactClass({
render: function() {
this.state.person.name.first = "bar"
Expand All @@ -342,10 +324,7 @@ fn test() {
}
});
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
constructor() {
someFn()
Expand All @@ -355,10 +334,7 @@ fn test() {
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
constructor(props) {
super(props)
Expand All @@ -368,78 +344,55 @@ fn test() {
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentWillMount() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentDidMount() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentWillReceiveProps() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
shouldComponentUpdate() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentWillUpdate() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentDidUpdate() {
this.state.foo = "bar"
}
}
"#,
None,
),
(
r#"
r#"
class Hello extends React.Component {
componentWillUnmount() {
this.state.foo = "bar"
}
}
"#,
None,
),
];

Tester::new(NoDirectMutationState::NAME, pass, fail).test_and_snapshot();
Expand Down

0 comments on commit e12323f

Please sign in to comment.