From 53ffbc6bd2fa730eb5fcf625a2158f8f4dd9fd56 Mon Sep 17 00:00:00 2001 From: keita hino Date: Wed, 20 Mar 2024 16:38:48 +0900 Subject: [PATCH] feat(linter): eslint-plugin-react checked-requires-onchange-or-readonly (#2754) partof: https://github.com/oxc-project/oxc/issues/1022 docs: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/checked-requires-onchange-or-readonly.md code: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/checked-requires-onchange-or-readonly.js test: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/tests/lib/rules/checked-requires-onchange-or-readonly.js --------- Co-authored-by: Dunqing --- crates/oxc_linter/src/rules.rs | 2 + .../checked_requires_onchange_or_readonly.rs | 292 ++++++++++++++++++ ...checked_requires_onchange_or_readonly.snap | 101 ++++++ 3 files changed, 395 insertions(+) create mode 100644 crates/oxc_linter/src/rules/react/checked_requires_onchange_or_readonly.rs create mode 100644 crates/oxc_linter/src/snapshots/checked_requires_onchange_or_readonly.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 872a64c7ce3d5..d1c8a2b7ca485 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -180,6 +180,7 @@ mod jest { mod react { pub mod button_has_type; + pub mod checked_requires_onchange_or_readonly; pub mod jsx_key; pub mod jsx_no_comment_textnodes; pub mod jsx_no_duplicate_props; @@ -572,6 +573,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::text_encoding_identifier_case, unicorn::throw_new_error, react::button_has_type, + react::checked_requires_onchange_or_readonly, react::jsx_no_target_blank, react::jsx_key, react::jsx_no_comment_textnodes, diff --git a/crates/oxc_linter/src/rules/react/checked_requires_onchange_or_readonly.rs b/crates/oxc_linter/src/rules/react/checked_requires_onchange_or_readonly.rs new file mode 100644 index 0000000000000..227a2c4602cac --- /dev/null +++ b/crates/oxc_linter/src/rules/react/checked_requires_onchange_or_readonly.rs @@ -0,0 +1,292 @@ +use oxc_ast::{ + ast::{Argument, Expression, JSXAttributeItem, ObjectPropertyKind}, + 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, + utils::{get_element_type, get_jsx_attribute_name, is_create_element_call}, + AstNode, +}; + +#[derive(Debug, Error, Diagnostic)] +enum CheckedRequiresOnchangeOrReadonlyDiagnostic { + #[error("eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`.")] + #[diagnostic(severity(warning), help("Add either `onChange` or `readOnly`."))] + MissingProperty(#[label] Span), + + #[error("eslint-plugin-react(checked-requires-onchange-or-readonly): Use either `checked` or `defaultChecked`, but not both.")] + #[diagnostic(severity(warning), help("Remove either `checked` or `defaultChecked`."))] + ExclusiveCheckedAttribute(#[label] Span, #[label] Span), +} + +#[derive(Debug, Default, Clone)] +pub struct CheckedRequiresOnchangeOrReadonly { + ignore_missing_properties: bool, + ignore_exclusive_checked_attribute: bool, +} + +declare_oxc_lint!( + /// ### What it does + /// This rule enforces onChange or readonly attribute for checked property of input elements. + /// It also warns when checked and defaultChecked properties are used together. + /// + /// ### Example + /// ```javascript + /// // Bad + /// + /// + /// + /// + /// React.createElement('input', { checked: false }); + /// React.createElement('input', { type: 'checkbox', checked: true }); + /// React.createElement('input', { type: 'checkbox', checked: true, defaultChecked: true }); + /// + /// // Good + /// {}} /> + /// + /// + /// + /// + /// React.createElement('input', { type: 'checkbox', checked: true, onChange() {} }); + /// React.createElement('input', { type: 'checkbox', checked: true, readOnly: true }); + /// React.createElement('input', { type: 'checkbox', checked: true, onChange() {}, readOnly: true }); + /// React.createElement('input', { type: 'checkbox', defaultChecked: true }); + /// ``` + CheckedRequiresOnchangeOrReadonly, + correctness +); + +impl Rule for CheckedRequiresOnchangeOrReadonly { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::JSXOpeningElement(jsx_opening_el) => { + let Some(element_type) = get_element_type(ctx, jsx_opening_el) else { return }; + if element_type != "input" { + return; + } + + let (checked_span, default_checked_span, is_missing_property) = + jsx_opening_el.attributes.iter().fold( + (None, None, true), + |(checked_span, default_checked_span, is_missing_property), attr| { + if let JSXAttributeItem::Attribute(jsx_attr) = attr { + let name = get_jsx_attribute_name(&jsx_attr.name); + ( + if name == "checked" { + Some(jsx_attr.span) + } else { + checked_span + }, + if default_checked_span.is_none() && name == "defaultChecked" { + Some(jsx_attr.span) + } else { + default_checked_span + }, + is_missing_property + && !(name == "onChange" || name == "readOnly"), + ) + } else { + (checked_span, default_checked_span, is_missing_property) + } + }, + ); + + if let Some(checked_span) = checked_span { + if !self.ignore_exclusive_checked_attribute { + if let Some(default_checked_span) = default_checked_span { + ctx.diagnostic( + CheckedRequiresOnchangeOrReadonlyDiagnostic::ExclusiveCheckedAttribute( + checked_span, + default_checked_span, + ), + ); + } + } + + if !self.ignore_missing_properties && is_missing_property { + ctx.diagnostic( + CheckedRequiresOnchangeOrReadonlyDiagnostic::MissingProperty( + checked_span, + ), + ); + } + } + } + AstKind::CallExpression(call_expr) => { + if !is_create_element_call(call_expr) { + return; + } + + let Some(Argument::Expression(Expression::StringLiteral(element_name))) = + call_expr.arguments.first() + else { + return; + }; + + if element_name.value != "input" { + return; + } + + let Some(Argument::Expression(Expression::ObjectExpression(obj_expr))) = + call_expr.arguments.get(1) + else { + return; + }; + + let (checked_span, default_checked_span, is_missing_property) = + obj_expr.properties.iter().fold( + (None, None, true), + |(checked_span, default_checked_span, is_missing_property), prop| { + if let ObjectPropertyKind::ObjectProperty(object_prop) = prop { + if let Some(name) = object_prop.key.static_name() { + ( + if checked_span.is_none() && name == "checked" { + Some(object_prop.span) + } else { + checked_span + }, + if default_checked_span.is_none() + && name == "defaultChecked" + { + Some(object_prop.span) + } else { + default_checked_span + }, + is_missing_property + && !(name == "onChange" || name == "readOnly"), + ) + } else { + (checked_span, default_checked_span, is_missing_property) + } + } else { + (checked_span, default_checked_span, is_missing_property) + } + }, + ); + + if let Some(checked_span) = checked_span { + if !self.ignore_exclusive_checked_attribute { + if let Some(default_checked_span) = default_checked_span { + ctx.diagnostic( + CheckedRequiresOnchangeOrReadonlyDiagnostic::ExclusiveCheckedAttribute( + checked_span, + default_checked_span, + ), + ); + } + } + + if !self.ignore_missing_properties && is_missing_property { + ctx.diagnostic( + CheckedRequiresOnchangeOrReadonlyDiagnostic::MissingProperty( + checked_span, + ), + ); + } + } + } + _ => {} + } + } + + fn from_configuration(value: serde_json::Value) -> Self { + let value = value.as_array().and_then(|arr| arr.first()).and_then(|val| val.as_object()); + + Self { + ignore_missing_properties: value + .and_then(|val| { + val.get("ignoreMissingProperties").and_then(serde_json::Value::as_bool) + }) + .unwrap_or(false), + ignore_exclusive_checked_attribute: value + .and_then(|val| { + val.get("ignoreExclusiveCheckedAttribute").and_then(serde_json::Value::as_bool) + }) + .unwrap_or(false), + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"React.createElement('input')", None), + (r"React.createElement('input', { checked: true, onChange: noop })", None), + (r"React.createElement('input', { checked: false, onChange: noop })", None), + (r"React.createElement('input', { checked: true, readOnly: true })", None), + (r"React.createElement('input', { checked: true, onChange: noop, readOnly: true })", None), + (r"React.createElement('input', { checked: foo, onChange: noop, readOnly: true })", None), + ( + r"", + Some(serde_json::json!([{ "ignoreMissingProperties": true }])), + ), + ( + r"", + Some(serde_json::json!([{ "ignoreMissingProperties": true }])), + ), + ( + r"", + Some(serde_json::json!([{ "ignoreExclusiveCheckedAttribute": true }])), + ), + ( + r"", + Some(serde_json::json!([{ "ignoreExclusiveCheckedAttribute": true }])), + ), + ( + r"", + Some( + serde_json::json!([{ "ignoreMissingProperties": true, "ignoreExclusiveCheckedAttribute": true }]), + ), + ), + (r"", None), + (r"React.createElement('span')", None), + (r"(()=>{})()", None), + ]; + + let fail = vec![ + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"", None), + (r"React.createElement('input', { checked: false })", None), + (r"React.createElement('input', { checked: true, defaultChecked: true })", None), + ( + r"", + Some(serde_json::json!([{ "ignoreMissingProperties": true }])), + ), + ( + r"", + Some(serde_json::json!([{ "ignoreExclusiveCheckedAttribute": true }])), + ), + ( + r"", + Some( + serde_json::json!([{ "ignoreMissingProperties": false, "ignoreExclusiveCheckedAttribute": false }]), + ), + ), + ]; + + Tester::new(CheckedRequiresOnchangeOrReadonly::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/checked_requires_onchange_or_readonly.snap b/crates/oxc_linter/src/snapshots/checked_requires_onchange_or_readonly.snap new file mode 100644 index 0000000000000..1fa82834fa165 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/checked_requires_onchange_or_readonly.snap @@ -0,0 +1,101 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: checked_requires_onchange_or_readonly +--- + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:21] + 1 │ + · ─────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:21] + 1 │ + · ────────────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ────────────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ────────────────────────────────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): Use either `checked` or `defaultChecked`, but not both. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── ────────────── + ╰──── + help: Remove either `checked` or `defaultChecked`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:32] + 1 │ React.createElement('input', { checked: false }) + · ────────────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): Use either `checked` or `defaultChecked`, but not both. + ╭─[checked_requires_onchange_or_readonly.tsx:1:32] + 1 │ React.createElement('input', { checked: true, defaultChecked: true }) + · ───────────── ──────────────────── + ╰──── + help: Remove either `checked` or `defaultChecked`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:32] + 1 │ React.createElement('input', { checked: true, defaultChecked: true }) + · ───────────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): Use either `checked` or `defaultChecked`, but not both. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── ────────────── + ╰──── + help: Remove either `checked` or `defaultChecked`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── + ╰──── + help: Add either `onChange` or `readOnly`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): Use either `checked` or `defaultChecked`, but not both. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── ────────────── + ╰──── + help: Remove either `checked` or `defaultChecked`. + + ⚠ eslint-plugin-react(checked-requires-onchange-or-readonly): `checked` should be used with either `onChange` or `readOnly`. + ╭─[checked_requires_onchange_or_readonly.tsx:1:24] + 1 │ + · ─────── + ╰──── + help: Add either `onChange` or `readOnly`.