feat(rome_js_analyze): partial code action for `noUselessFragments`
MODIFY
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_fragments.rs
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_fragments.rs
use crate::react::{jsx_member_name_is_react_fragment, jsx_reference_identifier_is_fragment};
use crate::semantic_services::Semantic;
+use crate::JsRuleAction;
use rome_analyze::context::RuleContext;
-use rome_analyze::{declare_rule, Rule, RuleCategory, RuleDiagnostic};
+use rome_analyze::{declare_rule, ActionCategory, Rule, RuleCategory, RuleDiagnostic};
use rome_console::markup;
+use rome_diagnostics::Applicability;
use rome_js_syntax::{
- JsSyntaxKind, JsxAnyChild, JsxAnyElementName, JsxElement, JsxFragment, JsxTagExpression,
+ JsLanguage, JsSyntaxKind, JsxAnyChild, JsxAnyElementName, JsxChildList, JsxElement,
+ JsxFragment, JsxTagExpression,
};
-use rome_rowan::{declare_node_union, AstNode, AstNodeList};
+use rome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt};
declare_rule! {
/// Disallow unnecessary fragments
@@ -28,6 +31,13 @@ declare_rule! {
/// ```
///
/// ```jsx,expect_diagnostic
+ /// <>
+ /// <>foo</>
+ /// <SomeComponent />
+ /// </>
+ /// ```
+ ///
+ /// ```jsx,expect_diagnostic
/// <></>
/// ```
pub(crate) NoUselessFragments {
@@ -47,6 +57,34 @@ declare_node_union! {
pub(crate) NoUselessFragmentsQuery = JsxFragment | JsxElement
}
+impl NoUselessFragmentsQuery {
+ fn replace_node(&self, mutation: &mut BatchMutation<JsLanguage>, new_node: JsxAnyChild) {
+ match self {
+ NoUselessFragmentsQuery::JsxFragment(fragment) => {
+ let old_node = JsxAnyChild::JsxFragment(fragment.clone());
+ mutation.replace_node(old_node, new_node);
+ }
+ NoUselessFragmentsQuery::JsxElement(element) => {
+ let old_node = JsxAnyChild::JsxElement(element.clone());
+ mutation.replace_node(old_node, new_node);
+ }
+ }
+ }
+
+ fn remove_node_from_list(&self, mutation: &mut BatchMutation<JsLanguage>) {
+ match self {
+ NoUselessFragmentsQuery::JsxFragment(fragment) => {
+ let old_node = JsxAnyChild::JsxFragment(fragment.clone());
+ mutation.remove_node(old_node);
+ }
+ NoUselessFragmentsQuery::JsxElement(element) => {
+ let old_node = JsxAnyChild::JsxElement(element.clone());
+ mutation.remove_node(old_node);
+ }
+ }
+ }
+}
+
impl Rule for NoUselessFragments {
const CATEGORY: RuleCategory = RuleCategory::Lint;
@@ -99,25 +137,78 @@ impl Rule for NoUselessFragments {
let opening_element = element.opening_element().ok()?;
let name = opening_element.name().ok()?;
- match name {
+ let is_valid_react_fragment = match name {
JsxAnyElementName::JsxMemberName(member_name) => {
- if jsx_member_name_is_react_fragment(&member_name, model)? {
- Some(NoUselessFragmentsState::Empty)
+ jsx_member_name_is_react_fragment(&member_name, model)?
+ }
+ JsxAnyElementName::JsxReferenceIdentifier(identifier) => {
+ jsx_reference_identifier_is_fragment(&identifier, model)?
+ }
+ JsxAnyElementName::JsxName(_) | JsxAnyElementName::JsxNamespaceName(_) => false,
+ };
+
+ if is_valid_react_fragment {
+ let child_list = element.children();
+ // The `Fragment` component supports only the "key" prop and react emits a warning for not supported props.
+ // We assume that the user knows - and fixed - that and only care about the prop that is actually supported.
+ let attribute_key =
+ opening_element
+ .attributes()
+ .into_iter()
+ .find_map(|attribute| {
+ let attribute = attribute.as_jsx_attribute()?;
+ let attribute_name = attribute.name().ok()?;
+ let attribute_name = attribute_name.as_jsx_name()?;
+
+ if attribute_name.value_token().ok()?.text_trimmed() == "key" {
+ Some(())
} else {
None
}
+ });
+ if attribute_key.is_none() {
+ return match child_list.first() {
+ Some(first) if child_list.len() == 1 => {
+ Some(NoUselessFragmentsState::Child(first))
}
- JsxAnyElementName::JsxReferenceIdentifier(identifier) => {
- if jsx_reference_identifier_is_fragment(&identifier, model)? {
- Some(NoUselessFragmentsState::Empty)
- } else {
+ None => Some(NoUselessFragmentsState::Empty),
+ _ => None,
+ };
+ }
+ }
+
None
}
}
- JsxAnyElementName::JsxName(_) | JsxAnyElementName::JsxNamespaceName(_) => None,
}
+
+ fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
+ let node = ctx.query();
+ let mut mutation = ctx.root().begin();
+
+ let is_in_list = node
+ .syntax()
+ .parent()
+ .map_or(false, |parent| JsxChildList::can_cast(parent.kind()));
+ if is_in_list {
+ let new_child = match state {
+ NoUselessFragmentsState::Empty => None,
+ NoUselessFragmentsState::Child(child) => Some(child.clone()),
+ };
+
+ if let Some(new_child) = new_child {
+ node.replace_node(&mut mutation, new_child);
+ } else {
+ node.remove_node_from_list(&mut mutation);
}
}
+
+ Some(JsRuleAction {
+ category: ActionCategory::Refactor,
+ applicability: Applicability::MaybeIncorrect,
+ message: markup! { "Remove the Fragment" }.to_owned(),
+ mutation,
+ })
}
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {

ADD
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/valid.jsx
+import React, {Fragment} from "react";
+
+<>
+ <React.Fragment key="1"></React.Fragment>
+ <Fragment key="1"></Fragment>
+</>
\ No newline at end of file

ADD
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/valid.jsx.snap
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: valid.jsx
+---
+# Input
+```js
+import React, {Fragment} from "react";
+
+<>
+ <React.Fragment key="1"></React.Fragment>
+ <Fragment key="1"></Fragment>
+</>
+```
+
+

feat(rome_js_analyze): partial code action for `noUselessFragments` (#3264)