diff --git a/crates/oxc_linter/src/rules/react/no_string_refs.rs b/crates/oxc_linter/src/rules/react/no_string_refs.rs index 3c1560e38..4d10868a8 100644 --- a/crates/oxc_linter/src/rules/react/no_string_refs.rs +++ b/crates/oxc_linter/src/rules/react/no_string_refs.rs @@ -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
Hello, world.
; + /// } + /// }); + /// + /// var Hello = createReactClass({ /// componentDidMount: function() { /// var component = this.refs.hello; /// // ...do something with component /// }, /// render: function() { /// return
Hello, world.
; - /// }, + /// } /// }); /// /// // Good @@ -59,76 +65,68 @@ declare_oxc_lint!( /// var component = this.hello; /// // ...do something with component /// }, - /// render: function() { - /// return
this.hello = c}>Hello, world.
; - /// }, + /// render() { + /// return
{ this.hello = c; }}>Hello, world.
; + /// } /// }); /// ``` 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
Hello {this.props.name}
; - } - }); - ", + var Hello = React.createReactClass({ + componentDidMount: function() { + var component = this.hello; + }, + render: function() { + return
this.hello = c}>Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - componentDidMount: function() { - var component = this.hello; - }, - render: function() { - return
this.hello = c}>Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - render: function() { - return
Hello {this.props.name}
; - } - }); - ", - None, - ), - ( - " - var Hello = createReactClass({ - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ]; @@ -189,73 +174,96 @@ fn test() { let fail = vec![ ( " - var Hello = createReactClass({ - componentDidMount: function() { - var component = this.refs.hello; - }, - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + componentDidMount: function() { + var component = this.refs.hello; + }, + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - componentDidMount: function() { - var component = this.refs.hello; - }, - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + componentDidMount: function() { + var component = this.refs.hello; + }, + render: function() { + return
Hello {this.props.name}
; + } + }); + ", None, ), ( " - var Hello = createReactClass({ - componentDidMount: function() { - var component = this.refs.hello; - }, - render: function() { - return
Hello {this.props.name}
; - } - }); - ", + var Hello = createReactClass({ + componentDidMount: function() { + var component = this.refs.hello; + }, + render: function() { + return
Hello {this.props.name}
; + } + }); + ", Some(serde_json::json!([{ "noTemplateLiterals": true }])), ), ( " - var Hello = createReactClass({ - componentDidMount: function() { - var component = this.refs.hello; - }, - render: function() { - return
Hello {this.props.name}
; + var Hello = createReactClass({ + componentDidMount: function() { + var component = this.refs.hello; + }, + render: function() { + return
Hello {this.props.name}
; + } + }); + ", + 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
Hello {this.props.name}
; + } + } ", Some(serde_json::json!([{ "noTemplateLiterals": true }])), ), diff --git a/crates/oxc_linter/src/snapshots/no_string_refs.snap b/crates/oxc_linter/src/snapshots/no_string_refs.snap index 08cdbe974..a21155282 100644 --- a/crates/oxc_linter/src/snapshots/no_string_refs.snap +++ b/crates/oxc_linter/src/snapshots/no_string_refs.snap @@ -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
Hello {this.props.name}
; - · ─────── + 3 │ render: function() { + 4 │ return
Hello {this.props.name}
; + · ─────────── + 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
Hello {this.props.name}
; + · ───────────── + 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
Hello {this.props.name}
; + · ─────────── + 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
Hello {this.props.name}
; - · ─────── + 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
Hello {this.props.name}
; - · ─────── - 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
Hello {this.props.name}
; - · ─────── - 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
Hello {this.props.name}
; - · ─────────────── - 8 │ } - ╰──── - help: Using string literals in ref attributes is deprecated. Use a callback instead. + help: Using this.xxx instead of this.refs.xxx diff --git a/crates/oxc_linter/src/utils/react.rs b/crates/oxc_linter/src/utils/react.rs index c14be2303..3f0fb7f00 100644 --- a/crates/oxc_linter/src/utils/react.rs +++ b/crates/oxc_linter/src/utils/react.rs @@ -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)) + }) +}