Add spaces to template literals when using parenSpacing

[scinos]

Changes:

• It adds an extra space to template literals when using parenSpace
• Adds lots of tests for parenSpacing

Fixes #19

[scinos]

To generate the tests, I copy/pasted a few existing tests from prettier (so they covers most of what prettier js covers), specially those related to parentheses.

Then I enabled parenSpacing: true and re-generated the snapshots.

For reference, these are the differences between parenSpacing: true and parenSpacing: false (i.e. the difference between wp-prettier and prettier, so to speak):

spacing.diff.txt (download it and open it with your favourite diff viewer)

[sirreal]

Here's the diff in an easy to review format:

Expand to view the diff

```diff diff --git b/tests/paren_space/__snapshots__/jsfmt.spec.js.snap a/tests/paren_space/__snapshots__/jsfmt.spec.js.snap index ee642b8b..7982c666 100644 --- b/tests/paren_space/__snapshots__/jsfmt.spec.js.snap +++ a/tests/paren_space/__snapshots__/jsfmt.spec.js.snap @@ -18,15 +18,15 @@ printWidth: 80 [...a, ...b]; =====================================output===================================== -[[]]; -[[], []]; -[[], [], []]; -[[], [0], []]; -[[], [0], [0]]; -[[], [0, 1], [0]]; -[[], [0, 1], [0, 1]]; -[...a, ...b]; -[...a, ...b]; +[ [] ]; +[ [], [] ]; +[ [], [], [] ]; +[ [], [ 0 ], [] ]; +[ [], [ 0 ], [ 0 ] ]; +[ [], [ 0, 1 ], [ 0 ] ]; +[ [], [ 0, 1 ], [ 0, 1 ] ]; +[ ...a, ...b ]; +[ ...a, ...b ]; ================================================================================ `; @@ -78,44 +78,44 @@ a => ({}:: b()\`\`[''].c++ && 0 ? 0 : 0); a:: (b => c); =====================================output===================================== -(a || b)::c; +( a || b )::c; a || b::c; ::obj.prop; -(void 0)::func(); -(+0)::is(-0); +( void 0 )::func(); +( +0 )::is( -0 ); a::b.c; -a::(b.c()); +a::( b.c() ); a::b.c(); -a::(b.c()()); -a::(b.c()()); -a::(b.c())(); -a::(b.c().d); -a::(c().d.e); -a::(b()); -a::(b::c()); -a::(b()::c); -a::(b().c::d); -a::(b.c::d); -a::(b::c.d); -a::(b.c::d::e); -a::(b::c::d); -a::(b::c::d.e); -a::(b::c::d).e; -a::(void 0); -a::(b.c()::d.e); -a::(b.c::d.e); -a::(b.c::d.e)::f.g; +a::( b.c()() ); +a::( b.c()() ); +a::( b.c() )(); +a::( b.c().d ); +a::( c().d.e ); +a::( b() ); +a::( b::c() ); +a::( b()::c ); +a::( b().c::d ); +a::( b.c::d ); +a::( b::c.d ); +a::( b.c::d::e ); +a::( b::c::d ); +a::( b::c::d.e ); +a::( b::c::d ).e; +a::( void 0 ); +a::( b.c()::d.e ); +a::( b.c::d.e ); +a::( b.c::d.e )::f.g; b.c::d.e; -(b.c::d).e; -(b::c::d).e; -new (a::b)(); -new f(a::b); -f[a::b]; -f[a::b()]; +( b.c::d ).e; +( b::c::d ).e; +new ( a::b )(); +new f( a::b ); +f[ a::b ]; +f[ a::b() ]; -(a) => ({}::b()\`\`[""].c++ && 0 ? 0 : 0); -((a) => b)::c; -a::((b) => c); +( a ) => ( {}::b()\`\`[ "" ].c++ && 0 ? 0 : 0 ); +( ( a ) => b )::c; +a::( ( b ) => c ); ================================================================================ `; @@ -144,13 +144,13 @@ class X { =====================================output===================================== class c { - ["i"]() {} + [ "i" ]() {} } -(class {} + 1); -(class a {} + 1); -(class extends b {} + 1); -(class a extends b {} + 1); +( class {} + 1 ); +( class a {} + 1 ); +( class extends b {} + 1 ); +( class a extends b {} + 1 ); class X { async *b() { @@ -205,17 +205,17 @@ if ( } const radioSelectedAttr = - (isAnyValueSelected && - node.getAttribute(radioAttr.toLowerCase()) === radioValue) || - (!isAnyValueSelected && values[a].default === true) || + ( isAnyValueSelected && + node.getAttribute( radioAttr.toLowerCase() ) === radioValue ) || + ( ! isAnyValueSelected && values[ a ].default === true ) || a === 0; function f() { - if (position) return { name: pair }; + if ( position ) return { name: pair }; else return { - name: pair.substring(0, position), - value: pair.substring(position + 1), + name: pair.substring( 0, position ), + value: pair.substring( position + 1 ), }; } @@ -241,8 +241,8 @@ a = ([s = 1,]) => 1 const { children, ...props } = this.props =====================================output===================================== -const [one, two = null, three = null] = arr; -a = ([s = 1]) => 1; +const [ one, two = null, three = null ] = arr; +a = ( [ s = 1 ] ) => 1; const { children, ...props } = this.props; ================================================================================ @@ -272,19 +272,19 @@ new (factory())(factory()) object.foo().bar().baz(); =====================================output===================================== -(this.x++).toString(); +( this.x++ ).toString(); -new (r++)(); +new ( r++ )(); -(x++)(); +( x++ )(); -const uuid = String(this._uuidCounter++); +const uuid = String( this._uuidCounter++ ); -!( +! ( { a: 1, b: 2 } // foo ); -new (factory())(factory()); +new ( factory() )( factory() ); object.foo().bar().baz(); @@ -320,24 +320,24 @@ new function () { }; =====================================output===================================== function* f() { - yield async (a) => a; + yield async ( a ) => a; } async function f3() { - a = (await 1) ? 1 : 1; + a = ( await 1 ) ? 1 : 1; } -(function () {}.length); +( function () {}.length ); typeof function () {}; -export default (function () {})(); -(function () {})()\`\`; -(function () {})\`\`; -new (function () {})(); -(function () {}); +export default ( function () {} )(); +( function () {} )()\`\`; +( function () {} )\`\`; +new ( function () {} )(); +( function () {} ); a = function f() {} || b; -(function () {} && a); +( function () {} && a ); a + function () {}; -new (function () {})(); +new ( function () {} )(); ================================================================================ `; @@ -362,13 +362,13 @@ export default (function f() { }) =====================================output===================================== import { fitsIn, oneLine } from "."; -import("module.js").then((a) => a); +import( "module.js" ).then( ( a ) => a ); export a, { b } from "./baz"; export default () => {}; -export default (function f() {}); +export default ( function f() {} ); ================================================================================ `; @@ -415,16 +415,16 @@ do {} while ( ) ); -for (var i = 0, len = arr.length; i < len; i++) {} +for ( var i = 0, len = arr.length; i < len; i++ ) {} -(async () => { - for await (num of asyncIterable) { - console.log(num); +( async () => { + for await ( num of asyncIterable ) { + console.log( num ); } - for await (const x of delegate_yield()) { + for await ( const x of delegate_yield() ) { x; } -})(); +} )(); ================================================================================ `; @@ -444,7 +444,7 @@ foo ?? (bar ?? baz); (foo ?? bar) ?? baz; =====================================output===================================== -const x = (foo, bar = foo ?? bar) => {}; +const x = ( foo, bar = foo ?? bar ) => {}; foo ? bar ?? foo : baz; @@ -472,15 +472,15 @@ a = () => ({}).x; (({} = 0), 1); =====================================output===================================== -() => ({}\`\`); -({}\`\`); -a = () => ({}.x); -({} && a, b); -({}::b, 0); -({}::b()\`\`[""].c++ && 0 ? 0 : 0, 0); -({}(), 0); -({} = 0); -({} = 0), 1; +() => ( {}\`\` ); +( {}\`\` ); +a = () => ( {}.x ); +( {} && a, b ); +( {}::b, 0 ); +( {}::b()\`\`[ "" ].c++ && 0 ? 0 : 0, 0 ); +( {}(), 0 ); +( {} = 0 ); +( {} = 0 ), 1; ================================================================================ `; @@ -526,31 +526,31 @@ printWidth: 80 (a = b ? c : function () { return 0; }); =====================================output===================================== -(a = b +( a = b ? c : function () { return 0; - }), - (a = b + } ), + ( a = b ? c : function () { return 0; - }), - (a = b + } ), + ( a = b ? c : function () { return 0; - }), - (a = b + } ), + ( a = b ? c : function () { return 0; - }), - (a = b + } ), + ( a = b ? c : function () { return 0; - }); + } ); ================================================================================ `; @@ -571,14 +571,14 @@ async () => ({ ...(await foo) }); declare class C { f(...r): void; } =====================================output===================================== -const foo = { ...(a || b) }; -const foo2 = { ...(a || b) }; -const foo3 = { ...(a ? b : c) }; +const foo = { ...( a || b ) }; +const foo2 = { ...( a || b ) }; +const foo3 = { ...( a ? b : c ) }; -async () => ({ ...(await foo) }); +async () => ( { ...( await foo ) } ); declare class C { - f(...r): void; + f( ...r ): void; } ================================================================================ @@ -606,18 +606,18 @@ switch (a) { } =====================================output===================================== -switch (a) { +switch ( a ) { case 3: - alert("3"); + alert( "3" ); break; case 4: - alert("4"); + alert( "4" ); break; case 5: - alert("5"); + alert( "5" ); break; default: - alert("default"); + alert( "default" ); } ================================================================================ @@ -667,9 +667,9 @@ return (

) =====================================output===================================== -\`\${1 ?? 2}\`; +\`\${ 1 ?? 2 }\`; -\`glp-text-\${isImagePresent ? 56 : 64}@M\`; +\`glp-text-\${ isImagePresent ? 56 : 64 }@M\`; const headerResolve = css.resolve\` .top-bar { @@ -695,7 +695,7 @@ const headerResolve = css.resolve\` return (

); @@ -723,7 +723,7 @@ printWidth: 80 a => a ? () => { a } : () => { a } =====================================output===================================== -(a) => +( a ) => a ? () => { a; @@ -755,9 +755,9 @@ function f() { throw foo.bar(); } -lint(ast, { - with: () => throw new Error("avoid using 'with' statements."), -}); +lint( ast, { + with: () => throw new Error( "avoid using 'with' statements." ), +} ); ================================================================================ `; @@ -776,7 +776,7 @@ finally { await baz() } =====================================output===================================== try { foo(); -} catch (e) { +} catch ( e ) { bar(); } finally { await baz(); @@ -795,7 +795,7 @@ printWidth: 80 with (0) { }; =====================================output===================================== -with (0) { +with ( 0 ) { } ================================================================================ ```

[sirreal]

The paren spacing snapshot diff looks about right to me. Let's apply this change (https://github.com/Automattic/wp-prettier/pull/20/files#r411954955) verify it's working correctly, then merge.

[scinos]

I can't figure out why the tests are failing. What I know is they are not related to this particular change.

So I propose merge this as-is to fix #19, release it as wp-prettier@2.0.5-alpha-1, and investigate the broken test later.

@sirreal @sgomes @griffbrad @jsnajdr opinions?

Note about the numbering scheme:

• 2.0.4 is used, and anything like 2.0.4-xxx will have a lower precedence (so people won't get it if they already have 2.0.4)
• We could use 2.0.5-alpha-1, 2.0.5-alpha-2... for all the patches we apply on top of 2.0.4.
• When prettier releases 2.0.5, we merge it an release it as 2.0.5-beta-1
• If we need to fix things, we can do 2.0.5-beta-2...
• When we are confident that our 2.0.5-beta-xxx series is stable, we can release it as just 2.0.5.

Ugly, but I can't figure out a less-ugly option. Open to suggestions

[sirreal]

I can't reproduce the CI failures locally either 😕

I did notice that the checkout is a merge, it merges this branch into wp-prettier-2.0.4 before running the tests. I've done that locally and run yarn test --clearCache; yarn test --ci. Everything still passes.