diff --git a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap index 67f6465492117..e8d03e6d82600 100644 --- a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap @@ -6,7 +6,7 @@ arguments: -c .oxlintrc.json working directory: fixtures/issue_11644 ---------- Found 0 warnings and 0 errors. -Finished in ms on 1 file with 165 rules using 1 threads. +Finished in ms on 1 file with 166 rules using 1 threads. ---------- CLI result: LintSucceeded ---------- diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index 851b412f98b3d..f987e5aa769b4 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -2480,6 +2480,12 @@ impl RuleRunner for crate::rules::react::no_string_refs::NoStringRefs { const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner for crate::rules::react::no_this_in_sfc::NoThisInSfc { + const NODE_TYPES: Option<&AstTypesBitset> = + Some(&AstTypesBitset::from_types(&[AstType::ThisExpression])); + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; +} + impl RuleRunner for crate::rules::react::no_unescaped_entities::NoUnescapedEntities { const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[AstType::JSXText])); diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index aebbc6eb2abfb..073302ab5a7e8 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -390,6 +390,7 @@ pub(crate) mod react { pub mod no_render_return_value; pub mod no_set_state; pub mod no_string_refs; + pub mod no_this_in_sfc; pub mod no_unescaped_entities; pub mod no_unknown_property; pub mod no_unsafe; @@ -1098,6 +1099,7 @@ oxc_macros::declare_all_lint_rules! { react::no_render_return_value, react::no_set_state, react::no_string_refs, + react::no_this_in_sfc, react::no_unescaped_entities, react::no_unknown_property, react::no_unsafe, diff --git a/crates/oxc_linter/src/rules/react/no_this_in_sfc.rs b/crates/oxc_linter/src/rules/react/no_this_in_sfc.rs new file mode 100644 index 0000000000000..e52c28fc73963 --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_this_in_sfc.rs @@ -0,0 +1,434 @@ +use oxc_ast::{AstKind, ast::BindingPattern}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{ + AstNode, + context::LintContext, + rule::Rule, + utils::{is_es5_component, is_es6_component, is_react_component_name}, +}; + +fn no_this_in_sfc_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Stateless functional components should not use `this`") + .with_help("Use props and context directly as function parameters instead of accessing them through `this`") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoThisInSfc; + +declare_oxc_lint!( + /// ### What it does + /// + /// Prevents using `this` in stateless functional components. + /// + /// ### Why is this bad? + /// + /// In React, stateless functional components (SFCs) receive props and context as function parameters, + /// not through `this`. Using `this` in an SFC typically indicates a mistake when converting from + /// class components or unfamiliarity with the two component styles. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```jsx + /// function Foo(props) { + /// return
{this.props.bar}
; + /// } + /// + /// function Foo(props) { + /// const { bar } = this.props; + /// return
{bar}
; + /// } + /// + /// const Foo = (props) => this.props.foo ? {props.bar} : null; + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```jsx + /// function Foo(props) { + /// return
{props.bar}
; + /// } + /// + /// function Foo({ bar }) { + /// return
{bar}
; + /// } + /// + /// class Foo extends React.Component { + /// render() { + /// return
{this.props.bar}
; + /// } + /// } + /// ``` + NoThisInSfc, + react, + correctness +); + +impl Rule for NoThisInSfc { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::ThisExpression(this_expr) = node.kind() else { return }; + + let Some(component_node) = get_parent_function(node, ctx) else { return }; + + if ctx + .nodes() + .ancestors(component_node.id()) + .any(|ancestor| is_es6_component(ancestor) || is_es5_component(ancestor)) + { + return; + } + + if is_in_nested_this_context(node, component_node, ctx) { + return; + } + + if !is_potential_react_component(component_node, ctx) { + return; + } + + ctx.diagnostic(no_this_in_sfc_diagnostic(this_expr.span)); + } + + fn should_run(&self, ctx: &crate::context::ContextHost) -> bool { + ctx.source_type().is_jsx() + } +} + +fn get_parent_function<'a, 'b>( + node: &'b AstNode<'a>, + ctx: &'b LintContext<'a>, +) -> Option<&'b AstNode<'a>> { + ctx.nodes().ancestors(node.id()).find(|ancestor| { + matches!(ancestor.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) + }) +} + +fn is_in_nested_this_context<'a>( + this_node: &AstNode<'a>, + component_node: &AstNode<'a>, + ctx: &LintContext<'a>, +) -> bool { + ctx.nodes() + .ancestors(this_node.id()) + .take_while(|ancestor| ancestor.id() != component_node.id()) + .any(|ancestor| match ancestor.kind() { + AstKind::Function(_) + | AstKind::MethodDefinition(_) + | AstKind::PropertyDefinition(_) => true, + AstKind::ObjectProperty(_) => { + matches!(ctx.nodes().parent_kind(ancestor.id()), AstKind::ObjectExpression(_)) + } + + _ => false, + }) +} + +fn is_potential_react_component<'a>(function_node: &AstNode<'a>, ctx: &LintContext<'a>) -> bool { + let function_name = get_function_name(function_node, ctx); + + if let Some(name) = function_name + && is_react_component_name(&name) + { + return true; + } + + false +} + +fn get_function_name<'a>(function_node: &AstNode<'a>, ctx: &LintContext<'a>) -> Option { + match function_node.kind() { + AstKind::Function(func) => func.id.as_ref().map(|id| id.name.to_string()), + AstKind::ArrowFunctionExpression(_) => { + let parent = ctx.nodes().parent_node(function_node.id()); + if let AstKind::VariableDeclarator(declarator) = parent.kind() + && let BindingPattern::BindingIdentifier(ident) = &declarator.id + { + return Some(ident.name.to_string()); + } + None + } + _ => None, + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + " + function Foo(props) { + const { foo } = props; + return
; + } + ", + None, + None, + ), + ( + " + function Foo({ foo }) { + return
; + } + ", + None, + None, + ), + ( + " + class Foo extends React.Component { + render() { + const { foo } = this.props; + return
; + } + } + ", + None, + None, + ), + ( + " + const Foo = createReactClass({ + render: function() { + return
{this.props.foo}
; + } + }); + ", + None, + None, + ), + ( + " + const Foo = React.createClass({ + render: function() { + return
{this.props.foo}
; + } + }); + ", + None, + Some(serde_json::json!({ "settings": { "react": { "createClass": "createClass" } } })), + ), + ( + " + function foo(bar) { + this.bar = bar; + this.props = 'baz'; + this.getFoo = function() { + return this.bar + this.props; + } + } + ", + None, + None, + ), + ( + " + function Foo(props) { + return props.foo ? {props.bar} : null; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + if (props.foo) { + return
{props.bar}
; + } + return null; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + if (props.foo) { + something(); + } + return null; + } + ", + None, + None, + ), + ("const Foo = (props) => {props.foo}", None, None), + ("const Foo = ({ foo }) => {foo}", None, None), + ("const Foo = (props) => props.foo ? {props.bar} : null;", None, None), + ("const Foo = ({ foo, bar }) => foo ? {bar} : null;", None, None), + ( + " + class Foo { + bar() { + () => { + this.something(); + return null; + }; + } + } + ", + None, + None, + ), + ( + " + class Foo { + bar = () => { + this.something(); + return null; + }; + } + ", + None, + None, + ), + ( + " + export const Example = ({ prop }) => { + return { + handleClick: () => {}, + renderNode() { + return
; + }, + }; + }; + ", + None, + None, + ), + ( + r#" + export const prepareLogin = new ValidatedMethod({ + name: "user.prepare", + validate: new SimpleSchema({ + }).validator(), + run({ remember }) { + if (Meteor.isServer) { + const connectionId = this.connection.id; // react/no-this-in-sfc + return Methods.prepareLogin(connectionId, remember); + } + return null; + }, + }); + "#, + None, + None, + ), + ( + " + obj.notAComponent = function () { + return this.a || null; + }; + ", + None, + None, + ), + ( + " + $.fn.getValueAsStringWeak = function (): string | null { + const val = this.length === 1 ? this.val() : null; + + return typeof val === 'string' ? val : null; + }; + ", + None, + None, + ), + ]; + + let fail = vec![ + ( + " + function Foo(props) { + const { foo } = this.props; + return
{foo}
; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + return
{this.props.foo}
; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + return
{this.state.foo}
; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + const { foo } = this.state; + return
{foo}
; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + return props.foo ?
{this.props.bar}
: null; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + if (props.foo) { + return
{this.props.bar}
; + } + return null; + } + ", + None, + None, + ), + ( + " + function Foo(props) { + if (this.props.foo) { + something(); + } + return null; + } + ", + None, + None, + ), + ("const Foo = (props) => {this.props.foo}", None, None), + ("const Foo = (props) => this.props.foo ? {props.bar} : null;", None, None), + ( + " + function Foo(props) { + function onClick(bar) { + this.props.onClick(); + } + return
{this.props.foo}
; + } + ", + None, + None, + ), + ]; + + Tester::new(NoThisInSfc::NAME, NoThisInSfc::PLUGIN, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/react_no_this_in_sfc.snap b/crates/oxc_linter/src/snapshots/react_no_this_in_sfc.snap new file mode 100644 index 0000000000000..03dc0041382cc --- /dev/null +++ b/crates/oxc_linter/src/snapshots/react_no_this_in_sfc.snap @@ -0,0 +1,88 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:30] + 2 │ function Foo(props) { + 3 │ const { foo } = this.props; + · ──── + 4 │ return
{foo}
; + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:27] + 2 │ function Foo(props) { + 3 │ return
{this.props.foo}
; + · ──── + 4 │ } + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:27] + 2 │ function Foo(props) { + 3 │ return
{this.state.foo}
; + · ──── + 4 │ } + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:30] + 2 │ function Foo(props) { + 3 │ const { foo } = this.state; + · ──── + 4 │ return
{foo}
; + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:39] + 2 │ function Foo(props) { + 3 │ return props.foo ?
{this.props.bar}
: null; + · ──── + 4 │ } + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:4:29] + 3 │ if (props.foo) { + 4 │ return
{this.props.bar}
; + · ──── + 5 │ } + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:3:18] + 2 │ function Foo(props) { + 3 │ if (this.props.foo) { + · ──── + 4 │ something(); + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:1:31] + 1 │ const Foo = (props) => {this.props.foo} + · ──── + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:1:24] + 1 │ const Foo = (props) => this.props.foo ? {props.bar} : null; + · ──── + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this` + + ⚠ eslint-plugin-react(no-this-in-sfc): Stateless functional components should not use `this` + ╭─[no_this_in_sfc.tsx:6:45] + 5 │ } + 6 │ return
{this.props.foo}
; + · ──── + 7 │ } + ╰──── + help: Use props and context directly as function parameters instead of accessing them through `this`