Skip to content

Commit

Permalink
Fix IndexOutOfBoundsException when AST before an enumeration class is…
Browse files Browse the repository at this point in the history
… changed by some rule (#1586)
  • Loading branch information
paul-dingemans committed Aug 17, 2022
1 parent 4815a12 commit b74ac02
Show file tree
Hide file tree
Showing 4 changed files with 377 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.psi.psiUtil.leaves
import org.jetbrains.kotlin.psi.psiUtil.siblings

public fun ASTNode.nextLeaf(includeEmpty: Boolean = false, skipSubtree: Boolean = false): ASTNode? {
Expand Down Expand Up @@ -246,6 +247,16 @@ public fun ASTNode.visit(enter: (node: ASTNode) -> Unit, exit: (node: ASTNode) -
exit(this)
}

/**
* Get the line number of the node in the original code sample. If the AST, prior to the current node, is changed by
* adding or removing a node containing a newline, the lineNumber will not be calculated correctly. Either it results in
* an incorrect lineNumber or an IndexOutOfBoundsException (Wrong offset). is thrown. The rule in which the problem
* manifest does not need to be the same rule which has changed the prior AST. Debugging such problems can be very hard.
*/
@Deprecated(
"Marked for removal in KtLint 0.48. The lineNumber is a calculated field. This calculation is not always " +
"reliable when formatting code.See KDOC for more information.",
)
public fun ASTNode.lineNumber(): Int? =
this.psi.containingFile?.viewProvider?.document?.getLineNumber(this.startOffset)?.let { it + 1 }

Expand Down Expand Up @@ -293,7 +304,40 @@ public fun ASTNode.logStructure(): ASTNode =
private fun String.replaceTabAndNewline(): String =
replace("\t", "\\t").replace("\n", "\\n")

@Deprecated(
message = "This method is marked for removal in KtLint 0.48.0 as it is not reliable.",
replaceWith = ReplaceWith("hasWhiteSpaceWithNewLineInClosedRange(from, to)"),
)
public fun containsLineBreakInRange(from: PsiElement, to: PsiElement): Boolean =
from.siblings(forward = true, withItself = true)
.takeWhile { !it.isEquivalentTo(to) }
.any { it.textContains('\n') }

/**
* Verifies that a whitespace leaf containing a newline exist in the closed range [from] - [to]. Also, returns true in
* case any of the boundary nodes [from] or [to] is a whitespace leaf containing a newline.
*/
public fun hasWhiteSpaceWithNewLineInClosedRange(from: ASTNode, to: ASTNode): Boolean =
from.isWhiteSpaceWithNewline() ||
leavesInOpenRange(from, to).any { it.isWhiteSpaceWithNewline() } ||
to.isWhiteSpaceWithNewline()

/**
* Verifies that no whitespace leaf contains a newline in the closed range [from] - [to]. Also, the boundary nodes
* [from] and [to] should not be a whitespace leaf containing a newline.
*/
public fun noWhiteSpaceWithNewLineInClosedRange(from: ASTNode, to: ASTNode): Boolean =
!from.isWhiteSpaceWithNewline() &&
leavesInOpenRange(from, to).none { it.isWhiteSpaceWithNewline() } &&
!to.isWhiteSpaceWithNewline()

/**
* Creates a sequence of leaf nodes in the open range [from] - [to]. This means that the boundary nodes are excluded
* from the range in case they would happen to be a leaf node. In case [from] is a [CompositeElement] than the first
* leaf node in the sequence is the first leaf node in this [CompositeElement]. In case [to] is a [CompositeElement]
* than the last node in the sequence is the last leaf node prior to this [CompositeElement].
*/
public fun leavesInOpenRange(from: ASTNode, to: ASTNode): Sequence<ASTNode> =
from
.leaves()
.takeWhile { it != to && it != to.firstChildNode }
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
package com.pinterest.ktlint.core.ast

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
import com.pinterest.ktlint.core.ast.ElementType.ENUM_ENTRY
import com.pinterest.ktlint.core.internal.prepareCodeForLinting
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.lang.FileASTNode
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

class PackageKtTest {
@Test
fun `Given an enum class body then get all leaves in the open range of the class body`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape {
FOO, FOOBAR, BAR
}
""".trimIndent(),
).toEnumClassBodySequence()

val actual =
leavesInOpenRange(enumClassBody.first(), enumClassBody.last())
.map { it.text }
.toList()

assertThat(actual).containsExactly(
// LBRACE is omitted from class body as it is an open range
"\n ",
"FOO",
",",
" ",
"FOOBAR",
",",
" ",
"BAR",
"\n",
// RBRACE is omitted from class body as it is an open range
)
}

@Nested
inner class NoWhiteSpaceWithNewLineInClosedRange {
@Test
fun `Given an enum class with no whitespace leaf containing a newline between the first and last enum entry`() {
val enumEntries =
transformCodeToAST(
"""
enum class Shape {
FOO, FOOBAR, BAR
}
""".trimIndent(),
).toEnumClassBodySequence()
.filter { it.elementType == ENUM_ENTRY }

val actual = noWhiteSpaceWithNewLineInClosedRange(enumEntries.first(), enumEntries.last())

assertThat(actual).isTrue
}

@Test
fun `Given a range of nodes starting with a whitespace leaf containing a newline but other whitespace leaves not containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape {
FOO, FOOBAR, BAR } // Malformed on purpose for test
""".trimIndent(),
).toEnumClassBodySequence()

val actual = noWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isFalse
}

@Test
fun `Given a range of nodes not starting or ending with a whitespace leaf containing a newline but containing another whitespace leaf containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape {
FOO,
FOOBAR,
BAR
}
""".trimIndent(),
).toEnumClassBodySequence()

val actual = noWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isFalse
}

@Test
fun `Given a range of nodes ending with a whitespace leaf containing a newline but other whitespace leaves not containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape { FOO, FOOBAR, BAR
} // Malformed on purpose for test
""".trimIndent(),
).toEnumClassBodySequence()

val actual = noWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isFalse
}

@Test
fun `Given a range of nodes not containing a whitespace with a newline but having a block comment in between which does contain a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape { FOO, /*
newline in a block comment is ignored as it is not part of a whitespace leaf
*/ FOOBAR, BAR }
""".trimIndent(),
).toEnumClassBodySequence()

