dprint / dprint-plugin-typescript

TypeScript and JavaScript code formatting plugin for dprint.
https://dprint.dev/plugins/typescript
MIT License
256 stars 57 forks source link

Arrow function parameters containing comment has parens removed incorrectly #533

Closed jakebailey closed 1 year ago

jakebailey commented 1 year ago

Describe the bug

dprint-plugin-typescript version: 0.85.0

Input Code

client
    .send(lookup)
    .then((
        resp, // $ExpectType Batch<Lookup>
    ) => console.log(resp))
    .catch((
        err, // $ExpectType any
    ) => console.error(err));

Config:

{
  "arrowFunction.useParentheses": "preferNone"
}

Expected Output

client
    .send(lookup)
    .then((
        resp, // $ExpectType Batch<Lookup>
    ) => console.log(resp))
    .catch((
        err, // $ExpectType any
    ) => console.error(err));

(or something)

Actual Output

client
    .send(lookup)
    .then(resp // $ExpectType Batch<Lookup>
     => console.log(resp))
    .catch(err // $ExpectType any
     => console.error(err));

Hit roughly 4 times in testing DT with dprint.

jakebailey commented 1 year ago

Actually, I'm confused. This code doesn't look illegal in the actual output, right?

Here are the actual errors:

Error formatting /home/jabaile/work/DefinitelyTyped/types/smartystreets-javascript-sdk/smartystreets-javascript-sdk-tests.ts. Message: Formatting succeeded initially, but failed when ensuring a stable format. This is most likely a bug in the plugin where the text it produces is not syntatically correct. Please report this as a bug to the plugin that formatted this file.

Expected ',', got '=>' at /home/jabaile/work/DefinitelyTyped/types/smartystreets-javascript-sdk/smartystreets-javascript-sdk-tests.ts:23:6

       => console.log(resp))
       ~~
Error formatting /home/jabaile/work/DefinitelyTyped/types/zdf/zdf-tests.ts. Message: Formatting succeeded initially, but failed when ensuring a stable format. This is most likely a bug in the plugin where the text it produces is not syntatically correct. Please report this as a bug to the plugin that formatted this file.

Expected ',', got '=>' at /home/jabaile/work/DefinitelyTyped/types/zdf/zdf-tests.ts:7:6

       => {},
       ~~
Error formatting /home/jabaile/work/DefinitelyTyped/types/gtag.js/gtag.js-tests.ts. Message: Formatting succeeded initially, but failed when ensuring a stable format. This is most likely a bug in the plugin where the text it produces is not syntatically correct. Please report this as a bug to the plugin that formatted this file.

Expected ',', got '=>' at /home/jabaile/work/DefinitelyTyped/types/gtag.js/gtag.js-tests.ts:139:6

       => {},
       ~~
Error formatting /home/jabaile/work/DefinitelyTyped/types/airbnb__node-memwatch/airbnb__node-memwatch-tests.ts. Message: Formatting succeeded initially, but failed when ensuring a stable format. This is most likely a bug in the plugin where the text it produces is not syntatically correct. Please report this as a bug to the plugin that formatted this file.

Expected ',', got '=>' at /home/jabaile/work/DefinitelyTyped/types/airbnb__node-memwatch/airbnb__node-memwatch-tests.ts:9:2

   => {});
   ~~

So maybe this is actually a swc bug?

dsherret commented 1 year ago

The invalid output can be reduced to:

call(param
=> 5);

It seems typescript is able to parse this, but swc, babel and chrome can't, so I'm going to count this as a dprint bug. It should be keeping the parens in this case.

dsherret commented 1 year ago

I actually had a condition sort of for this already, but it didn't handle a trailing commas on the parameter 😅.

Fixed in 0.85.1.