Skip to content

Commit

Permalink
Improve insert of suppression (pinterest#2546)
Browse files Browse the repository at this point in the history
* Prevent KtlintSuppressionOutOfBoundsException Suppression if offset equals the end the line.
* Insert suppression at parent in case the target location of the suppression is a whitespace.

Closes pinterest#2514
  • Loading branch information
paul-dingemans authored Feb 11, 2024
1 parent 7501f72 commit 1b7c449
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.pinterest.ktlint.rule.engine.api

import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
import com.pinterest.ktlint.rule.engine.internal.RuleExecutionContext
import com.pinterest.ktlint.rule.engine.internal.insertKtlintRuleSuppression
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
Expand Down Expand Up @@ -38,6 +39,14 @@ private fun ASTNode.findLeafElementAt(suppression: KtlintSuppression): ASTNode =

is KtlintSuppressionAtOffset ->
findLeafElementAt(suppression.offsetFromStartOf(text))
?.let {
if (it.isWhiteSpace()) {
// A suppression can not be added at a whitespace element. Insert it at the parent instead
it.treeParent
} else {
it
}
}
?: throw KtlintSuppressionNoElementFoundException(suppression)
}

Expand All @@ -60,11 +69,22 @@ private fun KtlintSuppressionAtOffset.offsetFromStartOf(code: String): Int {
}

val codeLine = lines[line - 1]
if (col > codeLine.length) {
throw KtlintSuppressionOutOfBoundsException(this)
return when {
col == 1 && codeLine.isEmpty() -> {
startOffsetOfLineContainingLintError
}
col <= codeLine.length -> {
startOffsetOfLineContainingLintError + (col - 1)
}
col == codeLine.length + 1 -> {
// Offset of suppression is set at EOL of the line. This is visually correct for the reader. But the newline character was stripped
// from the line because the lines were split using that character.
startOffsetOfLineContainingLintError + col
}
else -> {
throw KtlintSuppressionOutOfBoundsException(this)
}
}

return startOffsetOfLineContainingLintError + (col - 1)
}

public sealed class KtlintSuppressionException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
import com.pinterest.ktlint.ruleset.standard.rules.CONDITION_WRAPPING_RULE_ID
import com.pinterest.ktlint.ruleset.standard.rules.NO_CONSECUTIVE_BLANK_LINES_RULE_ID
import com.pinterest.ktlint.ruleset.standard.rules.NO_LINE_BREAK_BEFORE_ASSIGNMENT_RULE_ID
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -275,6 +277,91 @@ class KtlintRuleEngineSuppressionKtTest {
}
}

@Test
fun `Given a violation with offset exactly at the EOL of a line`() {
val code =
"""
fun foo(): String // Some comment
= "some-result"
""".trimIndent()
val formattedCode =
"""
@Suppress("ktlint:standard:no-line-break-before-assignment")
fun foo(): String // Some comment
= "some-result"
""".trimIndent()
val actual =
ktLintRuleEngine
.insertSuppression(
Code.fromSnippet(code, false),
KtlintSuppressionAtOffset(1, 34, NO_LINE_BREAK_BEFORE_ASSIGNMENT_RULE_ID),
)

assertThat(actual).isEqualTo(formattedCode)
}

@Nested
inner class `Consecutive line suppression can not be placed at line` {
@Test
fun `Given some consecutive blank lines at top level`() {
val code =
"""
val foo = "foo"
val bar = "bar"
""".trimIndent()
val formattedCode =
"""
@file:Suppress("ktlint:standard:no-consecutive-blank-lines")
val foo = "foo"
val bar = "bar"
""".trimIndent()
val actual =
ktLintRuleEngine
.insertSuppression(
Code.fromSnippet(code, false),
KtlintSuppressionAtOffset(3, 1, NO_CONSECUTIVE_BLANK_LINES_RULE_ID),
)

assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Given some consecutive blank lines inside a function`() {
val code =
"""
fun foobar() {
val foo = "foo"
val bar = "bar"
}
""".trimIndent()
val formattedCode =
"""
@Suppress("ktlint:standard:no-consecutive-blank-lines")
fun foobar() {
val foo = "foo"
val bar = "bar"
}
""".trimIndent()
val actual =
ktLintRuleEngine
.insertSuppression(
Code.fromSnippet(code, false),
KtlintSuppressionAtOffset(3, 1, NO_CONSECUTIVE_BLANK_LINES_RULE_ID),
)

assertThat(actual).isEqualTo(formattedCode)
}
}

private companion object {
val SOME_RULE_ID = RuleId("standard:some-rule-id")
}
Expand Down

0 comments on commit 1b7c449

Please sign in to comment.