val actual = noWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isTrue
}
}

@Nested
inner class HasWhiteSpaceWithNewLineInClosedRange {
@Test
fun `Given an enum class with no whitespace leaf containing a newline between the first and last enum entry`() {
val enumEntries =
transformCodeToAST(
"""
enum class Shape {
FOO, FOOBAR, BAR
}
""".trimIndent(),
).toEnumClassBodySequence()
.filter { it.elementType == ENUM_ENTRY }

val actual = hasWhiteSpaceWithNewLineInClosedRange(enumEntries.first(), enumEntries.last())

assertThat(actual).isFalse
}

@Test
fun `Given a range of nodes starting with a whitespace leaf containing a newline but other whitespace leaves not containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape {
FOO, FOOBAR, BAR } // Malformed on purpose for test
""".trimIndent(),
).toEnumClassBodySequence()

val actual = hasWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isTrue
}

@Test
fun `Given a range of nodes not starting or ending with a whitespace leaf containing a newline but containing another whitespace leaf containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape {
FOO,
FOOBAR,
BAR
}
""".trimIndent(),
).toEnumClassBodySequence()

val actual = hasWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isTrue
}

@Test
fun `Given a range of nodes ending with a whitespace leaf containing a newline but other whitespace leaves not containing a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape { FOO, FOOBAR, BAR
} // Malformed on purpose for test
""".trimIndent(),
).toEnumClassBodySequence()

val actual = hasWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isTrue
}

@Test
fun `Given a range of nodes not containing a whitespace with a newline but having a block comment in between which does contain a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape { FOO, /*
newline in a block comment is ignored as it is not part of a whitespace leaf
*/ FOOBAR, BAR }
""".trimIndent(),
).toEnumClassBodySequence()

val actual = noWhiteSpaceWithNewLineInClosedRange(enumClassBody.first(), enumClassBody.last())

assertThat(actual).isTrue
}
}

@Test
fun `Given a range of nodes not containing a whitespace with a newline but having a block comment in between which does contain a newline`() {
val enumClassBody =
transformCodeToAST(
"""
enum class Shape { FOO, /*
newline in a block comment should be ignored as it is not part of a whitespace leaf
*/ FOOBAR, BAR }
""".trimIndent(),
).toEnumClassBodySequence()

val actual = containsLineBreakInRange(enumClassBody.first().psi, enumClassBody.last().psi)

// This method should have returned false instead of true. As of that the method is deprecated.
assertThat(actual).isTrue
}

private fun transformCodeToAST(code: String) =
prepareCodeForLinting(
KtLint.ExperimentalParams(
text =
code,
ruleProviders = setOf(
RuleProvider { DummyRule() },
),
cb = { _, _ -> },
),
).rootNode

private fun FileASTNode.toEnumClassBodySequence() =
findChildByType(CLASS)
?.findChildByType(CLASS_BODY)
?.children()
.orEmpty()
}

/**
* A dummy rule for testing. Optionally the rule can be created with a lambda to be executed for each node visited.
*/
private open class DummyRule(
val block: (node: ASTNode) -> Unit = {},
) : Rule(DUMMY_RULE_ID) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
block(node)
}

companion object {
const val DUMMY_RULE_ID = "dummy-rule"
}
}

0 comments on commit b74ac02

Please sign in to comment.