Incorrect indenting of ternary with multi-line function calls with offsetTernaryExpressions: true

[brodybits]

updated:

Part 1a: Regression

Tell us about your environment

• ESLint Version: v7.19.0
• Node Version: v14.15.4
• npm Version: 6.14.10

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?

default

Please show your full configuration:

Here is my .eslintrc.yml configuration from https://github.com/brodybits/react-native-module-init, but with Prettier plugin items removed I think they are not important for this issue:

Configuration

parserOptions:
  ecmaVersion: 9

extends: standard

plugins:
  - jest

env: { jest/globals: true }

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

I am raising this with a redaction of the affected source code from https://github.com/brodybits/react-native-module-init, which is not formatted right by eslint v7.19.0 as I said in brodybits/react-native-module-init#90 (comment).

Here is the redaction of the affected source code that I have made in await-prompt1.js:

const prompts = require('prompts')

function onPromptState ({ aborted }) {
  if (aborted) {
    process.exit(1)
  }
}

function prompt (props) {
  return prompts([{ onState: onPromptState, ...props }])
}

Promise.resolve().then(async () => {
  const platforms = ['android', 'ios']

  const { androidPackageId } =
    platforms.indexOf('android') !== -1
      ? await prompt({
          type: 'text',
          name: 'androidPackageId',
          message: 'What is the desired Android package id?',
          initial: 'com.demo',
          validate: androidPackageId => androidPackageId.length > 0
        })
      : { androidPackageId: null }

  console.log({ androidPackageId })
})

npx eslint await-prompt1.js

What did you expect to happen?

Accepts the formatting as-is, like eslint 7.18.0

What actually happened? Please include the actual, raw output from ESLint.

/Users/brodybits/dev/react-native-module-init/await-prompt1.js
  19:1  error  Expected indentation of 8 spaces but found 10  indent
  20:1  error  Expected indentation of 8 spaces but found 10  indent
  21:1  error  Expected indentation of 8 spaces but found 10  indent
  22:1  error  Expected indentation of 8 spaces but found 10  indent
  23:1  error  Expected indentation of 8 spaces but found 10  indent
  24:1  error  Expected indentation of 6 spaces but found 8   indent

✖ 6 problems (6 errors, 0 warnings)
  6 errors and 0 warnings potentially fixable with the `--fix` option.

When I try npx eslint await-prompt1.js --fix, it changes the code like this:

--- await-prompt1.js.orig	2021-01-31 19:21:20.000000000 -0500
+++ await-prompt1.js	2021-01-31 19:32:02.000000000 -0500
@@ -16,12 +16,12 @@
   const { androidPackageId } =
     platforms.indexOf('android') !== -1
       ? await prompt({
-          type: 'text',
-          name: 'androidPackageId',
-          message: 'What is the desired Android package id?',
-          initial: 'com.demo',
-          validate: androidPackageId => androidPackageId.length > 0
-        })
+        type: 'text',
+        name: 'androidPackageId',
+        message: 'What is the desired Android package id?',
+        initial: 'com.demo',
+        validate: androidPackageId => androidPackageId.length > 0
+      })
       : { androidPackageId: null }
 
   console.log({ androidPackageId })

Are you willing to submit a pull request to fix this bug?

Yes (best effort)

This is regression is blocking standard/standard#1624, which could have otherwise been resolved by the recent eslint 7.19.0 update.

I think this regression is caused by my contribution in #13972, which I should have tested on https://github.com/brodybits/react-native-module-init before submitting it for review.

Part 1b: General case - object reducer function

I have reproduced a more general manifestation of the issue with an object reducer function in tests/lib/rules/indent.js:

        {
            code: unIndent`
              condition1
                ? reduce({
                  first,
                  second,
                })
                : reduce({
                  third,
                  fourth,
                })
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },

If I would revert the lib update in e1da90f (PR #13972), then this would be the expected indentation now unbalanced:

        {
            code: unIndent`
              condition1
                ? reduce({
                    first,
                    second,
                  })
                : reduce({
                  third,
                  fourth,
                })
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },

Part 2: General case - pipe function

I have reproduced another more general manifestation with a pipe function in tests/lib/rules/indent.js:

        {
            code: unIndent`
              condition1
                ? pipe(
                  first,
                  second,
                )
                : pipe(
                  third,
                  fourth,
                )
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },

This seems to be inconsistent with the expected formatting documented in: https://eslint.org/docs/rules/indent#offsetternaryexpressions

If I would revert the change to lib in e1da90f (PR #13972), then the expected indenting is shown as follows now unbalanced:

        {
            code: unIndent`
              condition1
                ? pipe(
                    first,
                    second,
                  )
                : pipe(
                  third,
                  fourth,
                )
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },

which looks similar to closed issue #13425.

[brodybits]

/cc @sheerun - probably something very simple to fix :)

[btmills]

Agreed this looks like a regression. I still have the release issue open, so while this isn’t a show-stopper that absolutely requires a patch release, if there’s a fix available tomorrow, we could release it as 7.19.1. The other option is reverting the PR that caused it and re-attempting the fix in the release two weeks from now.

[brodybits]

I would say that this issue is more than a regression.

I have now added multiple general cases to the description. I would say that reverting e1da90f (PR #13972) would lead to multiple cases of unbalanced ternary formatting:

        // issue #13971:
        {
            code: unIndent`
              condition1
                ? condition2
                    ? Promise.resolve(1)
                    : Promise.resolve(2)
                : condition2
                  ? Promise.resolve(2)
                  : Promise.resolve(3)
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },
        // from Part 1a in the description:
        {
            code: unIndent`
              condition1
                ? reduce({
                    first,
                    second,
                  })
                : reduce({
                  third,
                  fourth,
                })
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },
        // from Part 2 in the description:
        {
            code: unIndent`
              condition1
                ? pipe(
                    first,
                    second,
                  )
                : pipe(
                  third,
                  fourth,
                )
            `,
            options: [2, { offsetTernaryExpressions: true }]
        },

I would personally favor taking some more time to understand what is going on and how we can resolve the root causes before simply reverting e1da90f (PR #13972).

[Tol1]

So... No updates? After upgrading I got 300+ errors about indent, so I downgraded after saw this issue. And seems like I still cannot upgrade if I don't want to run fixes back and forth

[github-actions]

Oops! It looks like we lost track of this issue. @eslint/eslint-tsc what do we want to do here? This issue will auto-close in 7 days without an update.

[voxpelli]

This change is causing us some few failures across three external projects we test on, which we now will have to create PR:s for before rolling out the latest ESLint in StandardJS, see: standard/standard#1702

[pdeveltere]

This is also an issue for us. offsetTernaryExpressions is not working as expected!

[Tol1]

Woo, this issue has birthday soon. With major version release inbetween, still nothing?

[liuxy0551]

This problem still exists in v8.21.0.

[emurano]

Still exists in v8.28.0

[jsonMartin]

This still is not working for me either.

[sam3k]

Quick summary update on this issue:

• A revert was never made to undo the changes
• The team is awaiting for a full fix to be submitted

Anyone wants to submit a PR to fix this?

[matt-jolt]

Still no updates on this? Over 2 years old with solid test cases and a major version release in the meantime?

I think the priority and impact are lower than they should be, this issue prevents eslint from behaving how it should according to the docs. Its broken.

[nzakas]

@matt-jolt if you'd like to submit a PR, please do. The team has a lot of higher priorities to deal with, and as such, issues like this tend to sit for a while. There's just not enough time to tackle every issue.

[nzakas]

Hi folks, I just wanted to let everyone know that the ESLint team has decided to deprecate all formatting rules, including indent, and recommend people use source code formatters like Prettier or dprint instead. As a result, we are closing all outstanding issues related to these rules. For more information, please see #17522. Thanks.