From 76a56db132d66fd3df38a10647e3eac9af857413 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 9 Nov 2022 16:10:34 +0100 Subject: [PATCH] fix(rome_js_analyze): Verify method name of React API calls (#3619) --- crates/rome_js_analyze/src/react.rs | 144 +++++----- .../correctness/no_array_index_key.rs | 32 +-- .../specs/correctness/noArrayIndexKey.jsx | 4 +- .../correctness/noArrayIndexKey.jsx.snap | 267 +++++++++--------- .../noChildrenProp/noChildrenPropInvalid.jsx | 7 + .../noChildrenPropInvalid.jsx.snap | 41 +-- .../noChildrenProp/noChildrenPropValid.jsx | 6 + .../noChildrenPropValid.jsx.snap | 6 + .../noUselessFragments/noChildren.jsx | 2 +- .../noUselessFragments/noChildren.jsx.snap | 2 + .../createElement.js | 2 +- .../createElement.js.snap | 26 +- .../reactCreateElement.js | 2 +- .../reactCreateElement.js.snap | 2 + .../createElement.js | 2 +- .../createElement.js.snap | 5 + 16 files changed, 294 insertions(+), 256 deletions(-) diff --git a/crates/rome_js_analyze/src/react.rs b/crates/rome_js_analyze/src/react.rs index e105e11e9e6..dbede90cd8a 100644 --- a/crates/rome_js_analyze/src/react.rs +++ b/crates/rome_js_analyze/src/react.rs @@ -2,7 +2,7 @@ pub mod hooks; -use rome_js_semantic::SemanticModel; +use rome_js_semantic::{Binding, SemanticModel}; use rome_js_syntax::{ JsAnyCallArgument, JsAnyExpression, JsAnyNamedImportSpecifier, JsCallExpression, JsIdentifierBinding, JsImport, JsImportNamedClause, JsNamedImportSpecifierList, @@ -247,46 +247,46 @@ pub(crate) fn is_react_call_api( ) -> Option { // we bail straight away if the API doesn't exists in React debug_assert!(VALID_REACT_API.contains(&api_name)); + Some(match expression { JsAnyExpression::JsStaticMemberExpression(node) => { - let object = node.object().ok()?; let member = node.member().ok()?; let member = member.as_js_name()?; + + if member.value_token().ok()?.text_trimmed() != api_name { + return Some(false); + } + + let object = node.object().ok()?; let identifier = object.as_js_identifier_expression()?.name().ok()?; - let mut maybe_from_react = identifier.syntax().text_trimmed() == "React" - && member.syntax().text_trimmed() == api_name; - - if let Some(binding_identifier) = model.declaration(&identifier) { - let binding_identifier = - JsIdentifierBinding::cast_ref(binding_identifier.syntax())?; - if let Some(js_import) = binding_identifier - .syntax() - .ancestors() - .find_map(|ancestor| JsImport::cast_ref(&ancestor)) - { - maybe_from_react = js_import.source_is("react").ok()?; + match model.declaration(&identifier) { + Some(binding) => { + let binding_identifier = JsIdentifierBinding::cast_ref(binding.syntax())?; + + if let Some(js_import) = binding_identifier + .syntax() + .ancestors() + .find_map(|ancestor| JsImport::cast_ref(&ancestor)) + { + js_import.source_is("react").ok()? + } else { + false + } } + None => identifier.has_name("React"), } - maybe_from_react } + JsAnyExpression::JsIdentifierExpression(identifier) => { let name = identifier.name().ok()?; - let mut maybe_react = identifier.syntax().text_trimmed() == api_name; - if let Some(identifier_binding) = model.declaration(&name) { - let binding_identifier = - JsIdentifierBinding::cast_ref(identifier_binding.syntax())?; - if let Some(js_import) = binding_identifier - .syntax() - .ancestors() - .find_map(|ancestor| JsImport::cast_ref(&ancestor)) - { - maybe_react = js_import.source_is("react").ok()?; - } - } - maybe_react + + model + .declaration(&name) + .and_then(|binding| is_react_export(binding, api_name)) + .unwrap_or(false) } - _ => return None, + _ => false, }) } @@ -303,24 +303,25 @@ pub(crate) fn jsx_member_name_is_react_fragment( let object = member_name.object().ok()?; let member = member_name.member().ok()?; let object = object.as_jsx_reference_identifier()?; - let mut maybe_react_fragment = object.value_token().ok()?.text_trimmed() == "React" - && member.value_token().ok()?.text_trimmed() == "Fragment"; - if let Some(reference) = model.declaration(object) { - if let Some(js_import) = reference - .syntax() - .ancestors() - .find_map(|ancestor| JsImport::cast_ref(&ancestor)) - { - let source_is_react = js_import.source_is("react").ok()?; - maybe_react_fragment = - source_is_react && member.value_token().ok()?.text_trimmed() == "Fragment"; - } else { - // `React.Fragment` is a binding but it doesn't come from the "react" package - maybe_react_fragment = false; - } + + if member.value_token().ok()?.text_trimmed() != "Fragment" { + return Some(false); } - Some(maybe_react_fragment) + match model.declaration(object) { + Some(declaration) => { + if let Some(js_import) = declaration + .syntax() + .ancestors() + .find_map(|ancestor| JsImport::cast_ref(&ancestor)) + { + js_import.source_is("react").ok() + } else { + Some(false) + } + } + None => Some(object.value_token().ok()?.text_trimmed() == "React"), + } } /// Checks if the node `JsxReferenceIdentifier` is a react fragment. @@ -334,33 +335,7 @@ pub(crate) fn jsx_reference_identifier_is_fragment( model: &SemanticModel, ) -> Option { match model.declaration(name) { - Some(reference) => { - let ident = JsIdentifierBinding::cast_ref(reference.syntax())?; - - let import_specifier = ident.parent::()?; - let name_token = match &import_specifier { - JsAnyNamedImportSpecifier::JsNamedImportSpecifier(named_import) => { - named_import.name().ok()?.value().ok()? - } - JsAnyNamedImportSpecifier::JsShorthandNamedImportSpecifier(_) => { - ident.name_token().ok()? - } - JsAnyNamedImportSpecifier::JsUnknownNamedImportSpecifier(_) => { - return None; - } - }; - - if name_token.text_trimmed() != "Fragment" { - return Some(false); - } - - let import_specifier_list = import_specifier.parent::()?; - let import_specifiers = import_specifier_list.parent::()?; - let import_clause = import_specifiers.parent::()?; - let import = import_clause.parent::()?; - import.source_is("react").ok() - } - + Some(reference) => is_react_export(reference, "Fragment"), None => { let value_token = name.value_token().ok()?; let is_fragment = value_token.text_trimmed() == "Fragment"; @@ -368,3 +343,28 @@ pub(crate) fn jsx_reference_identifier_is_fragment( } } } + +fn is_react_export(binding: Binding, name: &str) -> Option { + let ident = JsIdentifierBinding::cast_ref(binding.syntax())?; + let import_specifier = ident.parent::()?; + let name_token = match &import_specifier { + JsAnyNamedImportSpecifier::JsNamedImportSpecifier(named_import) => { + named_import.name().ok()?.value().ok()? + } + JsAnyNamedImportSpecifier::JsShorthandNamedImportSpecifier(_) => ident.name_token().ok()?, + JsAnyNamedImportSpecifier::JsUnknownNamedImportSpecifier(_) => { + return Some(false); + } + }; + + if name_token.text_trimmed() != name { + return Some(false); + } + + let import_specifier_list = import_specifier.parent::()?; + let import_specifiers = import_specifier_list.parent::()?; + let import_clause = import_specifiers.parent::()?; + let import = import_clause.parent::()?; + + import.source_is("react").ok() +} diff --git a/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_array_index_key.rs b/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_array_index_key.rs index 04ad77e429c..1ca2a3c2257 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_array_index_key.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_array_index_key.rs @@ -5,8 +5,8 @@ use rome_analyze::{declare_rule, Rule, RuleDiagnostic}; use rome_console::markup; use rome_js_semantic::SemanticModel; use rome_js_syntax::{ - JsArrowFunctionExpression, JsCallExpression, JsExpressionStatement, JsFunctionDeclaration, - JsFunctionExpression, JsIdentifierBinding, JsIdentifierExpression, JsMethodClassMember, + JsAnyCallArgument, JsArrowFunctionExpression, JsCallExpression, JsExpressionStatement, + JsFunctionDeclaration, JsFunctionExpression, JsIdentifierBinding, JsMethodClassMember, JsMethodObjectMember, JsParameterList, JsPropertyObjectMember, JsReferenceIdentifier, JsxAttribute, JsxOpeningElement, JsxSelfClosingElement, }; @@ -326,28 +326,24 @@ fn find_react_children_function_argument( "forEach" | "map" ); - let object = member_expression.object().ok()?; - - let mut is_react_children = false; - // case we have `Children` - if let Some(identifier) = JsIdentifierExpression::cast_ref(object.syntax()) { - if identifier.name().ok()?.value_token().ok()?.text_trimmed() == "Children" { - is_react_children = array_call; - } - } else { - // case we have `React.Children` - is_react_children = is_react_call_api(&object, model, "Children")? && array_call; + if !array_call { + return None; } - if is_react_children { + let object = member_expression.object().ok()?; + + // React.Children.forEach/map or Children.forEach/map + if is_react_call_api(&object, model, "Children")? { let arguments = call_expression.arguments().ok()?; let arguments = arguments.args(); let mut arguments = arguments.into_iter(); - let _ = arguments.next()?.ok()?; - let second_argument = arguments.next()?.ok()?; - let second_argument = second_argument.as_js_any_expression()?; - FunctionLike::cast(second_argument.clone().into_syntax()) + match (arguments.next(), arguments.next(), arguments.next()) { + (Some(_), Some(Ok(JsAnyCallArgument::JsAnyExpression(second_argument))), None) => { + FunctionLike::cast(second_argument.into_syntax()) + } + _ => None, + } } else { None } diff --git a/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx b/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx index 488f9ace4bb..efa70cb986b 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx @@ -1,3 +1,5 @@ +import { Children, cloneElement } from "react"; + // invalid something.forEach((Element, index) => { foo @@ -57,4 +59,4 @@ something.forEach((element, index) => { something.forEach((element, index) => { -}); \ No newline at end of file +}); diff --git a/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx.snap index 702ceaa1d11..682b1d94dec 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noArrayIndexKey.jsx.snap @@ -4,6 +4,8 @@ expression: noArrayIndexKey.jsx --- # Input ```js +import { Children, cloneElement } from "react"; + // invalid something.forEach((Element, index) => { foo @@ -64,28 +66,29 @@ something.forEach((element, index) => { }); + ``` # Diagnostics ``` -noArrayIndexKey.jsx:3:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:5:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 1 │ // invalid - 2 │ something.forEach((Element, index) => { - > 3 │ foo + 3 │ // invalid + 4 │ something.forEach((Element, index) => { + > 5 │ foo │ ^^^^^ - 4 │ }); - 5 │ something.forEach((element, index, array) => { + 6 │ }); + 7 │ something.forEach((element, index, array) => { i This is the source of the key value. - 1 │ // invalid - > 2 │ something.forEach((Element, index) => { + 3 │ // invalid + > 4 │ something.forEach((Element, index) => { │ ^^^^^ - 3 │ foo - 4 │ }); + 5 │ foo + 6 │ }); i The order of the items may change, and this also affects performances and component state. @@ -95,25 +98,25 @@ noArrayIndexKey.jsx:3:21 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:6:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:8:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 4 │ }); - 5 │ something.forEach((element, index, array) => { - > 6 │ foo - │ ^^^^^ - 7 │ }); - 8 │ things.filter((thing, index) => { + 6 │ }); + 7 │ something.forEach((element, index, array) => { + > 8 │ foo + │ ^^^^^ + 9 │ }); + 10 │ things.filter((thing, index) => { i This is the source of the key value. - 3 │ foo - 4 │ }); - > 5 │ something.forEach((element, index, array) => { + 5 │ foo + 6 │ }); + > 7 │ something.forEach((element, index, array) => { │ ^^^^^ - 6 │ foo - 7 │ }); + 8 │ foo + 9 │ }); i The order of the items may change, and this also affects performances and component state. @@ -123,25 +126,25 @@ noArrayIndexKey.jsx:6:21 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:9:34 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:11:34 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 7 │ }); - 8 │ things.filter((thing, index) => { - > 9 │ otherThings.push(foo); + 9 │ }); + 10 │ things.filter((thing, index) => { + > 11 │ otherThings.push(foo); │ ^^^^^ - 10 │ }); - 11 │ + 12 │ }); + 13 │ i This is the source of the key value. - 6 │ foo - 7 │ }); - > 8 │ things.filter((thing, index) => { + 8 │ foo + 9 │ }); + > 10 │ things.filter((thing, index) => { │ ^^^^^ - 9 │ otherThings.push(foo); - 10 │ }); + 11 │ otherThings.push(foo); + 12 │ }); i The order of the items may change, and this also affects performances and component state. @@ -151,24 +154,24 @@ noArrayIndexKey.jsx:9:34 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:13:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:15:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 12 │ something.forEach((Element, index) => { - > 13 │ + 14 │ something.forEach((Element, index) => { + > 15 │ │ ^^^^^ - 14 │ }); - 15 │ something.forEach((element, index, array) => { + 16 │ }); + 17 │ something.forEach((element, index, array) => { i This is the source of the key value. - 10 │ }); - 11 │ - > 12 │ something.forEach((Element, index) => { + 12 │ }); + 13 │ + > 14 │ something.forEach((Element, index) => { │ ^^^^^ - 13 │ - 14 │ }); + 15 │ + 16 │ }); i The order of the items may change, and this also affects performances and component state. @@ -178,25 +181,25 @@ noArrayIndexKey.jsx:13:21 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:16:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:18:21 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 14 │ }); - 15 │ something.forEach((element, index, array) => { - > 16 │ + 16 │ }); + 17 │ something.forEach((element, index, array) => { + > 18 │ │ ^^^^^ - 17 │ }); - 18 │ things.filter((thing, index) => { + 19 │ }); + 20 │ things.filter((thing, index) => { i This is the source of the key value. - 13 │ - 14 │ }); - > 15 │ something.forEach((element, index, array) => { + 15 │ + 16 │ }); + > 17 │ something.forEach((element, index, array) => { │ ^^^^^ - 16 │ - 17 │ }); + 18 │ + 19 │ }); i The order of the items may change, and this also affects performances and component state. @@ -206,25 +209,25 @@ noArrayIndexKey.jsx:16:21 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:19:34 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:21:34 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 17 │ }); - 18 │ things.filter((thing, index) => { - > 19 │ otherThings.push(); + 19 │ }); + 20 │ things.filter((thing, index) => { + > 21 │ otherThings.push(); │ ^^^^^ - 20 │ }); - 21 │ things.reduce((collection, thing, index) => ( + 22 │ }); + 23 │ things.reduce((collection, thing, index) => ( i This is the source of the key value. - 16 │ - 17 │ }); - > 18 │ things.filter((thing, index) => { + 18 │ + 19 │ }); + > 20 │ things.filter((thing, index) => { │ ^^^^^ - 19 │ otherThings.push(); - 20 │ }); + 21 │ otherThings.push(); + 22 │ }); i The order of the items may change, and this also affects performances and component state. @@ -234,25 +237,25 @@ noArrayIndexKey.jsx:19:34 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:22:35 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:24:35 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 20 │ }); - 21 │ things.reduce((collection, thing, index) => ( - > 22 │ collection.concat() + 22 │ }); + 23 │ things.reduce((collection, thing, index) => ( + > 24 │ collection.concat() │ ^^^^^ - 23 │ ), []); - 24 │ + 25 │ ), []); + 26 │ i This is the source of the key value. - 19 │ otherThings.push(); - 20 │ }); - > 21 │ things.reduce((collection, thing, index) => ( + 21 │ otherThings.push(); + 22 │ }); + > 23 │ things.reduce((collection, thing, index) => ( │ ^^^^^ - 22 │ collection.concat() - 23 │ ), []); + 24 │ collection.concat() + 25 │ ), []); i The order of the items may change, and this also affects performances and component state. @@ -262,24 +265,24 @@ noArrayIndexKey.jsx:22:35 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:26:38 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:28:38 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 25 │ React.Children.map(this.props.children, (child, index) => ( - > 26 │ React.cloneElement(child, { key: index }) + 27 │ React.Children.map(this.props.children, (child, index) => ( + > 28 │ React.cloneElement(child, { key: index }) │ ^^^^^ - 27 │ )) - 28 │ + 29 │ )) + 30 │ i This is the source of the key value. - 23 │ ), []); - 24 │ - > 25 │ React.Children.map(this.props.children, (child, index) => ( + 25 │ ), []); + 26 │ + > 27 │ React.Children.map(this.props.children, (child, index) => ( │ ^^^^^ - 26 │ React.cloneElement(child, { key: index }) - 27 │ )) + 28 │ React.cloneElement(child, { key: index }) + 29 │ )) i The order of the items may change, and this also affects performances and component state. @@ -289,24 +292,24 @@ noArrayIndexKey.jsx:26:38 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:30:45 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:32:45 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 29 │ React.Children.forEach(this.props.children, function (child, index) { - > 30 │ return React.cloneElement(child, { key: index }) + 31 │ React.Children.forEach(this.props.children, function (child, index) { + > 32 │ return React.cloneElement(child, { key: index }) │ ^^^^^ - 31 │ }) - 32 │ + 33 │ }) + 34 │ i This is the source of the key value. - 27 │ )) - 28 │ - > 29 │ React.Children.forEach(this.props.children, function (child, index) { + 29 │ )) + 30 │ + > 31 │ React.Children.forEach(this.props.children, function (child, index) { │ ^^^^^ - 30 │ return React.cloneElement(child, { key: index }) - 31 │ }) + 32 │ return React.cloneElement(child, { key: index }) + 33 │ }) i The order of the items may change, and this also affects performances and component state. @@ -316,22 +319,22 @@ noArrayIndexKey.jsx:30:45 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:35:32 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:37:32 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 34 │ Children.map(this.props.children, (child, index) => ( - > 35 │ cloneElement(child, { key: index }) + 36 │ Children.map(this.props.children, (child, index) => ( + > 37 │ cloneElement(child, { key: index }) │ ^^^^^ - 36 │ )) - 37 │ + 38 │ )) + 39 │ i This is the source of the key value. - > 34 │ Children.map(this.props.children, (child, index) => ( + > 36 │ Children.map(this.props.children, (child, index) => ( │ ^^^^^ - 35 │ cloneElement(child, { key: index }) - 36 │ )) + 37 │ cloneElement(child, { key: index }) + 38 │ )) i The order of the items may change, and this also affects performances and component state. @@ -341,24 +344,24 @@ noArrayIndexKey.jsx:35:32 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:39:39 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:41:39 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 38 │ Children.forEach(this.props.children, function (child, index) { - > 39 │ return cloneElement(child, { key: index }) + 40 │ Children.forEach(this.props.children, function (child, index) { + > 41 │ return cloneElement(child, { key: index }) │ ^^^^^ - 40 │ }) - 41 │ + 42 │ }) + 43 │ i This is the source of the key value. - 36 │ )) - 37 │ - > 38 │ Children.forEach(this.props.children, function (child, index) { + 38 │ )) + 39 │ + > 40 │ Children.forEach(this.props.children, function (child, index) { │ ^^^^^ - 39 │ return cloneElement(child, { key: index }) - 40 │ }) + 41 │ return cloneElement(child, { key: index }) + 42 │ }) i The order of the items may change, and this also affects performances and component state. @@ -368,24 +371,24 @@ noArrayIndexKey.jsx:39:39 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:43:44 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:45:44 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 42 │ Children.forEach(this.props.children, function (child, index) { - > 43 │ const foo = cloneElement(child, { key: index }) + 44 │ Children.forEach(this.props.children, function (child, index) { + > 45 │ const foo = cloneElement(child, { key: index }) │ ^^^^^ - 44 │ return foo; - 45 │ }) + 46 │ return foo; + 47 │ }) i This is the source of the key value. - 40 │ }) - 41 │ - > 42 │ Children.forEach(this.props.children, function (child, index) { + 42 │ }) + 43 │ + > 44 │ Children.forEach(this.props.children, function (child, index) { │ ^^^^^ - 43 │ const foo = cloneElement(child, { key: index }) - 44 │ return foo; + 45 │ const foo = cloneElement(child, { key: index }) + 46 │ return foo; i The order of the items may change, and this also affects performances and component state. @@ -395,22 +398,22 @@ noArrayIndexKey.jsx:43:44 lint/correctness/noArrayIndexKey ━━━━━━━ ``` ``` -noArrayIndexKey.jsx:49:38 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noArrayIndexKey.jsx:51:38 lint/correctness/noArrayIndexKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid using the index of an array as key property in an element. - 48 │ things.map((thing, index) => ( - > 49 │ React.cloneElement(thing, { key: index }) + 50 │ things.map((thing, index) => ( + > 51 │ React.cloneElement(thing, { key: index }) │ ^^^^^ - 50 │ )); - 51 │ + 52 │ )); + 53 │ i This is the source of the key value. - > 48 │ things.map((thing, index) => ( + > 50 │ things.map((thing, index) => ( │ ^^^^^ - 49 │ React.cloneElement(thing, { key: index }) - 50 │ )); + 51 │ React.cloneElement(thing, { key: index }) + 52 │ )); i The order of the items may change, and this also affects performances and component state. diff --git a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx index 02c39406f0e..a39b478ae12 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx @@ -1,3 +1,5 @@ +import { createElement as aliased } from "react"; + <> @@ -9,3 +11,8 @@ createElement('div', { React.createElement('div', { children: 'foo' }) + + +aliased('div', { + children: 'foo' +}) diff --git a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx.snap index f35e7d29f7e..2873d60944c 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropInvalid.jsx.snap @@ -4,6 +4,8 @@ expression: noChildrenPropInvalid.jsx --- # Input ```js +import { createElement as aliased } from "react"; + <> @@ -16,19 +18,24 @@ React.createElement('div', { children: 'foo' }) + +aliased('div', { + children: 'foo' +}) + ``` # Diagnostics ``` -noChildrenPropInvalid.jsx:2:16 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noChildrenPropInvalid.jsx:4:16 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid passing children using a prop - 1 │ <> - > 2 │ + 3 │ <> + > 4 │ │ ^^^^^^^^ - 3 │ - 4 │ + 5 │ + 6 │ i The canonical way to pass children in React is to use JSX elements @@ -36,15 +43,15 @@ noChildrenPropInvalid.jsx:2:16 lint/correctness/noChildrenProp ━━━━━ ``` ``` -noChildrenPropInvalid.jsx:6:5 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noChildrenPropInvalid.jsx:12:5 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid passing children using a prop - 5 │ createElement('div', { - > 6 │ children: 'foo' - │ ^^^^^^^^ - 7 │ }) - 8 │ + 11 │ React.createElement('div', { + > 12 │ children: 'foo' + │ ^^^^^^^^ + 13 │ }) + 14 │ i The canonical way to pass children in React is to use additional arguments to React.createElement @@ -52,15 +59,15 @@ noChildrenPropInvalid.jsx:6:5 lint/correctness/noChildrenProp ━━━━━━ ``` ``` -noChildrenPropInvalid.jsx:10:5 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noChildrenPropInvalid.jsx:17:2 lint/correctness/noChildrenProp ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Avoid passing children using a prop - 9 │ React.createElement('div', { - > 10 │ children: 'foo' - │ ^^^^^^^^ - 11 │ }) - 12 │ + 16 │ aliased('div', { + > 17 │ children: 'foo' + │ ^^^^^^^^ + 18 │ }) + 19 │ i The canonical way to pass children in React is to use additional arguments to React.createElement diff --git a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx index b5ea51268e0..bf0efd8dbd5 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx @@ -1,3 +1,6 @@ +import { cloneElement } from "react"; +import React from "react"; + <> @@ -7,3 +10,6 @@ createElement('div', {}, 'foo') + +cloneElement('div', { children:
}); +React.cloneElement('div', { children:
}); diff --git a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx.snap index 4523817da33..d193e41a1ee 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noChildrenProp/noChildrenPropValid.jsx.snap @@ -4,6 +4,9 @@ expression: noChildrenPropValid.jsx --- # Input ```js +import { cloneElement } from "react"; +import React from "react"; + <> @@ -14,6 +17,9 @@ expression: noChildrenPropValid.jsx createElement('div', {}, 'foo') +cloneElement('div', { children:
}); +React.cloneElement('div', { children:
}); + ``` diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx index 2be0a35fa56..85ad76e5730 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx @@ -4,4 +4,4 @@ <> - \ No newline at end of file + diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx.snap index 0c8caa8df96..e2b12e7221a 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/noChildren.jsx.snap @@ -11,6 +11,7 @@ expression: noChildren.jsx + ``` # Diagnostics @@ -61,6 +62,7 @@ noChildren.jsx:6:5 lint/correctness/noUselessFragments FIXABLE ━━━━━ > 6 │ │ ^^^^^^^^^^^^^^^^^^^^^ 7 │ + 8 │ i Suggested fix: Remove the Fragment diff --git a/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js b/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js index 45fd0486666..9a524163ac5 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js +++ b/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js @@ -6,4 +6,4 @@ React.createElement('img', { React.createElement('img', { dangerouslySetInnerHTML: "text" -}) \ No newline at end of file +}) diff --git a/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js.snap index 51e6774c7a8..c538cfc609f 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noVoidElementsWithChildren/createElement.js.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 99 expression: createElement.js --- # Input @@ -14,6 +13,7 @@ React.createElement('img', { React.createElement('img', { dangerouslySetInnerHTML: "text" }) + ``` # Diagnostics @@ -42,20 +42,22 @@ createElement.js:7:1 lint/correctness/noVoidElementsWithChildren FIXABLE ━ ! img is a void element tag and must not have the dangerouslySetInnerHTML prop. - 5 │ }, 'child') - 6 │ - > 7 │ React.createElement('img', { - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - > 8 │ dangerouslySetInnerHTML: "text" - > 9 │ }) - │ ^^ + 5 │ }, 'child') + 6 │ + > 7 │ React.createElement('img', { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 8 │ dangerouslySetInnerHTML: "text" + > 9 │ }) + │ ^^ + 10 │ i Suggested fix: Remove the dangerouslySetInnerHTML prop. - 6 6 │ - 7 7 │ React.createElement('img', { - 8 │ - ····dangerouslySetInnerHTML:·"text" - 9 8 │ }) + 6 6 │ + 7 7 │ React.createElement('img', { + 8 │ - ····dangerouslySetInnerHTML:·"text" + 9 8 │ }) + 10 9 │ ``` diff --git a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js index bfd30c586cb..4d78eab0fd6 100644 --- a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js +++ b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js @@ -1,3 +1,3 @@ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'child' } -}); \ No newline at end of file +}); diff --git a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js.snap b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js.snap index f697f49b28e..684283d21eb 100644 --- a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js.snap +++ b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtml/reactCreateElement.js.snap @@ -7,6 +7,7 @@ expression: reactCreateElement.js React.createElement('div', { dangerouslySetInnerHTML: { __html: 'child' } }); + ``` # Diagnostics @@ -19,6 +20,7 @@ reactCreateElement.js:2:5 lint/security/noDangerouslySetInnerHtml ━━━━ > 2 │ dangerouslySetInnerHTML: { __html: 'child' } │ ^^^^^^^^^^^^^^^^^^^^^^^ 3 │ }); + 4 │ ! Setting content using code can expose users to cross-site scripting (XSS) attacks diff --git a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js index 5cbae195554..6411485fc1a 100644 --- a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js +++ b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js @@ -1,3 +1,3 @@ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, ['children']) React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') -React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) \ No newline at end of file +React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) diff --git a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js.snap b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js.snap index aa21f4378da..2fd650c30c7 100644 --- a/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js.snap +++ b/crates/rome_js_analyze/tests/specs/security/noDangerouslySetInnerHtmlWithChildren/createElement.js.snap @@ -7,6 +7,7 @@ expression: createElement.js React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, ['children']) React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) + ``` # Diagnostics @@ -41,6 +42,7 @@ createElement.js:2:30 lint/security/noDangerouslySetInnerHtmlWithChildren ━━ > 2 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 3 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) + 4 │ i This is the source of the children prop @@ -48,6 +50,7 @@ createElement.js:2:30 lint/security/noDangerouslySetInnerHtmlWithChildren ━━ > 2 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') │ ^^^^^^^^^^ 3 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) + 4 │ i Setting HTML content will inadvertently override any passed children in React @@ -63,6 +66,7 @@ createElement.js:3:30 lint/security/noDangerouslySetInnerHtmlWithChildren ━━ 2 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') > 3 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 4 │ i This is the source of the children prop @@ -70,6 +74,7 @@ createElement.js:3:30 lint/security/noDangerouslySetInnerHtmlWithChildren ━━ 2 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' } }, 'children') > 3 │ React.createElement('div', { dangerouslySetInnerHTML: { __html: 'HTML' }, children: 'children' }) │ ^^^^^^^^^^^^^^^^^^^^ + 4 │ i Setting HTML content will inadvertently override any passed children in React