fix & test handling of foo(, baz) (first argument missing)
#10023
Conversation
|
PR #9857 looks like a better approach, but it does not seem to cover function calls. |
|
I think this one is good enough before we decide what to do in #9979, can you fix this? Prettier pr-10023 --parser babelInput: a(,)Output: TypeError: Cannot read property 'type' of null
at isTemplateOnItsOwnLine (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:162062)
at printCallExpression (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:263948)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:270263
at Object.print (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:279870)
at Na (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:115224)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:114686
at printComments (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:111129)
at e (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:114677)
at ya.call (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:112909)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:268005 |
| @@ -0,0 +1 @@ | |||
| foo(, baz); | |||
There was a problem hiding this comment.
I wonder if we should test the cases with and without comments in each of the positions before and after the comma, for example:
| foo(, baz); | |
| foo(, baz); | |
| foo(,); | |
| foo(/*first*/, baz); | |
| foo(,/*second*/ baz); | |
| foo(, baz /* c */); | |
| foo(/* first */, /* second */ baz /* c */); | |
| foo(/* first */,); | |
| foo(, /* second */); | |
| foo(/* first */, /* second */); |
|
@fisker I may need a few days due to some urgent work, would not mind if you want to take this one over. I wonder if we should test with and without comments like I suggested on the test code. Thanks for looking into this. |
| @@ -0,0 +1 @@ | |||
| run_spec(__dirname, ["babel-flow", "babel-ts"]); | |||
There was a problem hiding this comment.
When I tried with "babel" I did encounter what I think was a parse error. I think it would be ideal if we could test with ["babel", "babel-ts"] or event ["babel", "typescript"].
There was a problem hiding this comment.
Two cases not good:
Prettier pr-10023
Playground link
--parser babelInput:
foo(/*bar*/, baz)Output:
foo /*bar*/(, baz);Prettier pr-10023
Playground link
--parser babelInput:
foo(/*bar*/, /*baz*/)Output:
TypeError: Cannot read property 'type' of null
at isTemplateOnItsOwnLine (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:162062)
at printCallExpression (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:263948)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:270263
at Object.print (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:279870)
at Na (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:115224)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:114686
at printComments (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:111129)
at e (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:114677)
at ya.call (https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:112909)
at https://deploy-preview-10023--prettier.netlify.app/lib/standalone.js:15:268005
|
Closing this as Prettier doesn't try to print code with omitted call arguments anymore. See #10065. |
Description
from this review comment on PR #9992: #9992 (comment)
Without these changes, code like
foo(, baz)would lead to an ugly exception withbabel-flow&babel-tsparsers./cc @fisker
Checklist
(If changing the API or CLI) I’ve documented the changes I’ve made (in thedocs/directory).(If the change is user-facing) I’ve added my changes tochangelog_unreleased/*/XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