From c733d6ebe8c6ca8d97562028d7ea3c3b4be5b723 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 28 Apr 2022 17:25:06 +0100 Subject: [PATCH] fix(rome_js_formatter): break computed expressions like prettier --- .../expressions/computed_member_expression.rs | 61 ++++++++++++++++--- .../specs/js/module/object/computed_member.js | 6 ++ .../js/module/object/computed_member.js.snap | 21 +++++++ .../prettier/js/assignment/issue-7091.js.snap | 9 ++- .../js/binary-expressions/comment.js.snap | 21 ++++--- .../specs/prettier/js/member/expand.js.snap | 9 ++- .../js/method-chain/break-last-member.js.snap | 21 ++++--- .../js/template-literals/expressions.js.snap | 16 ++--- .../js/ternaries/indent-after-paren.js.snap | 53 +++++++++------- .../typescript/cast/generic-cast.ts.snap | 12 ++-- 10 files changed, 160 insertions(+), 69 deletions(-) diff --git a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs index 52c5c8bb5c2..d2163f7ec87 100644 --- a/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs +++ b/crates/rome_js_formatter/src/js/expressions/computed_member_expression.rs @@ -1,29 +1,70 @@ use crate::format_traits::FormatOptional; -use rome_formatter::FormatResult; +use rome_formatter::{ + concat_elements, group_elements, soft_block_indent, soft_line_break, FormatResult, +}; use crate::{format_elements, Format, FormatElement, FormatNode, Formatter}; use rome_js_syntax::JsComputedMemberExpression; use rome_js_syntax::JsComputedMemberExpressionFields; +use rome_rowan::AstNode; impl FormatNode for JsComputedMemberExpression { fn format_fields(&self, formatter: &Formatter) -> FormatResult { + let mut current = self.clone(); + + // Find the left most computed expression + while let Some(computed_expression) = + JsComputedMemberExpression::cast(current.object()?.syntax().clone()) + { + current = computed_expression; + } + + // Format the left most computed expression let JsComputedMemberExpressionFields { object, optional_chain_token, l_brack_token, member, r_brack_token, - } = self.as_fields(); + } = current.as_fields(); - let optional_chain_token = optional_chain_token.format_or_empty(formatter)?; - - Ok(format_elements![ + let mut formatted = vec![format_elements![ object.format(formatter)?, - optional_chain_token, - l_brack_token.format(formatter)?, - member.format(formatter)?, - r_brack_token.format(formatter)?, - ]) + group_elements(format_elements![ + optional_chain_token.format_or_empty(formatter)?, + l_brack_token.format(formatter)?, + soft_line_break(), + soft_block_indent(member.format(formatter)?), + r_brack_token.format(formatter)?, + ]), + ]]; + + // Traverse upwards again and concatenate the computed expression until we find the first non-computed expression + while let Some(parent) = current.syntax().parent() { + if let Some(parent_computed) = JsComputedMemberExpression::cast(parent) { + let JsComputedMemberExpressionFields { + object: _, + optional_chain_token, + l_brack_token, + member, + r_brack_token, + } = parent_computed.as_fields(); + + formatted.push(group_elements(format_elements![ + optional_chain_token.format_or_empty(formatter)?, + l_brack_token.format(formatter)?, + soft_line_break(), + soft_block_indent(member.format(formatter)?), + r_brack_token.format(formatter)?, + ])); + + current = parent_computed; + } else { + break; + } + } + + Ok(concat_elements(formatted)) } } diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js b/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js index 571dea5b24f..472eee4a462 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js +++ b/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js @@ -6,3 +6,9 @@ foo.bar["bar"]["lorem_ispsum"].foo["lorem-ipsum"] = true; a[ b ] c?.[ d ] + +x["very"]["long"]["chain"]["of"]["computed"]["members"]["that"]["goes"]["on"]["for"]["ever"]["I"]["mean"]["it"]["for"]["ever"]["and"]["even"]["longer"]["than"]["that"] + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever["and"].even.longer.than.that.and["rofefefeeffeme"].doesnt.supportit + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever[and()].even.longer.than.that.and["rofefefeefefme"].doesnt.supportit diff --git a/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js.snap b/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js.snap index ec861a696bb..2ac0b4745ef 100644 --- a/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js.snap +++ b/crates/rome_js_formatter/tests/specs/js/module/object/computed_member.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/spec_test.rs +assertion_line: 242 expression: computed_member.js --- # Input @@ -12,6 +13,12 @@ foo.bar["bar"]["lorem_ispsum"].foo["lorem-ipsum"] = true; a[ b ] c?.[ d ] +x["very"]["long"]["chain"]["of"]["computed"]["members"]["that"]["goes"]["on"]["for"]["ever"]["I"]["mean"]["it"]["for"]["ever"]["and"]["even"]["longer"]["than"]["that"] + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever["and"].even.longer.than.that.and["rofefefeeffeme"].doesnt.supportit + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever[and()].even.longer.than.that.and["rofefefeefefme"].doesnt.supportit + ============================= # Outputs ## Output 1 @@ -29,3 +36,17 @@ foo.bar["bar"]["lorem_ispsum"].foo["lorem-ipsum"] = true; a[b]; c?.[d]; +x["very"]["long"]["chain"]["of"]["computed"]["members"]["that"]["goes"]["on"][ + "for" +]["ever"]["I"]["mean"]["it"]["for"]["ever"]["and"]["even"]["longer"]["than"][ + "that" +]; + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever[ + "and" +].even.longer.than.that.and["rofefefeeffeme"].doesnt.supportit; + +x.very.long.chain.of.static.members.that.goes.on.for.ever.I.mean.it.for.ever[ + and() +].even.longer.than.that.and["rofefefeefefme"].doesnt.supportit; + diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/issue-7091.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/issue-7091.js.snap index ff8d57e6d69..d7facc4f669 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/assignment/issue-7091.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/assignment/issue-7091.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: issue-7091.js --- # Input @@ -12,12 +13,10 @@ const { # Output ```js -const { imStore, showChat, customerServiceAccount } = store[config.reduxStoreName]; +const { imStore, showChat, customerServiceAccount } = store[ + config.reduxStoreName +]; ``` -# Lines exceeding max width of 80 characters -``` - 1: const { imStore, showChat, customerServiceAccount } = store[config.reduxStoreName]; -``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/binary-expressions/comment.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/binary-expressions/comment.js.snap index 862dc7a2f08..bfa546d446b 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/binary-expressions/comment.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/binary-expressions/comment.js.snap @@ -1,8 +1,7 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs -assertion_line: 125 +assertion_line: 175 expression: comment.js - --- # Input ```js @@ -97,14 +96,18 @@ var a = x( bifornCringerMoshedPerplexSawder, ); -foo[a + - a + // comment +foo[ a + - bar[b + - b + - b + // comment - b + - b]]; + a + // comment + a + + bar[ + b + + b + + b + // comment + b + + b + ] +]; !( a + diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/member/expand.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/member/expand.js.snap index 39e5554d97a..52e624be039 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/member/expand.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/member/expand.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: expand.js --- # Input @@ -44,7 +45,9 @@ window.FooClient.something.setVars({ # Output ```js -const veryVeryVeryVeryVeryVeryVeryLong = doc.expandedStates[doc.expandedStates.length - 1]; +const veryVeryVeryVeryVeryVeryVeryLong = doc.expandedStates[ + doc.expandedStates.length - 1 +]; const small = doc.expandedStates[doc.expandedStates.length - 1]; const promises = [ @@ -94,8 +97,4 @@ window.FooClient.something.setVars({ ``` -# Lines exceeding max width of 80 characters -``` - 1: const veryVeryVeryVeryVeryVeryVeryLong = doc.expandedStates[doc.expandedStates.length - 1]; -``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap index 5e3dc29af89..73bf791def3 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/method-chain/break-last-member.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: break-last-member.js --- # Input @@ -30,9 +31,13 @@ const { # Output ```js SomeVeryLongUpperCaseConstant.someVeryLongCallExpression().some_very_long_member_expression; -weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode.childrenInAnArray[0]; +weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode.childrenInAnArray[ + 0 +]; superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered; -superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[0]; +superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[ + 0 +]; expect( findDOMNode(component.instance()).getElementsByClassName(styles.inner)[0].style.paddingRight, @@ -54,11 +59,11 @@ const { # Lines exceeding max width of 80 characters ``` 1: SomeVeryLongUpperCaseConstant.someVeryLongCallExpression().some_very_long_member_expression; - 2: weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode.childrenInAnArray[0]; - 3: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered; - 4: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[0]; - 7: findDOMNode(component.instance()).getElementsByClassName(styles.inner)[0].style.paddingRight, - 10: const { course, conflicts = [], index, scheduleId, studentId, something } = a.this.props; - 12: const { course2, conflicts2 = [], index2, scheduleId2, studentId2, something2 } = this.props; + 2: weNeedToReachTheEightyCharacterLimitXXXXXXXXXXXXXXXXX.someNode.childrenInAnArray[ + 5: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered; + 6: superSupersuperSupersuperSupersuperSupersuperSuperLong.exampleOfOrderOfGetterAndSetterReordered[ + 11: findDOMNode(component.instance()).getElementsByClassName(styles.inner)[0].style.paddingRight, + 14: const { course, conflicts = [], index, scheduleId, studentId, something } = a.this.props; + 16: const { course2, conflicts2 = [], index2, scheduleId2, studentId2, something2 } = this.props; ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/template-literals/expressions.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/template-literals/expressions.js.snap index 11fd834d16b..e16141f9998 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/template-literals/expressions.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/template-literals/expressions.js.snap @@ -69,7 +69,9 @@ const foo = `such a long template string ${foo.bar.baz} that prettier will want const shouldWrapForNow = `such a long template string ${foo().bar.baz} that prettier will want to wrap it`; -const shouldNotWrap = `simple expressions should not break ${this} ${variable} ${a.b.c} ${this.b.c} ${a[b].c} ${a.b[c]} ${a.b["c"]} ${a?.b?.c}`; +const shouldNotWrap = `simple expressions should not break ${this} ${variable} ${a.b.c} ${this.b.c} ${a[ + b +].c} ${a.b[c]} ${a.b["c"]} ${a?.b?.c}`; console.log( chalk.white( @@ -132,11 +134,11 @@ throw new Error( 8: const description = `The value of the ${cssName} css of the ${this._name} element`; 10: const foo = `such a long template string ${foo.bar.baz} that prettier will want to wrap it`; 12: const shouldWrapForNow = `such a long template string ${foo().bar.baz} that prettier will want to wrap it`; - 14: const shouldNotWrap = `simple expressions should not break ${this} ${variable} ${a.b.c} ${this.b.c} ${a[b].c} ${a.b[c]} ${a.b["c"]} ${a?.b?.c}`; - 18: `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`, - 32: return `${process.env.OPENID_URL}/something/something/something?${Object.keys( - 42: `Trying update appcast for ${app.name} (${app.cask.appcast}) -> (${app.cask.appcastGenerated})`, - 50: `\nApparently jetbrains changed the release artifact for ${app.name}@${app.jetbrains.version}.\n`, - 64: `pretty-format: Option "theme" has a key "${key}" whose value "${value}" is undefined in ansi-styles.`, + 14: const shouldNotWrap = `simple expressions should not break ${this} ${variable} ${a.b.c} ${this.b.c} ${a[ + 20: `Covered Lines below threshold: ${coverageSettings.lines}%. Actual: ${coverageSummary.total.lines.pct}%`, + 34: return `${process.env.OPENID_URL}/something/something/something?${Object.keys( + 44: `Trying update appcast for ${app.name} (${app.cask.appcast}) -> (${app.cask.appcastGenerated})`, + 52: `\nApparently jetbrains changed the release artifact for ${app.name}@${app.jetbrains.version}.\n`, + 66: `pretty-format: Option "theme" has a key "${key}" whose value "${value}" is undefined in ansi-styles.`, ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/ternaries/indent-after-paren.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/ternaries/indent-after-paren.js.snap index 03643886ea3..bc83905fccf 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/ternaries/indent-after-paren.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/ternaries/indent-after-paren.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: indent-after-paren.js --- # Input @@ -519,7 +520,9 @@ foo = )()()(); foo = - foo.bar.baz[coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz]; + foo.bar.baz[ + coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz + ]; const decorated = (arg, ignoreRequestError) => { return ( @@ -559,22 +562,30 @@ fn?.( ); bifornCringerMoshedPerplexSawder = - fn[( - glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - ).prop]; + fn[ + ( + glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + ).prop + ]; -fn[( - glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol -).prop]; +fn[ + ( + glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + ).prop +]; bifornCringerMoshedPerplexSawder = - fn?.[( - glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - ).prop]; + fn?.[ + ( + glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + ).prop + ]; -fn?.[( - glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol -).prop]; +fn?.[ + ( + glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + ).prop +]; ``` @@ -618,14 +629,14 @@ fn?.[( 203: glimseGlyphsHazardNoopsTieTie === 0 && kochabCooieGameOnOboleUnweave === Math.PI ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol 210: coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz 215: coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz - 219: foo.bar.baz[coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz]; - 235: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 241: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 248: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 254: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 260: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 264: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 220: coooooooooooooooooooooooooooooooooooooooooooooooooooond ? baaaaaaaaaaaaaaaaaaaaar : baaaaaaaaaaaaaaaaaaaaaz + 237: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 243: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 250: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 256: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 263: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol 269: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol - 273: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 276: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol + 282: glimseGlyphsHazardNoopsTieTie === 0 ? averredBathersBoxroomBuggyNurl : anodyneCondosMalateOverateRetinol ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap index 4908a84b57b..1ba91e0d48a 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/cast/generic-cast.ts.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 175 expression: generic-cast.ts --- # Input @@ -172,7 +173,9 @@ function x() { const fitsObjLiteral = | undefined>{ a: "test" }; const fitsArrayLiteral = | undefined>["t1", "t2"]; - const breakAfterCast = | undefined>(permissions)[receiverType]; + const breakAfterCast = | undefined>(permissions)[ + receiverType + ]; const stillTooLong = < PermissionsChecker | undefined | number | string | boolean @@ -200,7 +203,9 @@ function x() { | boolean | null | never - >(permissions.someMethodWithLongName(parameter1, parameter2))[receiverTypeLongName]; + >(permissions.someMethodWithLongName(parameter1, parameter2))[ + receiverTypeLongName + ]; const testObjLiteral = | undefined>{ prop1: "myPropVal", @@ -244,7 +249,6 @@ function x() { # Lines exceeding max width of 80 characters ``` 7: const breakAfterCast = >someExistingConfigMap.mergeDeep( - 110: const breakAfterCast = | undefined>(permissions)[receiverType]; - 138: >(permissions.someMethodWithLongName(parameter1, parameter2))[receiverTypeLongName]; + 110: const breakAfterCast = | undefined>(permissions)[ ```