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

Binary operators in !(...) not checked for missing horizontal whitespace #2652

Closed
cflee opened this issue May 9, 2024 · 0 comments · Fixed by #2653
Closed

Binary operators in !(...) not checked for missing horizontal whitespace #2652

cflee opened this issue May 9, 2024 · 0 comments · Fixed by #2653
Milestone

Comments

@cflee
Copy link
Contributor

cflee commented May 9, 2024

Expected Behavior

Given this code:

val foo1 = !(null?:true)
val foo2 = !(7==7)

It should be a lint violation, and formatted to the following to match what IntelliJ does:

val foo1 = !(null ?: true)
val foo2 = !(7 == 7)

Kotlin Coding Conventions: Horizontal whitespace

Put spaces around binary operators (a + b). Exception: don't put spaces around the "range to" operator (0..i).

This rule currently seems to be implemented in SpacingAroundOperatorsRule.

Observed Behavior

No error is thrown during check and it is not formatted.

However, it is working correctly for these cases:

val foo1 = (null?:true)
val foo1 = null?:true

So I am only observing it for the !( ... ) cases.

Steps to Reproduce

Adding this test to SpacingAroundOperatorsRuleTest:

@Test
fun `Given operator inside !() missing spaces around the operator`() {
    val code =
        """
    val foo1 = !(null?:true)
    val foo2 = !(7==7)
    val foo3 = !(null?: true)
    val foo4 = (null?:true)
    val foo5 = null?:true
    """.trimIndent()
    val formattedCode =
        """
    val foo1 = !(null ?: true)
    val foo2 = !(7 == 7)
    val foo3 = !(null ?: true)
    val foo4 = (null ?: true)
    val foo5 = null ?: true
    """.trimIndent()
    spacingAroundOperatorsRuleAssertThat(code)
        .hasLintViolations(
            LintViolation(1, 18, "Missing spacing around \"?:\""),
            LintViolation(2, 15, "Missing spacing around \"==\""),
            LintViolation(3, 18, "Missing spacing before \"?:\""),
            LintViolation(4, 17, "Missing spacing around \"?:\""),
            LintViolation(5, 16, "Missing spacing around \"?:\""),
        ).isFormattedAs(formattedCode)
}

Results in this test failure:

[Lint errors which can be automatically corrected] 
Expecting actual:
  [LintViolationFields(line=4, col=17, detail=Missing spacing around "?:", canBeAutoCorrected=true),
    LintViolationFields(line=5, col=16, detail=Missing spacing around "?:", canBeAutoCorrected=true)]
to contain exactly in any order:
  [LintViolationFields(line=1, col=18, detail=Missing spacing around "?:", canBeAutoCorrected=true),
    LintViolationFields(line=2, col=15, detail=Missing spacing around "==", canBeAutoCorrected=true),
    LintViolationFields(line=3, col=18, detail=Missing spacing before "?:", canBeAutoCorrected=true),
    LintViolationFields(line=4, col=17, detail=Missing spacing around "?:", canBeAutoCorrected=true),
    LintViolationFields(line=5, col=16, detail=Missing spacing around "?:", canBeAutoCorrected=true)]
but could not find the following elements:
  [LintViolationFields(line=1, col=18, detail=Missing spacing around "?:", canBeAutoCorrected=true),
    LintViolationFields(line=2, col=15, detail=Missing spacing around "==", canBeAutoCorrected=true),
    LintViolationFields(line=3, col=18, detail=Missing spacing before "?:", canBeAutoCorrected=true)]
...

Your Environment

  • Version of ktlint used: current master branch faf8f66
cflee added a commit to cflee/ktlint that referenced this issue May 9, 2024
…ry expression

Instead of checking for whether any parent element is a KtPrefixExpression, check whether the closest ancestor that is a KtOperationExpression is a KtPrefixExpression.

Fixes pinterest#2652
paul-dingemans pushed a commit that referenced this issue May 12, 2024
…ry expression (#2653)

For example an expression wrapped in `!(<some-binary-expression>)` should check/fix spacing around the operator in the inner binary expression.

Fixes #2652
@paul-dingemans paul-dingemans added this to the 1.3.0 milestone May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants