False positive for AnnotationRule

[atulgpt]

False positive for AnnotationRule

[paul-dingemans]

Looking at the exact offset (3:46), it is the elvis operator (or more precise the code leaf after the annotated expression) which is to be pushed to the next line. So the expected format is:

private fun getErrorMessage(value: String): String =
    @Suppress("DEPRECATION")
    customMessage.takeUnless { it.isEmpty() }
        ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value)

When analyzing with the PsiViewer plugin in IntelliJ, it is clear that the ANNOTATED_EXPRESSION is following only:

    @Suppress("DEPRECATION")
    customMessage.takeUnless { it.isEmpty() }

The reasoning that default IntelliJ formatter accepts this format is less relevant as the format provided by Ktlint is also accepted by IntelliJ formatter. If the Ktlint format would not be accepted by IntelliJ formatter then the argument would be valid.

I am not sure whether this is an unintended side effect or intended behavior but I tend to the latter.

[TWiStErRob]

Would the same be true without the annotation present? i.e. the elvis should be on a new line. What really confused me is the "new line expected" from the "annotation" rule, when the annotation looked ok.

İs there any way to report the message or location in this case? Or maybe just add this example to the docs, because we checked it and it wasn't clear from the examples what is wrong with the code.

[paul-dingemans]

I had another look and discovered a problem. This may or may not be the same problem that you encountered but the original issue does not contain enough information to check that.

In the end, it really depends on the code style what is actually happening.

When code style is not set, or when set explicitly to ktlint_official, official or intellij_idea the violation is what I reported before:
With .editorconfig:

[{*.kt,*.kts}]
ktlint_standard=enabled
ktlint_experimental=enabled

the command ktlint --relative results in

17:37:42.462 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine - Enable default patterns [**/*.kt, **/*.kts]
src/main/kotlin/Foo.kt:3:46: Expected newline (standard:annotation)

Summary error count (descending) by rule:
  standard:annotation: 1

The violation message and the offset are in sync and it should be clear how the code is to be fixed. With ktlint-0.49.1 --relative -F the code is formatted as:

private fun getErrorMessage(value: String): String =
    @Suppress("DEPRECATION")
    customMessage.takeUnless { it.isEmpty() }
        ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value)

When code style is set to android or android_studio there are more violations reported:
With .editorconfig:

[{*.kt,*.kts}]
ktlint_code_style = android
ktlint_standard=enabled
ktlint_experimental=enabled

the command ktlint --relative results in

17:41:39.417 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine - Enable default patterns [**/*.kt, **/*.kts]
x.kts:6:97: Unnecessary trailing comma before ")" (standard:trailing-comma-on-call-site)
src/main/kotlin/Foo.kt:1:53: First line of body expression fits on same line as function signature (standard:function-signature)
src/main/kotlin/Foo.kt:3:1: Exceeded max line length (100) (standard:max-line-length)
src/main/kotlin/Foo.kt:3:31: Missing newline after "{" (standard:wrapping)
src/main/kotlin/Foo.kt:3:46: Expected newline (standard:annotation)
src/main/kotlin/Foo.kt:3:64: Argument should be on a separate line (unless all arguments can fit a single line) (standard:argument-list-wrapping)
src/main/kotlin/Foo.kt:3:102: Argument should be on a separate line (unless all arguments can fit a single line) (standard:argument-list-wrapping)
src/main/kotlin/Foo.kt:3:107: Missing newline before ")" (standard:argument-list-wrapping)

Summary error count (descending) by rule:
  standard:argument-list-wrapping: 3
  standard:annotation: 1
  standard:function-signature: 1
  standard:max-line-length: 1
  standard:trailing-comma-on-call-site: 1
  standard:wrapping: 1

When running format the output changes to:

17:44:18.369 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine - Enable default patterns [**/*.kt, **/*.kts]
src/main/kotlin/Foo.kt:1:53: Expected newline before annotation (standard:annotation)

Summary error count (descending) by rule:
  standard:annotation: 1

This log message is rather confusing when you compare it with the original code sample. However, it should not be compared with the original but with the modified code to make sense.
and the file is formatted as:

private fun getErrorMessage(value: String): String = @Suppress("DEPRECATION")
customMessage.takeUnless { it.isEmpty() }
    ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value)

What happened is following:

• The android code style has a default max-line-length (100)
• The first line of the body expression has been merged with the function signature as the total length of the merged line is less than the max-line-length. But in this case the body expression should not have been merged as it conflicts with another rule. I have to check whether this can be resolved.
• Once the formatting is done, KtlintRuleEngine executes a linting step. Normally this should only reveal lint errors which can not be automatically corrected. However, in this case it reveals that autocorrect has introduced a new problem. The user can however not fix this problem by manually inserting the newline before the annotated expression.
  This problem can also be reproduced with other code styles by setting .editorconfig properties that set the max line length and/or function signature rules.

Would the same be true without the annotation present? i.e. the elvis should be on a new line

Without the annotation the elvis operator is not pushed down. If pushing down would be needed in that scenario, it should be done by another rule but no such rule is in place. I know of at least one other situation (a chained method call) in which the elvis is also pushed down.
The annotation-rule pushes the elvis down to make more clear what the scope is of the annotation.

İs there any way to report the message or location in this case?

When calling the lint or format function on the KtlintRuleEngine any LintError is reported via a callback. This LintError contains the offset (line and position) on which the error was found. Ktlint CLI uses exact this same mechanism to produce the output shown above.

Or maybe just add this example to the docs, because we checked it and it wasn't clear from the examples what is wrong with the code.

I don't think that adding this example to the docs would be helpful as the same could be said for all unit tests which are not explicitly described in the docs.

[TWiStErRob]

Nice investigation! Thanks for the replies, all makes sense. I have two small clarification to make:

This may or may not be the same problem that you encountered but the original issue does not contain enough information to check that.

The issue is from the detekt project's CI on a PR (running ktlint via detekt's wrapper), so I think what you're looking for is:

https://github.com/detekt/detekt/blob/main/.editorconfig

---

Is there any way to report the message or location in this case?

This LintError contains the offset (line and position) on which the error was found.

In the screenshot you showed there's a range of characters highlighted in PSIViewer (the ANNOTATED_EXPRESSION node), same as in GitHub which is Sarif uploaded from detekt's execution. The fact that you said "This LintError contains the offset (line and position)" made me realize that it's not ktlint highlighting that full node (which made this confusing), but detekt's wrapper.

Looking at pure ktlint output, it's actually very clear where the problem is (│):

private fun getErrorMessage(value: String): String =
        @Suppress("DEPRECATION")
        customMessage.takeUnless { it.isEmpty() }│ ?: String.format(DEFAULT_ERROR_MESSAGE_WITH_NO_REASON, value)

and "Expected newline" makes perfect sense.

But detekt re-maps this single-location finding to the node that was being visited while it was emitted
(notice how the offset (_) parameter is ignored):
https://github.com/detekt/detekt/blob/948276a8db4578b962bf24c63eee3b8548bac627/detekt-formatting/src/main/kotlin/io/gitlab/arturbosch/detekt/formatting/FormattingRule.kt#L97

---

Based on:

• "Looking at the exact offset (3:46), it is the elvis operator which is to be pushed to the next line."
• "The annotation-rule pushes the elvis down to make more clear what the scope is of the annotation."

I think this issue is resolved by explanation, not a false positive, nothing to do from our report.

Based on:

• "This LintError contains the offset (line and position)"
• "detekt re-maps this single-location finding to the node that was being visited"
• "the offset parameter is ignored"

I think this explains the confusion about the message vs. location -> detekt/detekt#6105

So you can either close this, or keep it open for your other found issue.