This repository was archived by the owner on Aug 31, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 657
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(rome_js_formatter): Unnecessary parentheses for return/unary with…
… verbatim argument ## Summary This PR fixes #3735 by only adding parentheses around the argument if the argument isn't suppressed. This is necessary because the verbatim formatting of the argument includes the parentheses too. Formatting the parentheses in the return/unary formatting and as part of the verbatim results in duplicating the parentheses on every save. ## Test Plan I added a handful of new tests covering the problematic cases.
- Loading branch information
1 parent
33b5ce6
commit b1181da
Showing
7 changed files
with
172 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
.../rome_js_formatter/tests/specs/js/module/expression/unary_expression_verbatim_argument.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
!( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
a && b | ||
); | ||
|
||
|
45 changes: 45 additions & 0 deletions
45
..._js_formatter/tests/specs/js/module/expression/unary_expression_verbatim_argument.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
--- | ||
source: crates/rome_js_formatter/tests/spec_test.rs | ||
expression: unary_expression_verbatim_argument.js | ||
--- | ||
|
||
# Input | ||
|
||
```js | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
!( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
a && b | ||
); | ||
|
||
|
||
|
||
``` | ||
|
||
|
||
============================= | ||
|
||
# Outputs | ||
|
||
## Output 1 | ||
|
||
----- | ||
Indent style: Tab | ||
Line width: 80 | ||
Quote style: Double Quotes | ||
Quote properties: As needed | ||
Trailing comma: All | ||
Semicolons: Always | ||
----- | ||
|
||
```js | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
!( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
a && b | ||
); | ||
``` | ||
|
||
|
25 changes: 25 additions & 0 deletions
25
crates/rome_js_formatter/tests/specs/js/module/statement/return_verbatim_argument.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
function supported1(){ | ||
return ( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported2(){ | ||
return !( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported3(){ | ||
return ( | ||
// rome-ignore format: | ||
aVeryLongLogicalExpression && | ||
thatBreaksOverMultipleLines | ||
); | ||
} |
87 changes: 87 additions & 0 deletions
87
crates/rome_js_formatter/tests/specs/js/module/statement/return_verbatim_argument.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
--- | ||
source: crates/rome_js_formatter/tests/spec_test.rs | ||
expression: return_verbatim_argument.js | ||
--- | ||
|
||
# Input | ||
|
||
```js | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
function supported1(){ | ||
return ( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported2(){ | ||
return !( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported3(){ | ||
return ( | ||
// rome-ignore format: | ||
aVeryLongLogicalExpression && | ||
thatBreaksOverMultipleLines | ||
); | ||
} | ||
|
||
``` | ||
|
||
|
||
============================= | ||
|
||
# Outputs | ||
|
||
## Output 1 | ||
|
||
----- | ||
Indent style: Tab | ||
Line width: 80 | ||
Quote style: Double Quotes | ||
Quote properties: As needed | ||
Trailing comma: All | ||
Semicolons: Always | ||
----- | ||
|
||
```js | ||
// https://github.com/rome/tools/issues/3735 | ||
|
||
function supported1() { | ||
return ( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported2() { | ||
return !( | ||
// rome-ignore format: Work around https://github.com/rome/tools/issues/3734 | ||
// rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
a && b | ||
); | ||
} | ||
|
||
function supported3() { | ||
return ( | ||
// rome-ignore format: | ||
aVeryLongLogicalExpression && | ||
thatBreaksOverMultipleLines | ||
); | ||
} | ||
|
||
|
||
## Lines exceeding width of 80 characters | ||
|
||
6: // rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
14: // rome-ignore lint(style/useOptionalChain): Optional chaining creates more complicated ES2019 code | ||
``` | ||
|
||
|