Fix: set indent level to return/throw argument (fixes #8710)

[mysticatea]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix (template)

Fixes #8710 .

What changes did you make? (Give an overview)

This PR make that indent rule set indent levels of node.argument property of ReturnStatement/ThrowStatement.

Is there anything you'd like reviewers to focus on?

Please check this direction is correct.

[eslintbot]

LGTM

[not-an-aardvark]

Hmm, it seems like this shouldn't be necessary -- I would have expected the argument to be indented anyway because it's parenthesized. I'll take a look at it later today.

[not-an-aardvark]

The issue isn't with return or throw expressions in particular -- it applies to all parenthesized ternary expressions. For example, the rule currently expects this indentation, which is incorrect:

(
    foo ? bar :
baz
)

This is happening because addParensIndent only indents the first token after the left paren, because it assumes all of the other parenthesized tokens will depend on the first token.

There are a few ways to fix this:

1. Update the ConditionalExpression listener to always set the offset of node.alternate, but set it to 0 if it should not be indented

   Diff

   diff --git a/lib/rules/indent.js b/lib/rules/indent.js
   index 82268975..0da930d1 100644
   --- a/lib/rules/indent.js
   +++ b/lib/rules/indent.js
   @@ -989,50 +989,50 @@ module.exports = {
                    //     foo > 0 ? bar :
                    //     foo < 0 ? baz :
                    //     /*else*/ qiz ;
   -                if (!options.flatTernaryExpressions ||
   +                const offset = !options.flatTernaryExpressions ||
                        !astUtils.isTokenOnSameLine(node.test, node.consequent) ||
                        isFirstTokenOfStatement(firstToken, node)
   -                ) {
   -                    const questionMarkToken = sourceCode.getFirstTokenBetween(node.test, node.consequent, token => token.type === "Punctuator" && token.value === "?");
   -                    const colonToken = sourceCode.getFirstTokenBetween(node.consequent, node.alternate, token => token.type === "Punctuator" && token.value === ":");
   -
   -                    const consequentTokens = sourceCode.getTokensBetween(questionMarkToken, colonToken, { includeComments: true });
   -                    const alternateTokens = sourceCode.getTokensAfter(colonToken, token => token.range[1] <= node.range[1]);
   +                    ? 1
   +                    : 0;
   +                const questionMarkToken = sourceCode.getFirstTokenBetween(node.test, node.consequent, token => token.type === "Punctuator" && token.value === "?");
   +                const colonToken = sourceCode.getFirstTokenBetween(node.consequent, node.alternate, token => token.type === "Punctuator" && token.value === ":");
    
   -                    offsets.setDesiredOffset(questionMarkToken, firstToken, 1);
   -                    offsets.setDesiredOffset(colonToken, firstToken, 1);
   +                const consequentTokens = sourceCode.getTokensBetween(questionMarkToken, colonToken, { includeComments: true });
   +                const alternateTokens = sourceCode.getTokensAfter(colonToken, token => token.range[1] <= node.range[1]);
    
   -                    offsets.setDesiredOffset(consequentTokens[0], firstToken, 1);
   +                offsets.setDesiredOffset(questionMarkToken, firstToken, offset);
   +                offsets.setDesiredOffset(colonToken, firstToken, offset);
    
   -                    /*
   -                     * The alternate and the consequent should usually have the same indentation.
   -                     * If they share part of a line, align the alternate against the first token of the consequent.
   -                     * This allows the alternate to be indented correctly in cases like this:
   -                     * foo ? (
   -                     *   bar
   -                     * ) : ( // this '(' is aligned with the '(' above, so it's considered to be aligned with `foo`
   -                     *   baz // as a result, `baz` is offset by 1 rather than 2
   -                     * )
   -                     */
   -                    if (consequentTokens[consequentTokens.length - 1].loc.end.line === alternateTokens[0].loc.start.line) {
   -                        offsets.matchIndentOf(consequentTokens[0], alternateTokens[0]);
   -                    } else {
   +                offsets.setDesiredOffset(consequentTokens[0], firstToken, offset);
    
   -                        /**
   -                         * If the alternate and consequent do not share part of a line, offset the alternate from the first
   -                         * token of the conditional expression. For example:
   -                         * foo ? bar
   -                         *   : baz
   -                         *
   -                         * If `baz` were aligned with `bar` rather than being offset by 1 from `foo`, `baz` would end up
   -                         * having no expected indentation.
   -                         */
   -                        offsets.setDesiredOffset(alternateTokens[0], firstToken, 1);
   -                    }
   +                /*
   +                 * The alternate and the consequent should usually have the same indentation.
   +                 * If they share part of a line, align the alternate against the first token of the consequent.
   +                 * This allows the alternate to be indented correctly in cases like this:
   +                 * foo ? (
   +                 *   bar
   +                 * ) : ( // this '(' is aligned with the '(' above, so it's considered to be aligned with `foo`
   +                 *   baz // as a result, `baz` is offset by 1 rather than 2
   +                 * )
   +                 */
   +                if (consequentTokens[consequentTokens.length - 1].loc.end.line === alternateTokens[0].loc.start.line) {
   +                    offsets.matchIndentOf(consequentTokens[0], alternateTokens[0]);
   +                } else {
    
   -                    offsets.setDesiredOffsets(consequentTokens, consequentTokens[0], 0);
   -                    offsets.setDesiredOffsets(alternateTokens, alternateTokens[0], 0);
   +                    /**
   +                     * If the alternate and consequent do not share part of a line, offset the alternate from the first
   +                     * token of the conditional expression. For example:
   +                     * foo ? bar
   +                     *   : baz
   +                     *
   +                     * If `baz` were aligned with `bar` rather than being offset by 1 from `foo`, `baz` would end up
   +                     * having no expected indentation.
   +                     */
   +                    offsets.setDesiredOffset(alternateTokens[0], firstToken, offset);
                    }
   +
   +                offsets.setDesiredOffsets(consequentTokens, consequentTokens[0], 0);
   +                offsets.setDesiredOffsets(alternateTokens, alternateTokens[0], 0);
                },
    
                DoWhileStatement: node => addBlocklessNodeIndent(node.body),
   

2. Update addParensIndent to indent all tokens in the parentheses, instead of just the first token

   Diff

   diff --git a/lib/rules/indent.js b/lib/rules/indent.js
   index 82268975..50a2cfec 100644
   --- a/lib/rules/indent.js
   +++ b/lib/rules/indent.js
   @@ -882,7 +882,13 @@ module.exports = {
    
                    // We only want to handle parens around expressions, so exclude parentheses that are in function parameters and function call arguments.
                    if (!parameterParens.has(leftParen) && !parameterParens.has(rightParen)) {
   -                    offsets.setDesiredOffset(sourceCode.getTokenAfter(leftParen), leftParen, 1);
   +                    const parenthesizedTokens = new Set(sourceCode.getTokensBetween(leftParen, rightParen));
   +
   +                    parenthesizedTokens.forEach(token => {
   +                        if (!parenthesizedTokens.has(offsets.getFirstDependency(token))) {
   +                            offsets.setDesiredOffset(token, leftParen, 1);
   +                        }
   +                    });
                    }
    
                    offsets.matchIndentOf(leftParen, rightParen);
   

Option 2 seems like a better solution for preventing future bugs, although it probably has slightly worse performance.

[not-an-aardvark]

See #8711 (comment)

[not-an-aardvark]

I encountered the same issue with parentheses when fixing #8666 and #8717, so I included the fix for it in #8719. As a result, #8719 also fixes this issue.

[mysticatea]

@not-an-aardvark Thank you very much. I'm closing this PR as favor of #8719.