Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WRONG_INDENTATION: false positives for parentheses-surrounded infix expressions #1409

Closed
0x6675636b796f75676974687562 opened this issue Jun 27, 2022 · 1 comment · Fixed by #1410
Assignees
Labels
bug Something isn't working
Milestone

Comments

@0x6675636b796f75676974687562
Copy link
Member

0x6675636b796f75676974687562 commented Jun 27, 2022

The following code fragment triggers no errors, provided that expectedIndentAfterOperators is off:

val value = 1 to
    2

Now, let's surround the expression (which is an infix function call) with parentheses, preserving the indent:

val value = (1 to
    2)

DiKTat will report the following errors:

[WRONG_INDENTATION] only spaces are allowed for indentation and each indentation should equal to 4 spaces (tabs are not allowed): expected 8 but was 4
[WRONG_INDENTATION] only spaces are allowed for indentation and each indentation should equal to 4 spaces (tabs are not allowed): expected 8 but was 4

Strangely, if we move the expression onto the next line (and increment the indent), the errors immediately go away:

val value =
    (1 to
        2)

The issue is caused by the incomplete implementation of ExpressionIndentationChecker as a part of #75 (see 7d913ed).

Note

The fix for this issue will also have the following side-effect. Previously (1.2.0), DiKTat would format expressions wrapped before an infix function (see also #1340) like this:

fun f() {
    true
    || false
}

— unless those expressions are surrounded with parentheses, requiring an extra indent:

fun f() {
    (true
        || false)
}

From now on (1.2.1+), no extra indent will be required:

fun f() {
    (true
    || false)
}

— which is still incorrect, but will be fixed as a part of #1340.

Environment information

  • diktat version: 1.2.0
  • build tool (maven/gradle): Maven
  • how is diktat run (CLI, plugin, etc.): Maven plug-in
  • kotlin version: 1.7.0
  • operating system: Windows
@0x6675636b796f75676974687562 0x6675636b796f75676974687562 added the bug Something isn't working label Jun 27, 2022
@0x6675636b796f75676974687562
Copy link
Member Author

0x6675636b796f75676974687562 commented Jun 27, 2022

If we take a test for ExpressionIndentationChecker (IndentationRuleWarnTest.closing parenthesis bug()) examine the original code:

fun foo() {
    return x +
        (y +
            foo(x)
        )
}

— and reformat it:

fun foo() {
    return x + (y +
        foo(x)
    )
}

— the test will still pass. If, however, we convert the return statement to an expression body, it will immediately start to fail:

fun foo() = x + (y +
    foo(x)
)

@0x6675636b796f75676974687562 0x6675636b796f75676974687562 added this to the 1.2.1 milestone Jun 28, 2022
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 28, 2022
### What's done:

 * From now on, parentheses (`LPAR`/`RPAR`) don't increment or decrement the
   indent unless they're a part of a function declaration or a function call.
 * Parentheses which just enclose an expression don't affect the indent at all.
 * This fixes #1409.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 29, 2022
* Don't change the indent for parentheses-enclosed expressions

### What's done:

 * From now on, parentheses (`LPAR`/`RPAR`) don't increment or decrement the
   indent unless they're a part of a function declaration or a function call.
 * Parentheses which just enclose an expression don't affect the indent at all.
 * The only exclusion to the above rule is an `LPAR` immediately followed by a newline
   (in this special case the indent is still incremented).
 * This fixes #1409.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant