From 37d3df9e02cf16a408e84dde71a4dc64a5c910bd Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Fri, 29 Nov 2024 10:05:57 +0100 Subject: [PATCH] feat: add jsx-props-no-spread-multi rule (#1358) * feat: add jsx-props-no-spread-multi rule * fix: update schemas * chore: add recommended --- docs/rules/jsx_props_no_spread_multi.md | 18 +++ schemas/rules.v1.json | 1 + src/rules.rs | 2 + src/rules/jsx_props_no_spread_multi.rs | 139 ++++++++++++++++++++++++ www/static/docs.json | 9 ++ 5 files changed, 169 insertions(+) create mode 100644 docs/rules/jsx_props_no_spread_multi.md create mode 100644 src/rules/jsx_props_no_spread_multi.rs diff --git a/docs/rules/jsx_props_no_spread_multi.md b/docs/rules/jsx_props_no_spread_multi.md new file mode 100644 index 00000000..86d201ca --- /dev/null +++ b/docs/rules/jsx_props_no_spread_multi.md @@ -0,0 +1,18 @@ +Spreading the same expression twice is typically a mistake and causes +unnecessary computations. + +### Invalid: + +```tsx +
+
+ +``` + +### Valid: + +```tsx +
+
+ +``` diff --git a/schemas/rules.v1.json b/schemas/rules.v1.json index 377856b0..8ab44df1 100644 --- a/schemas/rules.v1.json +++ b/schemas/rules.v1.json @@ -19,6 +19,7 @@ "fresh-server-event-handlers", "getter-return", "guard-for-in", + "jsx-props-no-spread-multi", "no-array-constructor", "no-async-promise-executor", "no-await-in-loop", diff --git a/src/rules.rs b/src/rules.rs index 928a7d0c..6986a120 100644 --- a/src/rules.rs +++ b/src/rules.rs @@ -24,6 +24,7 @@ pub mod fresh_handler_export; pub mod fresh_server_event_handlers; pub mod getter_return; pub mod guard_for_in; +pub mod jsx_props_no_spread_multi; pub mod no_array_constructor; pub mod no_async_promise_executor; pub mod no_await_in_loop; @@ -256,6 +257,7 @@ fn get_all_rules_raw() -> Vec> { Box::new(fresh_server_event_handlers::FreshServerEventHandlers), Box::new(getter_return::GetterReturn), Box::new(guard_for_in::GuardForIn), + Box::new(jsx_props_no_spread_multi::JSXPropsNoSpreadMulti), Box::new(no_array_constructor::NoArrayConstructor), Box::new(no_async_promise_executor::NoAsyncPromiseExecutor), Box::new(no_await_in_loop::NoAwaitInLoop), diff --git a/src/rules/jsx_props_no_spread_multi.rs b/src/rules/jsx_props_no_spread_multi.rs new file mode 100644 index 00000000..3c5bf654 --- /dev/null +++ b/src/rules/jsx_props_no_spread_multi.rs @@ -0,0 +1,139 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::collections::HashSet; + +use super::{Context, LintRule}; +use crate::diagnostic::{LintFix, LintFixChange}; +use crate::handler::{Handler, Traverse}; +use crate::Program; +use deno_ast::view::{JSXAttrOrSpread, JSXOpeningElement, NodeTrait}; +use deno_ast::{SourceRange, SourceRanged}; + +#[derive(Debug)] +pub struct JSXPropsNoSpreadMulti; + +const CODE: &str = "jsx-props-no-spread-multi"; + +impl LintRule for JSXPropsNoSpreadMulti { + fn tags(&self) -> &'static [&'static str] { + &["recommended", "react", "jsx"] + } + + fn code(&self) -> &'static str { + CODE + } + + fn lint_program_with_ast_view( + &self, + context: &mut Context, + program: Program, + ) { + JSXPropsNoSpreadMultiHandler.traverse(program, context); + } + + #[cfg(feature = "docs")] + fn docs(&self) -> &'static str { + include_str!("../../docs/rules/jsx_props_no_spread_multi.md") + } +} + +const MESSAGE: &str = "Duplicate spread attribute found"; +const HINT: &str = "Remove this spread attribute"; + +struct JSXPropsNoSpreadMultiHandler; + +impl Handler for JSXPropsNoSpreadMultiHandler { + fn jsx_opening_element( + &mut self, + node: &JSXOpeningElement, + ctx: &mut Context, + ) { + let mut seen: HashSet<&str> = HashSet::new(); + for attr in node.attrs { + if let JSXAttrOrSpread::SpreadElement(spread) = attr { + let text = spread.expr.text(); + if seen.contains(text) { + ctx.add_diagnostic_with_fixes( + spread.range(), + CODE, + MESSAGE, + Some(HINT.to_string()), + vec![LintFix { + description: "Remove this spread attribute".into(), + changes: vec![LintFixChange { + new_text: "".into(), + range: SourceRange { + start: attr.range().start - 2, + end: attr.range().end + 1, + }, + }], + }], + ); + } + + seen.insert(text); + } + } + } +} + +// most tests are taken from ESlint, commenting those +// requiring code path support +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn jsx_props_no_spread_multi_valid() { + assert_lint_ok! { + JSXPropsNoSpreadMulti, + filename: "file:///foo.jsx", + r#"
"#, + r#"
"#, + r#""#, + r#""#, + r#""#, + }; + } + + #[test] + fn jsx_props_no_spread_multi_invalid() { + assert_lint_err! { + JSXPropsNoSpreadMulti, + filename: "file:///foo.jsx", + r#"
"#: [ + { + col: 15, + message: MESSAGE, + hint: HINT, + fix: ( + "Remove this spread attribute", + "
" + ) + } + ], + r#""#: [ + { + col: 15, + message: MESSAGE, + hint: HINT, + fix: ( + "Remove this spread attribute", + "" + ) + } + ], + r#"
"#: [ + { + col: 25, + message: MESSAGE, + hint: HINT, + fix: ( + "Remove this spread attribute", + "
" + ) + } + ], + }; + } +} diff --git a/www/static/docs.json b/www/static/docs.json index ace6916a..c68cb6b8 100644 --- a/www/static/docs.json +++ b/www/static/docs.json @@ -111,6 +111,15 @@ "docs": "Require `for-in` loops to include an `if` statement\n\nLooping over objects with a `for-in` loop will include properties that are\ninherited through the prototype chain. This behavior can lead to unexpected\nitems in your for loop.\n\n### Invalid:\n\n```typescript\nfor (const key in obj) {\n foo(obj, key);\n}\n```\n\n### Valid:\n\n```typescript\nfor (const key in obj) {\n if (Object.hasOwn(obj, key)) {\n foo(obj, key);\n }\n}\n```\n\n```typescript\nfor (const key in obj) {\n if (!Object.hasOwn(obj, key)) {\n continue;\n }\n foo(obj, key);\n}\n```\n", "tags": [] }, + { + "code": "jsx-props-no-spread-multi", + "docs": "Spreading the same expression twice is typically a mistake and causes\nunnecessary computations.\n\n### Invalid:\n\n```tsx\n
\n
\n\n```\n\n### Valid:\n\n```tsx\n
\n
\n\n```\n", + "tags": [ + "recommended", + "react", + "jsx" + ] + }, { "code": "no-array-constructor", "docs": "Enforce conventional usage of array construction\n\nArray construction is conventionally done via literal notation such as `[]` or\n`[1, 2, 3]`. Using the `new Array()` is discouraged as is `new Array(1, 2, 3)`.\nThere are two reasons for this. The first is that a single supplied argument\ndefines the array length, while multiple arguments instead populate the array of\nno fixed size. This confusion is avoided when pre-populated arrays are only\ncreated using literal notation. The second argument to avoiding the `Array`\nconstructor is that the `Array` global may be redefined.\n\nThe one exception to this rule is when creating a new array of fixed size, e.g.\n`new Array(6)`. This is the conventional way to create arrays of fixed length.\n\n### Invalid:\n\n```typescript\n// This is 4 elements, not a size 100 array of 3 elements\nconst a = new Array(100, 1, 2, 3);\n\nconst b = new Array(); // use [] instead\n```\n\n### Valid:\n\n```typescript\nconst a = new Array(100);\nconst b = [];\nconst c = [1, 2, 3];\n```\n",