rome / tools

Unified developer tools for JavaScript, TypeScript, and the web
https://docs.rome.tools/
MIT License
23.76k stars 663 forks source link

πŸ› Formatter introduces infinite parentheses around certain logic expressions over time #3735

Closed lgarron closed 1 year ago

lgarron commented 1 year ago

Environment information

Rome CLI version 10.0.1

What happened?

Consider the following code, meant to work around https://github.com/rome/tools/issues/3734 :

export function supported(): boolean {
  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
    navigator.credentials &&
    navigator.credentials.create &&
    navigator.credentials.get &&
    window.PublicKeyCredential
  );
}

This successfully prevents the formatter from breaking the // rome-ignore, but it seems Rome's formatter is not idempotent β€” it adds new parentheses on every formatting invocation. After a few runs of npx rome format you get:

export function supported(): boolean {
  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
    navigator.credentials &&
    navigator.credentials.create &&
    navigator.credentials.get &&
    window.PublicKeyCredential
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  )
  );
}

This is kind of hilarious when formatting on save, but I presume it's not intended behaviour.

Expected result

Rome's formatter is idempotent β€” in this case, if it adds parentheses then subsequent invocations keep the exact same parentheses without adding more.

(The parentheses are redundant in this particular case, though.)

Code of Conduct

lgarron commented 1 year ago

Note that if you modify the sample code by moving the // rome-ignore format comment one line up, this issue doesn't occur. So I can easily work around it. But I thought I'd file a bug to save others the headache, if this can be fixed.