From f663e6b5d1389fe236a8f401456f8b976eb9d482 Mon Sep 17 00:00:00 2001 From: unvalley <38400669+unvalley@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:10:38 +0900 Subject: [PATCH] fix(lint): prevent crash when applying `noUselessFragments` unsafe fixes (#3338) --- CHANGELOG.md | 2 + .../lint/complexity/no_useless_fragments.rs | 20 ++--- .../noUselessFragments/issue_3149.jsx | 5 ++ .../noUselessFragments/issue_3149.jsx.snap | 76 +++++++++++++++++++ 4 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx create mode 100644 crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index d8875a0a1d9b..5853def22452 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - Don't request alt text for elements hidden from assistive technologies ([#3316](https://github.com/biomejs/biome/issues/3316)). Contributed by @robintown +- Fix [[#3149](https://github.com/biomejs/biome/issues/3149)] crashes that occurred when applying the `noUselessFragments` unsafe fixes in certain scenarios. Contributed by @unvalley + ### Parser ## v1.8.3 (2024-06-27) diff --git a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs index 385ad5b6d86e..e54cbce18c3c 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_useless_fragments.rs @@ -5,13 +5,13 @@ use biome_analyze::context::RuleContext; use biome_analyze::{declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic, RuleSource}; use biome_console::markup; use biome_js_factory::make::{ - js_expression_statement, js_string_literal_expression, jsx_expression_child, jsx_string, - jsx_string_literal, jsx_tag_expression, token, JsxExpressionChildBuilder, + js_string_literal_expression, jsx_expression_child, jsx_string, jsx_string_literal, + jsx_tag_expression, token, JsxExpressionChildBuilder, }; use biome_js_syntax::{ - AnyJsExpression, AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, - JsParenthesizedExpression, JsSyntaxKind, JsxChildList, JsxElement, JsxExpressionAttributeValue, - JsxFragment, JsxTagExpression, JsxText, T, + AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, JsParenthesizedExpression, JsSyntaxKind, + JsxChildList, JsxElement, JsxExpressionAttributeValue, JsxFragment, JsxTagExpression, JsxText, + T, }; use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt}; @@ -294,13 +294,9 @@ impl Rule for NoUselessFragments { .into_syntax() }) } else { - child.expression().map(|expression| { - if let AnyJsExpression::JsIdentifierExpression(node) = expression { - node.into_syntax() - } else { - js_expression_statement(expression).build().into_syntax() - } - }) + child + .expression() + .map(|expression| expression.into_syntax()) } } diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx new file mode 100644 index 000000000000..93b9a8256be2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx @@ -0,0 +1,5 @@ +function fn(member) { + fn(<>{member.expression}); + fn(<>{member.expression()}); + (<>{1}).toString(); +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx.snap b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx.snap new file mode 100644 index 000000000000..5a9ea6f37f51 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_3149.jsx.snap @@ -0,0 +1,76 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue_3149.jsx +--- +# Input +```jsx +function fn(member) { + fn(<>{member.expression}); + fn(<>{member.expression()}); + (<>{1}).toString(); +} + +``` + +# Diagnostics +``` +issue_3149.jsx:2:6 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid using unnecessary Fragment. + + 1 │ function fn(member) { + > 2 │ fn(<>{member.expression}); + │ ^^^^^^^^^^^^^^^^^^^^^^^^ + 3 │ fn(<>{member.expression()}); + 4 │ (<>{1}).toString(); + + i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment. + + i Unsafe fix: Remove the Fragment + + 2 │ ··fn(<>{member.expression}); + │ --- ---- + +``` + +``` +issue_3149.jsx:3:6 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid using unnecessary Fragment. + + 1 │ function fn(member) { + 2 │ fn(<>{member.expression}); + > 3 │ fn(<>{member.expression()}); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ + 4 │ (<>{1}).toString(); + 5 │ } + + i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment. + + i Unsafe fix: Remove the Fragment + + 3 │ ··fn(<>{member.expression()}); + │ --- ---- + +``` + +``` +issue_3149.jsx:4:4 lint/complexity/noUselessFragments FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid using unnecessary Fragment. + + 2 │ fn(<>{member.expression}); + 3 │ fn(<>{member.expression()}); + > 4 │ (<>{1}).toString(); + │ ^^^^^^^^ + 5 │ } + 6 │ + + i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment. + + i Unsafe fix: Remove the Fragment + + 4 │ ··(<>{1}).toString(); + │ --- ---- + +```