Skip to content

Commit

Permalink
fix(linter): react/no_set_state + react/no_string_refs rules find…
Browse files Browse the repository at this point in the history
… correct parent (#5615)

`get_parent_es6_component` was finding *any* binding which is inside a class component, rather than parent of current node, leading to false positives.

The added test cases were not behaving correctly previously.
  • Loading branch information
overlookmotel committed Sep 9, 2024
1 parent 2e367c9 commit 54e2e76
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
22 changes: 21 additions & 1 deletion crates/oxc_linter/src/rules/react/no_set_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Rule for NoSetState {
if !matches!(member_expr.object(), Expression::ThisExpression(_))
|| !member_expr.static_property_name().is_some_and(|str| str == "setState")
|| !(get_parent_es5_component(node, ctx).is_some()
|| get_parent_es6_component(ctx).is_some())
|| get_parent_es6_component(node, ctx).is_some())
{
return;
}
Expand Down Expand Up @@ -105,6 +105,26 @@ fn test() {
}
});
",
"
var Hello = function() {
this.setState({})
};
createReactClass({
render: function() {
let x;
}
});
",
"
var Hello = function() {
this.setState({})
};
class Other extends React.Component {
render() {
let x;
}
};
",
];

let fail = vec![
Expand Down
36 changes: 35 additions & 1 deletion crates/oxc_linter/src/rules/react/no_string_refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl Rule for NoStringRefs {
if matches!(member_expr.object(), Expression::ThisExpression(_))
&& member_expr.static_property_name() == Some("refs")
&& (get_parent_es5_component(node, ctx).is_some()
|| get_parent_es6_component(ctx).is_some())
|| get_parent_es6_component(node, ctx).is_some())
{
ctx.diagnostic(this_refs_deprecated(member_expr.span()));
}
Expand All @@ -141,6 +141,14 @@ fn test() {
use crate::tester::Tester;

let pass = vec![
(
"
var Hello = function() {
return this.refs;
};
",
None,
),
(
"
var Hello = React.createReactClass({
Expand Down Expand Up @@ -174,6 +182,32 @@ fn test() {
",
None,
),
(
"
var Hello = function() {
return this.refs;
};
createReactClass({
render: function() {
let x;
}
});
",
None,
),
(
"
var Hello = function() {
return this.refs;
};
class Other extends React.Component {
render() {
let x;
}
};
",
None,
),
];

let fail = vec![
Expand Down
18 changes: 7 additions & 11 deletions crates/oxc_linter/src/utils/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use oxc_ast::{
},
match_member_expression, AstKind,
};
use oxc_semantic::{AstNode, SymbolFlags};
use oxc_semantic::AstNode;

use crate::{LintContext, OxlintSettings};

Expand Down Expand Up @@ -199,16 +199,12 @@ pub fn get_parent_es5_component<'a, 'b>(
})
}

pub fn get_parent_es6_component<'a, 'b>(ctx: &'b LintContext<'a>) -> Option<&'b AstNode<'a>> {
ctx.semantic().symbols().iter_rev().find_map(|symbol| {
let flags = ctx.semantic().symbols().get_flags(symbol);
if flags.contains(SymbolFlags::Class) {
let node = ctx.semantic().symbol_declaration(symbol);
if is_es6_component(node) {
return Some(node);
}
}
None
pub fn get_parent_es6_component<'a, 'b>(
node: &'b AstNode<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b AstNode<'a>> {
ctx.nodes().ancestors(node.id()).find_map(|node_id| {
is_es6_component(ctx.nodes().get_node(node_id)).then(|| ctx.nodes().get_node(node_id))
})
}

Expand Down

0 comments on commit 54e2e76

Please sign in to comment.