From ef1a9a7b9e5834612215a3d29ab3e6f204bbba8c Mon Sep 17 00:00:00 2001 From: l3ops Date: Tue, 8 Nov 2022 17:10:20 +0100 Subject: [PATCH] fix(rome_js_analyze): improve the logic of jsx_reference_identifier_is_fragment --- crates/rome_js_analyze/src/react.rs | 53 +++++++++++++------ .../fromImportRenameValid.jsx | 7 ++- .../fromImportRenameValid.jsx.snap | 8 ++- .../noUselessFragments/notFragmentValid.jsx | 6 +++ .../notFragmentValid.jsx.snap | 17 ++++++ .../noUselessFragments/userDefinedValid.jsx | 2 +- .../userDefinedValid.jsx.snap | 3 +- 7 files changed, 73 insertions(+), 23 deletions(-) create mode 100644 crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx create mode 100644 crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx.snap diff --git a/crates/rome_js_analyze/src/react.rs b/crates/rome_js_analyze/src/react.rs index d31e62f7cce..e105e11e9e6 100644 --- a/crates/rome_js_analyze/src/react.rs +++ b/crates/rome_js_analyze/src/react.rs @@ -4,8 +4,10 @@ pub mod hooks; use rome_js_semantic::SemanticModel; use rome_js_syntax::{ - JsAnyCallArgument, JsAnyExpression, JsCallExpression, JsIdentifierBinding, JsImport, - JsObjectExpression, JsPropertyObjectMember, JsxMemberName, JsxReferenceIdentifier, + JsAnyCallArgument, JsAnyExpression, JsAnyNamedImportSpecifier, JsCallExpression, + JsIdentifierBinding, JsImport, JsImportNamedClause, JsNamedImportSpecifierList, + JsNamedImportSpecifiers, JsObjectExpression, JsPropertyObjectMember, JsxMemberName, + JsxReferenceIdentifier, }; use rome_rowan::{AstNode, AstSeparatedList}; @@ -331,21 +333,38 @@ pub(crate) fn jsx_reference_identifier_is_fragment( name: &JsxReferenceIdentifier, model: &SemanticModel, ) -> Option { - let value_token = name.value_token().ok()?; - let mut maybe_react_fragment = value_token.text_trimmed() == "Fragment"; - if let Some(reference) = model.declaration(name) { - 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; - } else { - // `Fragment` is a binding g but it doesn't come from the "react" package - maybe_react_fragment = false; + 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(maybe_react_fragment) + None => { + let value_token = name.value_token().ok()?; + let is_fragment = value_token.text_trimmed() == "Fragment"; + Some(is_fragment) + } + } } diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx index 0263f8548e4..f20deb2583e 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx @@ -1,6 +1,9 @@ -import AwesomeReact, { Fragment as AwesomeFragment } from "noReact"; +import AwesomeNoReact, { Fragment as AwesomeFragment } from "noReact"; +import AwesomeReact, { StrictMode as AwesomeStrictMode } from "react"; <> - foo + foo + + foo diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx.snap index 2c41a16804a..9fbf19b72f1 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/fromImportRenameValid.jsx.snap @@ -1,14 +1,18 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 99 expression: fromImportRenameValid.jsx --- # Input ```js -import AwesomeReact, { Fragment as AwesomeFragment } from "noReact"; +import AwesomeNoReact, { Fragment as AwesomeFragment } from "noReact"; +import AwesomeReact, { StrictMode as AwesomeStrictMode } from "react"; <> - foo + foo + + foo ``` diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx new file mode 100644 index 00000000000..d1495bc9634 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx @@ -0,0 +1,6 @@ +import React, { StrictMode } from "react"; + +<> + + + diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx.snap new file mode 100644 index 00000000000..ec5b4c8fb77 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/notFragmentValid.jsx.snap @@ -0,0 +1,17 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 99 +expression: notFragmentValid.jsx +--- +# Input +```js +import React, { StrictMode } from "react"; + +<> + + + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx index 722e5e1740a..27f7d45fef5 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx @@ -2,5 +2,5 @@ function Fragment() {} let React = { Fragment }; <> test - test + test diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx.snap index a5b8c1f8949..c8b666310c3 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/userDefinedValid.jsx.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 99 expression: userDefinedValid.jsx --- # Input @@ -8,7 +9,7 @@ function Fragment() {} let React = { Fragment }; <> test - test + test ```