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

Fix for ArrayIndexOutOfBoundsException in #371 #612

Merged
merged 8 commits into from
Dec 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion diktat-rules/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@
</goals>
<configuration>
<sourceDirs>
<source>src/test/kotlin</source>
<source>${project.basedir}/src/main/kotlin</source>
<source>${project.basedir}/src/test/kotlin</source>
</sourceDirs>
</configuration>
</execution>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,18 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
private fun handleLocalProperty(property: KtProperty, usages: List<KtNameReferenceExpression>) {
val declarationScope = property.getDeclarationScope()

val firstUsageStatementLine = getFirstUsageStatementOrBlock(usages, declarationScope).node.getLineNumber(isFixMode)!!
val firstUsage = usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
checkLineNumbers(property, firstUsageStatementLine, firstUsageLine = firstUsage.node.getLineNumber(isFixMode)!!)
val firstUsageStatementLine = getFirstUsageStatementOrBlock(usages, declarationScope).node.getLineNumber()
val firstUsage = usages.minBy { it.node.getLineNumber() }!!
checkLineNumbers(property, firstUsageStatementLine, firstUsageLine = firstUsage.node.getLineNumber())
}

@Suppress("UnsafeCallOnNullableType")
private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List<KtProperty>) {
// need to check that properties are declared consecutively with only maybe empty lines
properties
.sortedBy { it.node.getLineNumber(isFixMode)!! }
.sortedBy { it.node.getLineNumber() }
.zip(properties.size - 1 downTo 0)
.forEach { (property, offset) ->
checkLineNumbers(property, statement.node.getLineNumber(isFixMode)!!, offset)
checkLineNumbers(property, statement.node.getLineNumber(), offset)
}
}

Expand All @@ -117,15 +116,15 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
siblings
.last()
.node
.lastLineNumber(isFixMode)!! - siblings
.lastLineNumber() - siblings
.first()
.node
.getLineNumber(isFixMode)!! - 1
.getLineNumber() - 1
}

