diff --git a/crates/oxc_ast/src/ast_kind.rs b/crates/oxc_ast/src/ast_kind.rs index 5365fd4263d6d..739c509dee87e 100644 --- a/crates/oxc_ast/src/ast_kind.rs +++ b/crates/oxc_ast/src/ast_kind.rs @@ -110,6 +110,7 @@ pub enum AstKind<'a> { // JSX // Please make sure to add these to `is_jsx` below. JSXElement(&'a JSXElement<'a>), + JSXFragment(&'a JSXFragment<'a>), JSXOpeningElement(&'a JSXOpeningElement<'a>), JSXElementName(&'a JSXElementName<'a>), @@ -231,7 +232,13 @@ impl<'a> AstKind<'a> { } pub fn is_jsx(self) -> bool { - matches!(self, Self::JSXElement(_) | Self::JSXOpeningElement(_) | Self::JSXElementName(_)) + matches!( + self, + Self::JSXElement(_) + | Self::JSXOpeningElement(_) + | Self::JSXElementName(_) + | Self::JSXFragment(_) + ) } pub fn is_specific_id_reference(&self, name: &str) -> bool { @@ -350,6 +357,7 @@ impl<'a> GetSpan for AstKind<'a> { Self::JSXOpeningElement(x) => x.span, Self::JSXElementName(x) => x.span(), Self::JSXElement(x) => x.span, + Self::JSXFragment(x) => x.span, Self::TSModuleBlock(x) => x.span, @@ -513,6 +521,7 @@ impl<'a> AstKind<'a> { Self::JSXOpeningElement(_) => "JSXOpeningElement".into(), Self::JSXElementName(_) => "JSXElementName".into(), Self::JSXElement(_) => "JSXElement".into(), + Self::JSXFragment(_) => "JSXFragment".into(), Self::TSModuleBlock(_) => "TSModuleBlock".into(), diff --git a/crates/oxc_ast/src/visit.rs b/crates/oxc_ast/src/visit.rs index a0178474dc5d1..3635cd232c4c1 100644 --- a/crates/oxc_ast/src/visit.rs +++ b/crates/oxc_ast/src/visit.rs @@ -1077,9 +1077,12 @@ pub trait Visit<'a>: Sized { } fn visit_jsx_fragment(&mut self, elem: &JSXFragment<'a>) { + let kind = AstKind::JSXFragment(self.alloc(elem)); + self.enter_node(kind); for child in &elem.children { self.visit_jsx_child(child); } + self.leave_node(kind); } fn visit_jsx_child(&mut self, child: &JSXChild<'a>) { diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 6e87f01d3f60f..ba4dacd378ba5 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -123,6 +123,10 @@ mod jest { pub mod valid_title; } +mod react { + pub mod jsx_key; +} + mod unicorn { pub mod catch_error_name; pub mod error_message; @@ -243,6 +247,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::no_thenable, unicorn::throw_new_error, unicorn::prefer_array_flat_map, + react::jsx_key, import::named, import::no_cycle, import::no_self_import, diff --git a/crates/oxc_linter/src/rules/react/jsx_key.rs b/crates/oxc_linter/src/rules/react/jsx_key.rs new file mode 100644 index 0000000000000..3ea13e887aad6 --- /dev/null +++ b/crates/oxc_linter/src/rules/react/jsx_key.rs @@ -0,0 +1,384 @@ +use oxc_ast::{ + ast::{Expression, JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +enum JsxKeyDiagnostic { + #[error("eslint-plugin-react/jsx-key: Missing \"key\" prop for element in array.")] + #[diagnostic(severity(warning), help("Missing \"key\" prop for element in array."))] + MissingKeyPropForElementInArray(#[label] Span), + #[error("eslint-plugin-react/jsx-key: Missing \"key\" prop for element in iterator.")] + #[diagnostic(severity(warning), help("Missing \"key\" prop for element in iterator."))] + MissingKeyPropForElementInIterator(#[label] Span), + #[error(r#"eslint-plugin-react/jsx-key: \"key\" prop must be placed before any `{{...spread}}`, to avoid conflicting with React's new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html"#)] + #[diagnostic(severity(warning), help("Missing \"key\" prop for element in iterator."))] + KeyPropMustBePlacedBeforeSpread(#[label] Span), +} + +#[derive(Debug, Default, Clone)] +pub struct JsxKey; + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce `key` prop for elements in array + /// + /// ### Example + /// ```javascript + /// // Bad + /// [1, 2, 3].map(x => ); + /// [1, 2, 3]?.map(x => ) + /// + /// // Good + /// [1, 2, 3].map(x => ); + /// [1, 2, 3]?.map(x => ) + /// ``` + JsxKey, + correctness +); + +impl Rule for JsxKey { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::JSXElement(jsx_elem) => { + check_jsx_element(node, jsx_elem, ctx); + check_jsx_element_is_key_before_spread(jsx_elem, ctx); + } + AstKind::JSXFragment(jsx_frag) => { + check_jsx_fragment(node, jsx_frag, ctx); + } + + _ => {} + } + } +} + +enum InsideArrayOrIterator { + Array, + Iterator, +} + +fn is_in_array_or_iter<'a, 'b>( + node: &'b AstNode<'a>, + ctx: &'b LintContext<'a>, +) -> Option { + let mut node = node; + + loop { + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return None; + }; + + match parent.kind() { + AstKind::ArrayExpression(_) => return Some(InsideArrayOrIterator::Array), + AstKind::CallExpression(v) => { + let callee = &v.callee.without_parenthesized(); + + if let Expression::MemberExpression(v) = callee { + if let Some(static_property_name) = v.static_property_name() { + if TARGET_METHODS.contains(static_property_name) { + return Some(InsideArrayOrIterator::Iterator); + } + } + } + + return None; + } + AstKind::JSXElement(_) | AstKind::JSXOpeningElement(_) => return None, + _ => node = parent, + } + } +} + +fn check_jsx_element<'a>(node: &AstNode<'a>, jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) { + if let Some(outer) = is_in_array_or_iter(node, ctx) { + if !jsx_elem.opening_element.attributes.iter().any(|attr| { + let JSXAttributeItem::Attribute(attr) = attr else { return false }; + + let JSXAttributeName::Identifier(attr_ident) = &attr.name else { + return false; + }; + attr_ident.name == "key" + }) { + ctx.diagnostic(gen_diagnostic(jsx_elem.span, &outer)); + } + } +} + +fn check_jsx_element_is_key_before_spread<'a>(jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) { + let mut key_idx: Option = None; + let mut spread_idx: Option = None; + + for (i, attr) in jsx_elem.opening_element.attributes.iter().enumerate() { + match attr { + JSXAttributeItem::Attribute(attr) => { + let JSXAttributeName::Identifier(ident) = &attr.name else { continue }; + if ident.name == "key" { + key_idx = Some(i); + } + } + JSXAttributeItem::SpreadAttribute(_) => spread_idx = Some(i), + } + if key_idx.is_some() && spread_idx.is_some() { + break; + } + } + + if let (Some(key_idx), Some(spread_idx)) = (key_idx, spread_idx) { + if key_idx > spread_idx { + ctx.diagnostic(JsxKeyDiagnostic::KeyPropMustBePlacedBeforeSpread(jsx_elem.span)); + } + } +} + +fn check_jsx_fragment<'a>(node: &AstNode<'a>, fragment: &JSXFragment<'a>, ctx: &LintContext<'a>) { + if let Some(outer) = is_in_array_or_iter(node, ctx) { + ctx.diagnostic(gen_diagnostic(fragment.span, &outer)); + } +} + +fn gen_diagnostic(span: Span, outer: &InsideArrayOrIterator) -> JsxKeyDiagnostic { + match outer { + InsideArrayOrIterator::Array => JsxKeyDiagnostic::MissingKeyPropForElementInArray(span), + InsideArrayOrIterator::Iterator => { + JsxKeyDiagnostic::MissingKeyPropForElementInIterator(span) + } + } +} + +const TARGET_METHODS: phf::Set<&'static str> = phf::phf_set! { + // .map(() => ) + "map", + // .map(() => ) + "flatMap", + // Array.from(, () => ) + "from" +}; + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + (r#"fn()"#, None), + (r#"[1, 2, 3].map(function () {})"#, None), + (r#";"#, None), + (r#"[, ];"#, None), + (r#"[1, 2, 3].map(function(x) { return });"#, None), + (r#"[1, 2, 3].map(x => );"#, None), + (r#"[1, 2 ,3].map(x => x && );"#, None), + (r#"[1, 2 ,3].map(x => x ? : );"#, None), + (r#"[1, 2, 3].map(x => { return });"#, None), + (r#"Array.from([1, 2, 3], function(x) { return });"#, None), + (r#"Array.from([1, 2, 3], (x => ));"#, None), + (r#"Array.from([1, 2, 3], (x => {return }));"#, None), + (r#"Array.from([1, 2, 3], someFn);"#, None), + (r#"Array.from([1, 2, 3]);"#, None), + (r#"[1, 2, 3].foo(x => );"#, None), + (r#"var App = () => ;"#, None), + (r#"[1, 2, 3].map(function(x) { return; });"#, None), + (r#"foo(() => );"#, None), + (r#"foo(() => <>>);"#, None), + (r#"<>>;"#, None), + (r#";"#, None), + (r#";"#, None), + (r#";"#, None), + (r#"const spans = [,];"#, None), + ( + r#" + function Component(props) { + return hasPayment ? ( + + + {props.modal && props.calculatedPrice && ( + + )} + + ) : null; + } + "#, + None, + ), + ( + r#" + import React, { FC, useRef, useState } from 'react'; + + import './ResourceVideo.sass'; + import VimeoVideoPlayInModal from '../vimeoVideoPlayInModal/VimeoVideoPlayInModal'; + + type Props = { + videoUrl: string; + videoTitle: string; + }; + + const ResourceVideo: FC = ({ + videoUrl, + videoTitle, + }: Props): JSX.Element => { + return ( + + + {videoTitle} + + ); + }; + + export default ResourceVideo; + "#, + None, + ), + ( + r#" + // testrule.jsx + const trackLink = () => {}; + const getAnalyticsUiElement = () => {}; + + const onTextButtonClick = (e, item) => trackLink([, getAnalyticsUiElement(item), item.name], e); + "#, + None, + ), + ( + r#" + function Component({ allRatings }) { + return ( + + {Object.entries(allRatings)?.map(([key, value], index) => { + const rate = value?.split(/(?=[%, /])/); + + if (!rate) return null; + + return ( + + + {rate?.[0]} + {rate?.[1]} + + ); + })} + + ); + } + "#, + None, + ), + ( + r#" + const baz = foo?.bar?.()?.[1] ?? 'qux'; + + qux()?.map() + + const directiveRanges = comments?.map(tryParseTSDirective) + "#, + None, + ), + ( + r#" + import { observable } from "mobx"; + + export interface ClusterFrameInfo { + frameId: number; + processId: number; + } + + export const clusterFrameMap = observable.map(); + + "#, + None, + ), + ]; + + let fail = vec![ + (r#"[];"#, None), + (r#"[];"#, None), + (r#"[, ];"#, None), + (r#"[1, 2 ,3].map(function(x) { return });"#, None), + (r#"[1, 2 ,3].map(x => );"#, None), + (r#"[1, 2 ,3].map(x => x && );"#, None), + (r#"[1, 2 ,3].map(x => x ? : );"#, None), + (r#"[1, 2 ,3].map(x => x ? : );"#, None), + (r#"[1, 2 ,3].map(x => { return });"#, None), + (r#"Array.from([1, 2 ,3], function(x) { return });"#, None), + (r#"Array.from([1, 2 ,3], (x => { return }));"#, None), + (r#"Array.from([1, 2 ,3], (x => ));"#, None), + (r#"[1, 2, 3]?.map(x => )"#, None), + (r#"[1, 2, 3]?.map(x => )"#, None), + (r#"[1, 2, 3]?.map(x => <>>)"#, None), + ("[1, 2, 3].map(x => <>{x}>);", None), + ("[<>>];", None), + (r#"[];"#, None), + (r#"[];"#, None), + ( + r#" + const Test = () => { + const list = [1, 2, 3, 4, 5]; + + return ( + + {list.map(item => { + if (item < 2) { + return {item}; + } + + return ; + })} + + ); + }; + "#, + None, + ), + ( + r#" + const TestO = () => { + const list = [1, 2, 3, 4, 5]; + + return ( + + {list.map(item => { + if (item < 2) { + return {item}; + } else if (item < 5) { + return + } else { + return + } + + return ; + })} + + ); + }; + "#, + None, + ), + ( + r#" + const TestCase = () => { + const list = [1, 2, 3, 4, 5]; + + return ( + + {list.map(item => { + if (item < 2) return {item}; + else if (item < 5) return ; + else return ; + })} + + ); + }; + "#, + None, + ), + ]; + + Tester::new(JsxKey::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/jsx_key.snap b/crates/oxc_linter/src/snapshots/jsx_key.snap new file mode 100644 index 0000000000000..f67f44f0108fc --- /dev/null +++ b/crates/oxc_linter/src/snapshots/jsx_key.snap @@ -0,0 +1,228 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: jsx_key +--- + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in array. + ╭─[jsx_key.tsx:1:1] + 1 │ []; + · ─────── + ╰──── + help: Missing "key" prop for element in array. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in array. + ╭─[jsx_key.tsx:1:1] + 1 │ []; + · ──────────────── + ╰──── + help: Missing "key" prop for element in array. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in array. + ╭─[jsx_key.tsx:1:1] + 1 │ [, ]; + · ─────── + ╰──── + help: Missing "key" prop for element in array. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(function(x) { return }); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(x => ); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(x => x && ); + · ───────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(x => x ? : ); + · ────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(x => x ? : ); + · ───────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2 ,3].map(x => { return }); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ Array.from([1, 2 ,3], function(x) { return }); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ Array.from([1, 2 ,3], (x => { return })); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ Array.from([1, 2 ,3], (x => )); + · ─────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2, 3]?.map(x => ) + · ────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2, 3]?.map(x => ) + · ─────────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2, 3]?.map(x => <>>) + · ───────────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2, 3]?.map(x => <>>) + · ────────────────────────────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:1:1] + 1 │ [1, 2, 3].map(x => <>{x}>); + · ──────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in array. + ╭─[jsx_key.tsx:1:1] + 1 │ [<>>]; + · ───── + ╰──── + help: Missing "key" prop for element in array. + + ⚠ eslint-plugin-react/jsx-key: \"key\" prop must be placed before any `{...spread}`, to avoid conflicting with React's new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new- + │ jsx-transform.html + ╭─[jsx_key.tsx:1:1] + 1 │ []; + · ───────────────────────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: \"key\" prop must be placed before any `{...spread}`, to avoid conflicting with React's new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new- + │ jsx-transform.html + ╭─[jsx_key.tsx:1:1] + 1 │ []; + · ───────────────────────────────────── + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:8:1] + 8 │ if (item < 2) { + 9 │ return {item}; + · ───────────────── + 10 │ } + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:11:1] + 11 │ + 12 │ return ; + · ─────── + 13 │ })} + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:8:1] + 8 │ if (item < 2) { + 9 │ return {item}; + · ───────────────── + 10 │ } else if (item < 5) { + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:10:1] + 10 │ } else if (item < 5) { + 11 │ return + · ─────────── + 12 │ } else { + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:12:1] + 12 │ } else { + 13 │ return + · ─────────── + 14 │ } + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:15:1] + 15 │ + 16 │ return ; + · ─────── + 17 │ })} + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:7:1] + 7 │ {list.map(item => { + 8 │ if (item < 2) return {item}; + · ───────────────── + 9 │ else if (item < 5) return ; + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:8:1] + 8 │ if (item < 2) return {item}; + 9 │ else if (item < 5) return ; + · ─────── + 10 │ else return ; + ╰──── + help: Missing "key" prop for element in iterator. + + ⚠ eslint-plugin-react/jsx-key: Missing "key" prop for element in iterator. + ╭─[jsx_key.tsx:9:1] + 9 │ else if (item < 5) return ; + 10 │ else return ; + · ─────── + 11 │ })} + ╰──── + help: Missing "key" prop for element in iterator. + +