From 5ac8496f61b6c7188e742a01e8d877e483edb541 Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Thu, 18 Apr 2024 11:19:59 +0800 Subject: [PATCH 01/13] format template element on a single line --- .../src/js/auxiliary/template_element.rs | 33 +++++-- .../src/js/lists/template_element_list.rs | 92 +------------------ .../src/utils/test_each_template.rs | 3 +- 3 files changed, 31 insertions(+), 97 deletions(-) diff --git a/crates/biome_js_formatter/src/js/auxiliary/template_element.rs b/crates/biome_js_formatter/src/js/auxiliary/template_element.rs index bb969c821eab..f7874f726e76 100644 --- a/crates/biome_js_formatter/src/js/auxiliary/template_element.rs +++ b/crates/biome_js_formatter/src/js/auxiliary/template_element.rs @@ -6,12 +6,20 @@ use biome_formatter::{ use crate::context::TabWidth; use crate::js::expressions::array_expression::FormatJsArrayExpressionOptions; -use crate::js::lists::template_element_list::{TemplateElementIndention, TemplateElementLayout}; +use crate::js::lists::template_element_list::TemplateElementIndention; use biome_js_syntax::{ AnyJsExpression, JsSyntaxNode, JsSyntaxToken, JsTemplateElement, TsTemplateElement, }; use biome_rowan::{declare_node_union, AstNode, SyntaxResult}; +enum TemplateElementLayout { + /// Tries to format the expression on a single line regardless of the print width. + SingleLine, + + /// Tries to format the expression on a single line but may break the expression if the line otherwise exceeds the print width. + Fit, +} + #[derive(Debug, Clone, Default)] pub(crate) struct FormatJsTemplateElement { options: TemplateElementOptions, @@ -44,8 +52,6 @@ declare_node_union! { #[derive(Debug, Copy, Clone, Default)] pub struct TemplateElementOptions { - pub(crate) layout: TemplateElementLayout, - /// The indention to use for this element pub(crate) indention: TemplateElementIndention, @@ -66,7 +72,7 @@ impl FormatTemplateElement { impl Format for FormatTemplateElement { fn fmt(&self, f: &mut JsFormatter) -> FormatResult<()> { - let format_expression = format_with(|f| match &self.element { + let mut format_expression = format_with(|f| match &self.element { AnyTemplateElement::JsTemplateElement(template) => match template.expression()? { AnyJsExpression::JsArrayExpression(expression) => { let option = FormatJsArrayExpressionOptions { @@ -80,9 +86,19 @@ impl Format for FormatTemplateElement { AnyTemplateElement::TsTemplateElement(template) => { write!(f, [template.ty().format()]) } - }); + }) + .memoized(); + + let layout = + if !self.element.has_new_line_in_range() && + // to make sure the expression won't break to avoid reformatting issue + !format_expression.inspect(f)?.will_break() { + TemplateElementLayout::SingleLine + } else { + TemplateElementLayout::Fit + }; - let format_inner = format_with(|f: &mut JsFormatter| match self.options.layout { + let format_inner = format_with(|f: &mut JsFormatter| match layout { TemplateElementLayout::SingleLine => { let mut buffer = RemoveSoftLinesBuffer::new(f); write!(buffer, [format_expression]) @@ -111,6 +127,7 @@ impl Format for FormatTemplateElement { | JsLogicalExpression(_) | JsInstanceofExpression(_) | JsInExpression(_) + | JsIdentifierExpression(_) ) ); @@ -179,6 +196,10 @@ impl AnyTemplateElement { AnyTemplateElement::TsTemplateElement(template) => template.r_curly_token(), } } + + fn has_new_line_in_range(&self) -> bool { + self.syntax().text().contains_char('\n') + } } /// Writes `content` with the specified `indention`. diff --git a/crates/biome_js_formatter/src/js/lists/template_element_list.rs b/crates/biome_js_formatter/src/js/lists/template_element_list.rs index 0c2df6f7997a..185c14c145ee 100644 --- a/crates/biome_js_formatter/src/js/lists/template_element_list.rs +++ b/crates/biome_js_formatter/src/js/lists/template_element_list.rs @@ -5,10 +5,10 @@ use crate::prelude::*; use crate::utils::test_each_template::EachTemplateTable; use biome_formatter::FormatRuleWithOptions; use biome_js_syntax::{ - AnyJsExpression, AnyJsLiteralExpression, AnyJsTemplateElement, AnyTsTemplateElement, - JsLanguage, JsTemplateElementList, TsTemplateElementList, + AnyJsTemplateElement, AnyTsTemplateElement, JsLanguage, JsTemplateElementList, + TsTemplateElementList, }; -use biome_rowan::{declare_node_union, AstNodeListIterator, SyntaxResult}; +use biome_rowan::{declare_node_union, AstNodeListIterator}; use std::iter::FusedIterator; #[derive(Debug, Clone, Default)] @@ -49,12 +49,6 @@ pub(crate) enum AnyTemplateElementList { impl Format for AnyTemplateElementList { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - let layout = if self.is_simple(f.comments()) { - TemplateElementLayout::SingleLine - } else { - TemplateElementLayout::Fit - }; - let mut indention = TemplateElementIndention::default(); let mut after_new_line = false; @@ -64,7 +58,6 @@ impl Format for AnyTemplateElementList { let options = TemplateElementOptions { after_new_line, indention, - layout, }; match &element { @@ -104,36 +97,6 @@ impl Format for AnyTemplateElementList { } impl AnyTemplateElementList { - /// Returns `true` for `JsTemplate` if all elements are simple expressions that should be printed on a single line. - /// - /// Simple expressions are: - /// * Identifiers: `this`, `a` - /// * Members: `a.b`, `a[b]`, `a.b[c].d`, `a.b[5]`, `a.b["test"]` - fn is_simple(&self, comments: &JsComments) -> bool { - match self { - AnyTemplateElementList::JsTemplateElementList(list) => { - if list.is_empty() { - return false; - } - - let mut expression_elements = list.iter().filter_map(|element| match element { - AnyJsTemplateElement::JsTemplateElement(element) => Some(element), - _ => None, - }); - - expression_elements.all(|expression_element| { - match expression_element.expression() { - Ok(expression) => { - is_simple_member_expression(expression, comments).unwrap_or(false) - } - Err(_) => false, - } - }) - } - AnyTemplateElementList::TsTemplateElementList(_) => false, - } - } - fn elements(&self) -> TemplateElementIterator { match self { AnyTemplateElementList::JsTemplateElementList(list) => { @@ -146,59 +109,10 @@ impl AnyTemplateElementList { } } -#[derive(Debug, Copy, Clone, Default)] -pub enum TemplateElementLayout { - /// Applied when all expressions are identifiers, `this`, static member expressions, or computed member expressions with number or string literals. - /// Formats the expressions on a single line, even if their width otherwise would exceed the print width. - SingleLine, - - /// Tries to format the expression on a single line but may break the expression if the line otherwise exceeds the print width. - #[default] - Fit, -} - declare_node_union! { AnyTemplateElementOrChunk = AnyTemplateElement | AnyTemplateChunkElement } -fn is_simple_member_expression( - expression: AnyJsExpression, - comments: &JsComments, -) -> SyntaxResult { - let mut current = expression; - - loop { - if comments.has_comments(current.syntax()) { - return Ok(false); - } - - current = match current { - AnyJsExpression::JsStaticMemberExpression(expression) => expression.object()?, - AnyJsExpression::JsComputedMemberExpression(expression) => { - if matches!( - expression.member()?, - AnyJsExpression::AnyJsLiteralExpression( - AnyJsLiteralExpression::JsStringLiteralExpression(_) - | AnyJsLiteralExpression::JsNumberLiteralExpression(_) - ) | AnyJsExpression::JsIdentifierExpression(_) - ) { - expression.object()? - } else { - break; - } - } - AnyJsExpression::JsIdentifierExpression(_) | AnyJsExpression::JsThisExpression(_) => { - return Ok(true); - } - _ => { - break; - } - } - } - - Ok(false) -} - enum TemplateElementIterator { JsTemplateElementList(AstNodeListIterator), TsTemplateElementList(AstNodeListIterator), diff --git a/crates/biome_js_formatter/src/utils/test_each_template.rs b/crates/biome_js_formatter/src/utils/test_each_template.rs index 3832055fe63e..504e91002cd1 100644 --- a/crates/biome_js_formatter/src/utils/test_each_template.rs +++ b/crates/biome_js_formatter/src/utils/test_each_template.rs @@ -1,5 +1,5 @@ use crate::js::auxiliary::template_element::TemplateElementOptions; -use crate::js::lists::template_element_list::{TemplateElementIndention, TemplateElementLayout}; +use crate::js::lists::template_element_list::TemplateElementIndention; use crate::prelude::*; use biome_formatter::printer::Printer; use biome_formatter::{ @@ -243,7 +243,6 @@ impl EachTemplateTable { let options = TemplateElementOptions { after_new_line: false, indention: TemplateElementIndention::default(), - layout: TemplateElementLayout::Fit, }; // print the current column with infinite print width From e0837ff3253c38e359eaad594266a261f0ee1b5c Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Mon, 6 May 2024 13:53:18 +0800 Subject: [PATCH 02/13] update snapshots --- .../js/chain-expression/issue-15785-3.js.snap | 59 - .../js/multiparser-css/issue-5697.js.snap | 71 ++ .../js/multiparser-html/lit-html.js.snap | 22 +- .../js/template-literals/expressions.js.snap | 178 --- .../js/template-literals/indention.js.snap | 228 ---- .../sequence-expressions.js.snap | 31 - .../specs/prettier/jsx/text-wrap/test.js.snap | 1070 ----------------- .../template-literal.ts.snap | 65 - .../template-literals/as-expression.ts.snap | 65 - 9 files changed, 77 insertions(+), 1712 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/chain-expression/issue-15785-3.js.snap create mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/multiparser-css/issue-5697.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/template-literals/expressions.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/template-literals/indention.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/template-literals/sequence-expressions.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/jsx/text-wrap/test.js.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/typescript/satisfies-operators/template-literal.ts.snap delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/typescript/template-literals/as-expression.ts.snap diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/chain-expression/issue-15785-3.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/chain-expression/issue-15785-3.js.snap deleted file mode 100644 index c1d6f1320a57..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/chain-expression/issue-15785-3.js.snap +++ /dev/null @@ -1,59 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/chain-expression/issue-15785-3.js ---- -# Input - -```js -logger.log( - `A long template string with a conditional: ${channel?.id}, and then some more content that continues until ${JSON.stringify(location)}` -); -logger.log( - `A long template string with a conditional: ${channel.id}, and then some more content that continues until ${JSON.stringify(location)}` -); - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -1,6 +1,14 @@ - logger.log( -- `A long template string with a conditional: ${channel?.id}, and then some more content that continues until ${JSON.stringify(location)}`, -+ `A long template string with a conditional: ${ -+ channel?.id -+ }, and then some more content that continues until ${JSON.stringify( -+ location, -+ )}`, - ); - logger.log( -- `A long template string with a conditional: ${channel.id}, and then some more content that continues until ${JSON.stringify(location)}`, -+ `A long template string with a conditional: ${ -+ channel.id -+ }, and then some more content that continues until ${JSON.stringify( -+ location, -+ )}`, - ); -``` - -# Output - -```js -logger.log( - `A long template string with a conditional: ${ - channel?.id - }, and then some more content that continues until ${JSON.stringify( - location, - )}`, -); -logger.log( - `A long template string with a conditional: ${ - channel.id - }, and then some more content that continues until ${JSON.stringify( - location, - )}`, -); -``` diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-css/issue-5697.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-css/issue-5697.js.snap new file mode 100644 index 000000000000..6853cab36e3f --- /dev/null +++ b/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-css/issue-5697.js.snap @@ -0,0 +1,71 @@ +--- +source: crates/biome_formatter_test/src/snapshot_builder.rs +info: js/multiparser-css/issue-5697.js +--- +# Input + +```js +const StyledH1 = styled.div` + font-size: 2.5em; + font-weight: ${(props) => (props.strong ? 500 : 100)}; + font-family: ${constants.text.displayFont.fontFamily}; + letter-spacing: ${(props) => (props.light ? '0.04em' : 0)}; + color: ${(props) => props.textColor}; + ${(props) => + props.center + ? ` display: flex; + align-items: center; + justify-content: center; + text-align: center;` + : ''} + @media (max-width: ${(props) => (props.noBreakPoint ? '0' : constants.layout.breakpoint.break1)}px) { + font-size: 2em; + } +`; + +``` + + +# Prettier differences + +```diff +--- Prettier ++++ Biome +@@ -11,8 +11,7 @@ + justify-content: center; + text-align: center;` + : ""} +- @media (max-width: ${(props) => +- props.noBreakPoint ? "0" : constants.layout.breakpoint.break1}px) { ++ @media (max-width: ${(props) => (props.noBreakPoint ? "0" : constants.layout.breakpoint.break1)}px) { + font-size: 2em; + } + `; +``` + +# Output + +```js +const StyledH1 = styled.div` + font-size: 2.5em; + font-weight: ${(props) => (props.strong ? 500 : 100)}; + font-family: ${constants.text.displayFont.fontFamily}; + letter-spacing: ${(props) => (props.light ? "0.04em" : 0)}; + color: ${(props) => props.textColor}; + ${(props) => + props.center + ? ` display: flex; + align-items: center; + justify-content: center; + text-align: center;` + : ""} + @media (max-width: ${(props) => (props.noBreakPoint ? "0" : constants.layout.breakpoint.break1)}px) { + font-size: 2em; + } +`; +``` + +# Lines exceeding max width of 80 characters +``` + 14: @media (max-width: ${(props) => (props.noBreakPoint ? "0" : constants.layout.breakpoint.break1)}px) { +``` diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-html/lit-html.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-html/lit-html.js.snap index c1e16894fbad..7b07d4799d73 100644 --- a/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-html/lit-html.js.snap +++ b/crates/biome_js_formatter/tests/specs/prettier/js/multiparser-html/lit-html.js.snap @@ -2,7 +2,6 @@ source: crates/biome_formatter_test/src/snapshot_builder.rs info: js/multiparser-html/lit-html.js --- - # Input ```js @@ -117,7 +116,7 @@ ${ foo}:${bar}; ```diff --- Prettier +++ Biome -@@ -14,48 +14,63 @@ +@@ -14,48 +14,59 @@ render() { return html` @@ -192,15 +191,11 @@ ${ foo}:${bar}; - const tpl = html\`
\${innerExpr(1)} ${outerExpr(2)}
\`; - `; +const trickyParens = html``; -+const nestedFun = /* HTML */ `${outerExpr( -+ 1, -+)} `; ++const nestedFun = /* HTML */ `${outerExpr(1)} `; const closingScriptTagShouldBeEscapedProperly = /* HTML */ ` `; @@ -320,11 +315,7 @@ function HelloWorld() { } const trickyParens = html``; -const nestedFun = /* HTML */ `${outerExpr( - 1, -)} `; +const nestedFun = /* HTML */ `${outerExpr(1)} `; const closingScriptTagShouldBeEscapedProperly = /* HTML */ ` `; + 77: const closingScriptTag2 = /* HTML */ `