fix(rome_js_analyze): false positives #3668

rome · Nov 25, 2022 · a57a0bf · a57a0bf
1 parent f9a3db6
commit a57a0bf
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 23 deletions.
diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs
@@ -134,6 +134,7 @@ mod tests {
     use rome_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic, Severity};
     use rome_js_parser::parse;
     use rome_js_syntax::{SourceType, TextRange, TextSize};
+    use std::slice;
 
     use crate::{analyze, AnalysisFilter, ControlFlow};
 
@@ -149,25 +150,21 @@ mod tests {
             String::from_utf8(buffer).unwrap()
         }
 
-        const SOURCE: &str = r#"function f() {
-            return (
-                <div
-                ><img /></div>
-            )
-        };
-        f();
+        const SOURCE: &str = r#"const obj = {
+  element: <>test</>
+};
         "#;
 
         let parsed = parse(SOURCE, FileId::zero(), SourceType::jsx());
 
         let mut error_ranges: Vec<TextRange> = Vec::new();
         let options = AnalyzerOptions::default();
-        let _rule_filter = RuleFilter::Rule("correctness", "flipBinExp");
+        let rule_filter = RuleFilter::Rule("correctness", "noUselessFragments");
         analyze(
             FileId::zero(),
             &parsed.tree(),
             AnalysisFilter {
-                // enabled_rules: Some(slice::from_ref(&rule_filter)),
+                enabled_rules: Some(slice::from_ref(&rule_filter)),
                 ..AnalysisFilter::default()
             },
             &options,

diff --git a/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_useless_fragments.rs b/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_useless_fragments.rs
@@ -9,12 +9,10 @@ use rome_js_factory::make::{
     ident, js_expression_statement, js_string_literal_expression, jsx_tag_expression,
 };
 use rome_js_syntax::{
-    JsLanguage, JsSyntaxKind, JsxAnyChild, JsxAnyElementName, JsxAnyTag, JsxChildList, JsxElement,
-    JsxFragment, JsxTagExpression,
-};
-use rome_rowan::{
-    declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt, SyntaxNodeOptionExt,
+    JsLanguage, JsParenthesizedExpression, JsSyntaxKind, JsxAnyChild, JsxAnyElementName, JsxAnyTag,
+    JsxChildList, JsxElement, JsxFragment, JsxTagExpression,
 };
+use rome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt};
 
 declare_rule! {
     /// Disallow unnecessary fragments
@@ -108,31 +106,42 @@ impl Rule for NoUselessFragments {
         let model = ctx.model();
         match node {
             NoUselessFragmentsQuery::JsxFragment(fragment) => {
-                let matches_allowed_parents = node
+                let parents_where_fragments_must_be_preserved = node
                     .syntax()
                     .parent()
                     .map(|parent| match JsxTagExpression::try_cast(parent) {
-                        Ok(parent) => {
-                            let parent_kind = parent.syntax().parent().kind();
-                            matches!(
-                                parent_kind,
-                                Some(
+                        Ok(parent) => parent
+                            .syntax()
+                            .parent()
+                            .and_then(|parent| {
+                                if let Some(parenthesized_expression) =
+                                    JsParenthesizedExpression::cast_ref(&parent)
+                                {
+                                    parenthesized_expression.syntax().parent()
+                                } else {
+                                    Some(parent)
+                                }
+                            })
+                            .map(|parent| {
+                                matches!(
+                                    parent.kind(),
                                     JsSyntaxKind::JS_RETURN_STATEMENT
                                         | JsSyntaxKind::JS_INITIALIZER_CLAUSE
                                         | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION
                                         | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION
                                         | JsSyntaxKind::JS_FUNCTION_EXPRESSION
                                         | JsSyntaxKind::JS_FUNCTION_DECLARATION
+                                        | JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER
                                 )
-                            )
-                        }
+                            })
+                            .unwrap_or(false),
                         Err(_) => false,
                     })
                     .unwrap_or(false);
 
                 let child_list = fragment.children();
 
-                if !matches_allowed_parents {
+                if !parents_where_fragments_must_be_preserved {
                     match child_list.first() {
                         Some(first) if child_list.len() == 1 => {
                             Some(NoUselessFragmentsState::Child(first))

diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/issue_3668.jsx b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/issue_3668.jsx
@@ -0,0 +1,9 @@
+// should not trigger
+function Component2() {
+    const str = 'str';
+    return (<>{str}</>);
+}
+
+const obj = {
+    element: <>test</>
+};
diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/issue_3668.jsx.snap b/crates/rome_js_analyze/tests/specs/correctness/noUselessFragments/issue_3668.jsx.snap
@@ -0,0 +1,19 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: issue_3668.jsx
+---
+# Input
+```js
+// should not trigger
+function Component2() {
+    const str = 'str';
+    return (<>{str}</>);
+}
+
+const obj = {
+    element: <>test</>
+};
+
+```
+
+