dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-25 04:42:10 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

feat(linter): support react/no-string-refs (#1055)

Browse source 
I noticed that there is already a
https://github.com/web-infra-dev/oxc/pull/1053 with the same rule PR.
This PR is currently only used to compare it

However, it seems to be missing a lot of detailing, such as not handling
the react component for es6, which is caused by the lack of test cases.

Also, the `get_parent_es5_component` and `get_parent_es6_component`
function is very useful for other react rules. If #1053 is merged, I
will create another PR to add them
This commit is contained in:
Dunqing 2023-10-25 10:08:33 -05:00 committed by GitHub
parent 5cfeda5022
commit 47837e5396
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 307 additions and 232 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

330
crates/oxc_linter/src/rules/react/no_string_refs.rs
View file

@ -1,6 +1,8 @@
use oxc_ast::ast::{CallExpression, JSXAttributeItem, JSXAttributeName};
use oxc_ast::{
ast::{Expression, JSXAttributeValue, JSXExpression, JSXExpressionContainer, MemberExpression},
ast::{
Expression, JSXAttribute, JSXAttributeItem, JSXAttributeName, JSXAttributeValue,
JSXExpression, JSXExpressionContainer,
},
AstKind,
};
use oxc_diagnostics::{
@ -8,49 +10,53 @@ use oxc_diagnostics::{
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use oxc_span::{GetSpan, Span};
use crate::{context::LintContext, rule::Rule, AstNode};
use crate::{
context::LintContext,
rule::Rule,
utils::{get_parent_es5_component, get_parent_es6_component},
AstNode,
};
#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-react(no-string-refs):")]
#[diagnostic(
severity(warning),
help("Using string literals in ref attributes is deprecated. Use a callback instead.")
)]
struct NoStringRefsDiagnostic(#[label] pub Span);
enum NoStringRefsDiagnostic {
#[error("eslint-plugin-react(no-string-refs): Using this.refs is deprecated.")]
#[diagnostic(severity(warning), help("Using this.xxx instead of this.refs.xxx"))]
ThisRefsDeprecated(#[label] Span),
#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-react(no-string-refs):")]
#[diagnostic(severity(warning), help("Using this.refs is deprecated."))]
struct NoThisRefsDiagnostic(#[label] pub Span);
#[error("eslint-plugin-react(no-string-refs): Using string literals in ref attributes is deprecated.")]
#[diagnostic(severity(warning), help("Using reference callback instead"))]
StringInRefDeprecated(#[label] Span),
}
#[derive(Debug, Default, Clone)]
pub struct NoStringRefs {
/// When set to `true`, it will give a warning when using template literals for refs.
pub no_template_literals: bool,
no_template_literals: bool,
}
declare_oxc_lint!(
/// ### What it does
///
/// This rule disallows using string references in JSX `ref` attributes.
///
/// ### Why is this bad?
///
/// String refs are considered legacy in the React documentation. Callback refs are preferred.
/// This rule prevents using string literals in ref attributes.
///
/// ### Example
/// ```javascript
/// // Bad
/// var Hello = createReactClass({
/// render: function() {
/// return <div ref="hello">Hello, world.</div>;
/// }
/// });
///
/// var Hello = createReactClass({
/// componentDidMount: function() {
/// var component = this.refs.hello;
/// // ...do something with component
/// },
/// render: function() {
/// return <div ref="hello">Hello, world.</div>;
/// },
/// }
/// });
///
/// // Good
@ -59,76 +65,68 @@ declare_oxc_lint!(
/// var component = this.hello;
/// // ...do something with component
/// },
/// render: function() {
/// return <div ref={c => this.hello = c}>Hello, world.</div>;
/// },
/// render() {
/// return <div ref={(c) => { this.hello = c; }}>Hello, world.</div>;
/// }
/// });
/// ```
NoStringRefs,
correctness
);
impl Rule for NoStringRefs {
fn from_configuration(value: serde_json::Value) -> Self {
Self {
no_template_literals: value.get(0).and_then(|x| x.get("noTemplateLiterals")).map_or(
false,
|x| match x {
serde_json::Value::Bool(b) => *b,
_ => false,
},
),
fn contains_string_literal(
expr_container: &JSXExpressionContainer,
no_template_literals: bool,
) -> bool {
if let JSXExpression::Expression(expr) = &expr_container.expression {
matches!(expr, Expression::StringLiteral(_))
|| no_template_literals && matches!(expr, Expression::TemplateLiteral(_))
} else {
false
}
}
fn is_literal_ref_attribute(attr: &JSXAttribute, no_template_literals: bool) -> bool {
let JSXAttributeName::Identifier(attr_ident) = &attr.name else { return false };
if attr_ident.name == "ref" {
if let Some(attr_value) = &attr.value {
return match attr_value {
JSXAttributeValue::ExpressionContainer(expr_container) => {
contains_string_literal(expr_container, no_template_literals)
}
JSXAttributeValue::StringLiteral(_) => true,
_ => false,
};
}
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::JSXAttributeItem(JSXAttributeItem::Attribute(attribute)) = node.kind() {
let Some(value) = &attribute.0.value else {
return;
};
if let JSXAttributeName::Identifier(iden) = &attribute.0.name {
if iden.name.as_str() != "ref" {
return;
};
}
match value {
JSXAttributeValue::StringLiteral(s) => {
ctx.diagnostic(NoStringRefsDiagnostic(s.span));
}
JSXAttributeValue::ExpressionContainer(JSXExpressionContainer {
expression: JSXExpression::Expression(expr),
..
}) => match expr {
Expression::TemplateLiteral(s) if self.no_template_literals => {
ctx.diagnostic(NoStringRefsDiagnostic(s.span));
}
Expression::StringLiteral(s) => {
ctx.diagnostic(NoStringRefsDiagnostic(s.span));
}
_ => return,
},
_ => return,
}
}
if let AstKind::MemberExpression(MemberExpression::StaticMemberExpression(expr)) =
node.kind()
{
if let (&Expression::ThisExpression(_), "refs") =
(&expr.object, expr.property.name.as_str())
{
for node_id in ctx.nodes().ancestors(node.id()).skip(1) {
let parent = ctx.nodes().get_node(node_id);
if let AstKind::CallExpression(CallExpression {
callee: Expression::Identifier(iden),
..
}) = parent.kind()
{
if iden.name.as_str() == "createReactClass" {
ctx.diagnostic(NoThisRefsDiagnostic(expr.span));
}
}
false
}
impl Rule for NoStringRefs {
fn from_configuration(value: serde_json::Value) -> Self {
let no_template_literals =
value.get("allowExpressions").and_then(serde_json::Value::as_bool).unwrap_or(false);
Self { no_template_literals }
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() {
AstKind::JSXAttributeItem(JSXAttributeItem::Attribute(attr)) => {
if is_literal_ref_attribute(attr, self.no_template_literals) {
ctx.diagnostic(NoStringRefsDiagnostic::StringInRefDeprecated(attr.span));
}
}
AstKind::MemberExpression(member_expr) => {
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(node, ctx).is_some())
{
ctx.diagnostic(NoStringRefsDiagnostic::ThisRefsDeprecated(member_expr.span()));
}
}
_ => {}
}
}
}
@ -140,48 +138,35 @@ fn test() {
let pass = vec![
(
"
var Hello = blah({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div>Hello {this.props.name}</div>;
}
});
",
var Hello = React.createReactClass({
componentDidMount: function() {
var component = this.hello;
},
render: function() {
return <div ref={c => this.hello = c}>Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
componentDidMount: function() {
var component = this.hello;
},
render: function() {
return <div ref={c => this.hello = c}>Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
render: function() {
return <div ref={`hello`}>Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
render: function() {
return <div ref={`hello`}>Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
render: function() {
return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
render: function() {
return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
}
});
",
None,
),
];
@ -189,73 +174,96 @@ fn test() {
let fail = vec![
(
"
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div>Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div>Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
render: function() {
return <div ref=\"hello\">Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
render: function() {
return <div ref=\"hello\">Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
render: function() {
return <div ref={'hello'}>Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
render: function() {
return <div ref={'hello'}>Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref=\"hello\">Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref=\"hello\">Hello {this.props.name}</div>;
}
});
",
None,
),
(
"
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref={`hello`}>Hello {this.props.name}</div>;
}
});
",
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref={`hello`}>Hello {this.props.name}</div>;
}
});
",
Some(serde_json::json!([{ "noTemplateLiterals": true }])),
),
(
"
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
var Hello = createReactClass({
componentDidMount: function() {
var component = this.refs.hello;
},
render: function() {
return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
}
});
",
Some(serde_json::json!([{ "noTemplateLiterals": true }])),
),
(
"
class Hello extends React.Component {
componentDidMount() {
var component = this.refs.hello;
}
});
}
",
None,
),
(
"
class Hello extends React.PureComponent {
componentDidMount() {
var component = this.refs.hello;
}
render() {
return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
}
}
",
Some(serde_json::json!([{ "noTemplateLiterals": true }])),
),

140
crates/oxc_linter/src/snapshots/no_string_refs.snap
View file

@ -2,85 +2,85 @@
source: crates/oxc_linter/src/tester.rs
expression: no_string_refs
---
⚠ eslint-plugin-react(no-string-refs):
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.refs is deprecated.
help: Using this.xxx instead of this.refs.xxx
⚠ eslint-plugin-react(no-string-refs):
⚠ eslint-plugin-react(no-string-refs): Using string literals in ref attributes is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ render: function() {
4 │ return <div ref="hello">Hello {this.props.name}</div>;
· ───────
3 │ render: function() {
4 │ return <div ref="hello">Hello {this.props.name}</div>;
· ───────────
5 │ }
╰────
help: Using reference callback instead
⚠ eslint-plugin-react(no-string-refs): Using string literals in ref attributes is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ render: function() {
4 │ return <div ref={'hello'}>Hello {this.props.name}</div>;
· ─────────────
5 │ }
╰────
help: Using reference callback instead
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.xxx instead of this.refs.xxx
⚠ eslint-plugin-react(no-string-refs): Using string literals in ref attributes is deprecated.
╭─[no_string_refs.tsx:6:1]
6 │ render: function() {
7 │ return <div ref="hello">Hello {this.props.name}</div>;
· ───────────
8 │ }
╰────
help: Using reference callback instead
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.xxx instead of this.refs.xxx
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.xxx instead of this.refs.xxx
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount() {
4 │ var component = this.refs.hello;
· ─────────
5 │ }
╰────
help: Using string literals in ref attributes is deprecated. Use a callback instead.
help: Using this.xxx instead of this.refs.xxx
⚠ eslint-plugin-react(no-string-refs):
⚠ eslint-plugin-react(no-string-refs): Using this.refs is deprecated.
╭─[no_string_refs.tsx:3:1]
3 │ render: function() {
4 │ return <div ref={'hello'}>Hello {this.props.name}</div>;
· ───────
3 │ componentDidMount() {
4 │ var component = this.refs.hello;
· ─────────
5 │ }
╰────
help: Using string literals in ref attributes is deprecated. Use a callback instead.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.refs is deprecated.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:6:1]
6 │ render: function() {
7 │ return <div ref="hello">Hello {this.props.name}</div>;
· ───────
8 │ }
╰────
help: Using string literals in ref attributes is deprecated. Use a callback instead.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.refs is deprecated.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:6:1]
6 │ render: function() {
7 │ return <div ref={`hello`}>Hello {this.props.name}</div>;
· ───────
8 │ }
╰────
help: Using string literals in ref attributes is deprecated. Use a callback instead.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:3:1]
3 │ componentDidMount: function() {
4 │ var component = this.refs.hello;
· ─────────
5 │ },
╰────
help: Using this.refs is deprecated.
⚠ eslint-plugin-react(no-string-refs):
╭─[no_string_refs.tsx:6:1]
6 │ render: function() {
7 │ return <div ref={`hello${index}`}>Hello {this.props.name}</div>;
· ───────────────
8 │ }
╰────
help: Using string literals in ref attributes is deprecated. Use a callback instead.
help: Using this.xxx instead of this.refs.xxx

69
crates/oxc_linter/src/utils/react.rs
View file

@ -1,4 +1,10 @@
use oxc_ast::ast::{CallExpression, JSXAttributeItem, JSXAttributeName, JSXOpeningElement};
use oxc_ast::{
ast::{CallExpression, Expression, JSXAttributeItem, JSXAttributeName, JSXOpeningElement},
AstKind,
};
use oxc_semantic::AstNode;
use crate::LintContext;
pub fn is_create_element_call(call_expr: &CallExpression) -> bool {
if let Some(member_expr) = call_expr.callee.get_member_expr() {
@ -21,3 +27,64 @@ pub fn has_jsx_prop<'a>(
}
})
}
const PRAGMA: &str = "React";
const CREATE_CLASS: &str = "createReactClass";
pub fn is_es5_component(node: &AstNode) -> bool {
let AstKind::CallExpression(call_expr) = node.kind() else { return false };
if let Expression::MemberExpression(member_expr) = &call_expr.callee {
if let Expression::Identifier(ident) = member_expr.object() {
return ident.name == PRAGMA
&& member_expr.static_property_name() == Some(CREATE_CLASS);
}
}
if let Some(ident_reference) = call_expr.callee.get_identifier_reference() {
return ident_reference.name == CREATE_CLASS;
}
false
}
const COMPONENT: &str = "Component";
const PURE_COMPONENT: &str = "PureComponent";
pub fn is_es6_component(node: &AstNode) -> bool {
let AstKind::Class(class_expr) = node.kind() else { return false };
if let Some(super_class) = &class_expr.super_class {
if let Expression::MemberExpression(member_expr) = super_class {
if let Expression::Identifier(ident) = member_expr.object() {
return ident.name == PRAGMA
&& member_expr
.static_property_name()
.is_some_and(|name| name == COMPONENT || name == PURE_COMPONENT);
}
}
if let Some(ident_reference) = super_class.get_identifier_reference() {
return ident_reference.name == COMPONENT || ident_reference.name == PURE_COMPONENT;
}
}
false
}
pub fn get_parent_es5_component<'a, 'b>(
node: &'b AstNode<'a>,
ctx: &'b LintContext<'a>,
) -> Option<&'b AstNode<'a>> {
ctx.nodes().ancestors(node.id()).skip(1).find_map(|node_id| {
is_es5_component(ctx.nodes().get_node(node_id)).then(|| ctx.nodes().get_node(node_id))
})
}
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()).skip(1).find_map(|node_id| {
is_es6_component(ctx.nodes().get_node(node_id)).then(|| ctx.nodes().get_node(node_id))
})
}