From a7614680bcc2a4ef7c03c67d36e8f5041b65baf0 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 1 Jul 2021 18:19:20 +0300 Subject: [PATCH 1/2] Don't use PSI for line number calculation ### What's done: * Changed logic * Added test --- .../chapter3/identifiers/LocalVariablesRule.kt | 3 +++ .../org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 14 +++----------- .../cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt | 9 +++++++++ .../main/kotlin/LocalVariableWithOffsetExpected.kt | 9 +++++++++ .../src/main/kotlin/LocalVariableWithOffsetTest.kt | 8 ++++++++ 5 files changed, 32 insertions(+), 11 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetExpected.kt create mode 100644 diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt index 8170e552c3..26c72ffe79 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt @@ -96,6 +96,9 @@ class LocalVariablesRule(configRules: List) : DiktatRule( checkLineNumbers(property, firstUsageStatementLine, firstUsageLine = firstUsage.node.getLineNumber(), offset = offset) } + /** + * Check declarations of properties, that are used on the same line + */ @Suppress("TOO_LONG_FUNCTION") private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List) { val numLinesAfterLastProp = diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index c1674e948d..ec4cf31b97 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -40,7 +40,6 @@ import com.pinterest.ktlint.core.ast.isLeaf import com.pinterest.ktlint.core.ast.isPartOfComment import com.pinterest.ktlint.core.ast.isRoot import com.pinterest.ktlint.core.ast.isWhiteSpace -import com.pinterest.ktlint.core.ast.lineNumber import com.pinterest.ktlint.core.ast.parent import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.TokenType @@ -52,7 +51,6 @@ 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 @@ -762,18 +760,12 @@ fun ASTNode.hasEqBinaryExpression(): Boolean = ?: false /** - * 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. + * Get line number, where this node's content starts. * - * @return line number or null if it cannot be calculated + * @return line number */ fun ASTNode.getLineNumber(): Int = - psi.containingFile - .viewProvider - .document - ?.takeIf { it.getLineEndOffset(it.lineCount - 1) >= psi.endOffset } - ?.let { lineNumber() } - ?: calculateLineNumber() + calculateLineNumber() /** * This function calculates line number instead of using cached values. diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt index 90cbdd62b3..fafb72d0b9 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt @@ -265,6 +265,15 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin", fixAndCompare("KdocFormattingMultilineTagsExpected.kt", "KdocFormattingMultilineTagsTest.kt") } + @Test + @Tag("DiktatRuleSetProvider") + fun `regression - FP of local variables rule`() { + fixAndCompare("LocalVariableWithOffsetExpected.kt", "LocalVariableWithOffsetTest.kt") + org.assertj.core.api.Assertions.assertThat(unfixedLintErrors).noneMatch { + it.ruleId == "diktat-ruleset:local-variables" + } + } + companion object { private const val DEFAULT_CONFIG_PATH = "../diktat-analysis.yml" private val unfixedLintErrors: MutableList = mutableListOf() diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetExpected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetExpected.kt new file mode 100644 index 0000000000..4bf0abc083 --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetExpected.kt @@ -0,0 +1,9 @@ +package org.cqfn.diktat + +override fun boo() { + val listTestResult: MutableList = mutableListOf() + files.chunked(warnPluginConfig.batchSize ?: 1).map { chunk -> + handleTestFile(chunk.map { it.single() }, warnPluginConfig, generalConfig) + }.forEach { listTestResult.addAll(it) } +} + diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetTest.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetTest.kt new file mode 100644 index 0000000000..10fa014e9a --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/LocalVariableWithOffsetTest.kt @@ -0,0 +1,8 @@ +package org.cqfn.diktat + + override fun boo() { + val listTestResult: MutableList = mutableListOf() + files.chunked(warnPluginConfig.batchSize ?: 1).map { chunk -> + handleTestFile(chunk.map { it.single() }, warnPluginConfig, generalConfig) + }.forEach { listTestResult.addAll(it) } +} \ No newline at end of file From 2f574b331cb8d77a876a268d7a9ea524c179ee4e Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 1 Jul 2021 18:30:07 +0300 Subject: [PATCH 2/2] Don't use PSI for line number calculation ### What's done: * Changed logic * Added test --- .../rules/chapter3/identifiers/LocalVariablesRule.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt index 26c72ffe79..a174c8f711 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt @@ -97,7 +97,9 @@ class LocalVariablesRule(configRules: List) : DiktatRule( } /** - * Check declarations of properties, that are used on the same line + * Check declarations, for which the properties are used on the same line. + * If properties are used for the first time in the same statement, then they can be declared on consecutive lines + * with maybe empty lines in between. */ @Suppress("TOO_LONG_FUNCTION") private fun handleConsecutiveDeclarations(statement: PsiElement, properties: List) { @@ -176,6 +178,8 @@ class LocalVariablesRule(configRules: List) : DiktatRule( /** * Returns the [KtBlockExpression] with which a property should be compared. + * If the usage is in nested block, compared to declaration, then statement from declaration scope, which contains block + * with usage, is returned. * * @return either the line on which the property is used if it is first used in the same scope, or the block in the same scope as declaration */