if (firstUsageStatementLine - numLinesToSkip != property.node.lastLineNumber(isFixMode)!! + 1 + offset) {
if (firstUsageStatementLine - numLinesToSkip != property.node.lastLineNumber() + 1 + offset) {
LOCAL_VARIABLE_EARLY_DECLARATION.warn(configRules, emitWarn, isFixMode,
warnMessage(property.name!!, property.node.getLineNumber(isFixMode)!!, firstUsageLine
warnMessage(property.name!!, property.node.getLineNumber(), firstUsageLine
?: firstUsageStatementLine), property.startOffset, property.node)
}
}
Expand All @@ -137,7 +136,7 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
*/
@Suppress("UnsafeCallOnNullableType", "GENERIC_VARIABLE_WRONG_DECLARATION")
private fun getFirstUsageStatementOrBlock(usages: List<KtNameReferenceExpression>, declarationScope: KtBlockExpression?): PsiElement {
val firstUsage = usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
val firstUsage = usages.minBy { it.node.getLineNumber() }!!
val firstUsageScope = firstUsage.getParentOfType<KtBlockExpression>(true)

return if (firstUsageScope == declarationScope) {
Expand All @@ -147,7 +146,7 @@ class LocalVariablesRule(private val configRules: List<RulesConfig>) : Rule("loc
.find { it.parent == declarationScope }!!
} else {
// first usage is in deeper block compared to declaration, need to check how close is declaration to the first line of the block
usages.minBy { it.node.getLineNumber(isFixMode)!! }!!
usages.minBy { it.node.getLineNumber() }!!
.parentsWithSelf
.find { it.parent == declarationScope }!!
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import org.jetbrains.kotlin.psi.KtAnnotationEntry
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtIfExpression
import org.jetbrains.kotlin.psi.psiUtil.children
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.parents
import org.jetbrains.kotlin.psi.psiUtil.siblings
import org.slf4j.Logger
Expand Down Expand Up @@ -656,7 +657,7 @@ fun ASTNode.extractLineOfText(): String {
* Checks node has `@Test` annotation
*/
fun ASTNode.hasTestAnnotation() = findChildByType(MODIFIER_LIST)
?.getAllChildrenWithType(ElementType.ANNOTATION_ENTRY)
?.getAllChildrenWithType(ANNOTATION_ENTRY)
?.flatMap { it.findAllNodesWithSpecificType(ElementType.CONSTRUCTOR_CALLEE) }
?.any { it.findLeafWithSpecificType(ElementType.IDENTIFIER)?.text == "Test" }
?: false
Expand All @@ -683,11 +684,9 @@ fun isLocatedInTest(filePathParts: List<String>, testAnchors: List<String>) = fi
fun ASTNode.firstLineOfText(suffix: String = "") = text.lines().run { singleOrNull() ?: (first() + suffix) }

/**
* Return the number of the last line in the file
*
* @param isFixMode whether autofix mode is on
* Return the number in the file of the last line of this node's text
*/
fun ASTNode.lastLineNumber(isFixMode: Boolean) = getLineNumber(isFixMode)?.plus(text.count { it == '\n' })
fun ASTNode.lastLineNumber() = getLineNumber() + text.count { it == '\n' }

/**
* copy-pasted method from ktlint to determine line and column number by offset
Expand Down Expand Up @@ -720,44 +719,44 @@ data class ReplacementResult(val oldNodes: List<ASTNode>, val newNodes: List<AST
* checks that this one node is placed after the other node in code (by comparing lines of code where nodes start)
*/
fun ASTNode.isGoingAfter(otherNode: ASTNode): Boolean {
val thisLineNumber = this.calculateLineNumber()
val otherLineNumber = otherNode.calculateLineNumber()

requireNotNull(thisLineNumber) { "Node ${this.text} should have a line number" }
requireNotNull(otherLineNumber) { "Node ${otherNode.text} should have a line number" }
val thisLineNumber = this.getLineNumber()
val otherLineNumber = otherNode.getLineNumber()

return (thisLineNumber > otherLineNumber)
}

/**
* Get line number, where this node's content starts
* Get line number, where this node's content starts. To avoid `ArrayIndexOutOfBoundsException`s we check whether node's maximum offset is less than
* Document's maximum offset, and calculate line number manually if needed.
*
* @param isFixMode if fix mode is off, then the text can't be altered and we can use cached values for line numbers
* @return line number or null if it cannot be calculated
*/
fun ASTNode.getLineNumber(isFixMode: Boolean): Int? =
if (!isFixMode) {
lineNumber()
} else {
calculateLineNumber()
}
fun ASTNode.getLineNumber(): Int =
psi.containingFile
.viewProvider
.document
?.takeIf { it.getLineEndOffset(it.lineCount - 1) >= psi.endOffset }
?.let { lineNumber() }
?: calculateLineNumber()

/**
* This function calculates line number instead of using cached values.
* It should be used when AST could be previously mutated by auto fixers.
*/
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION")
private fun ASTNode.calculateLineNumber(): Int? {
private fun ASTNode.calculateLineNumber(): Int {
var count = 0
// todo use runningFold or something similar when we migrate to apiVersion 1.4
return parents()
.last()
return getRootNode()
.text
.lineSequence()
// calculate offset for every line end, `+1` for `\n` which is trimmed in `lineSequence`
.indexOfFirst {
count += it.length + 1
count > startOffset
}
.let { if (it == -1) null else it + 1 }
.let {
require(it >= 0) { "Cannot calculate line number correctly, node's offset $startOffset is greater than file length ${getRootNode().textLength}" }
it + 1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,13 @@ fun readFile(foo: Foo) {

class D {
val x = 0

/**
* @return
*/
fun bar(): Bar {
val qux = 42
return Bar(qux)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ fun readFile(foo: Foo) {
var bar: Bar
}

class D {val x = 0}
class D {val x = 0
fun bar(): Bar {val qux = 42; return Bar(qux)}
}