From 74d62a9d14f6fdd2615d06ac421b9ddd7c5e83db Mon Sep 17 00:00:00 2001 From: Daria Pleshkova <81246075+diphtongue@users.noreply.github.com> Date: Wed, 20 Dec 2023 15:47:29 +0300 Subject: [PATCH] Fixed bug in WRONG_DECLARATIONS_ORDER rule (#1881) - Fixed new line, which appeared before line with comment. - Added warn and fix tests. Closes #1865 --- .../diktat/smoke/DiktatSmokeTestBase.kt | 1 + .../chapter3/ClassLikeStructuresOrderRule.kt | 9 +++-- .../rules/chapter3/files/WhiteSpaceRule.kt | 3 +- .../ClassLikeStructuresOrderFixTest.kt | 6 ++++ .../ClassLikeStructuresOrderRuleWarnTest.kt | 19 +++++++++-- .../CompanionObjectWithCommentExpected.kt | 34 +++++++++++++++++++ .../CompanionObjectWithCommentTest.kt | 33 ++++++++++++++++++ 7 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentExpected.kt create mode 100644 diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentTest.kt diff --git a/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt b/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt index 9d38c6738c..f431b5509c 100644 --- a/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt +++ b/diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt @@ -36,6 +36,7 @@ import com.saveourtool.diktat.ruleset.utils.indentation.IndentationConfig.Compan import com.charleskorn.kaml.Yaml import com.charleskorn.kaml.YamlConfiguration +import generated.WarningNames import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Tag diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt index 8823f15481..2eea45445b 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt @@ -80,7 +80,7 @@ class ClassLikeStructuresOrderRule(configRules: List) : DiktatRule( node.checkAndReorderBlocks(blocks) } - @Suppress("UnsafeCallOnNullableType") + @Suppress("UnsafeCallOnNullableType", "CyclomaticComplexMethod") private fun checkNewLinesBeforeProperty(node: ASTNode) { // checking only top-level and class-level properties if (node.treeParent.elementType != CLASS_BODY) { @@ -88,6 +88,11 @@ class ClassLikeStructuresOrderRule(configRules: List) : DiktatRule( } val previousProperty = node.prevSibling { it.elementType == PROPERTY } ?: return + val nearComment = node.findChildByType(TokenSet.create(KDOC, EOL_COMMENT, BLOCK_COMMENT)) + val prevComment = nearComment?.prevSibling() + val nextComment = nearComment?.nextSibling() + val isCorrectEolComment = (prevComment == null || !prevComment.textContains('\n')) && + nextComment == null val hasCommentBefore = node .findChildByType(TokenSet.create(KDOC, EOL_COMMENT, BLOCK_COMMENT)) @@ -100,7 +105,7 @@ class ClassLikeStructuresOrderRule(configRules: List) : DiktatRule( (previousProperty.psi as KtProperty).accessors.isNotEmpty() val whiteSpaceBefore = previousProperty.nextSibling { it.elementType == WHITE_SPACE } ?: return - val isBlankLineRequired = hasCommentBefore || hasAnnotationsBefore || hasCustomAccessors + val isBlankLineRequired = (!isCorrectEolComment && hasCommentBefore) || hasAnnotationsBefore || hasCustomAccessors val numRequiredNewLines = 1 + (if (isBlankLineRequired) 1 else 0) val actualNewLines = whiteSpaceBefore.text.count { it == '\n' } // for some cases (now - if this or previous property has custom accessors), blank line is allowed before it diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/WhiteSpaceRule.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/WhiteSpaceRule.kt index b761d888e9..3de6a54d3f 100644 --- a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/WhiteSpaceRule.kt +++ b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/WhiteSpaceRule.kt @@ -462,7 +462,8 @@ class WhiteSpaceRule(configRules: List) : DiktatRule( private val log = KotlinLogging.logger {} const val NAME_ID = "horizontal-whitespace" - private const val NUM_PARENTS_FOR_LAMBDA = 3 // this is the number of parent nodes needed to check if this node is lambda from argument list + // this is the number of parent nodes needed to check if this node is lambda from argument list + private const val NUM_PARENTS_FOR_LAMBDA = 3 private val keywordsWithSpaceAfter = TokenSet.create( // these keywords are followed by { ELSE_KEYWORD, TRY_KEYWORD, DO_KEYWORD, FINALLY_KEYWORD, INIT_KEYWORD, diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt index 9cc42d3575..a7780eabd7 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt @@ -32,4 +32,10 @@ class ClassLikeStructuresOrderFixTest : FixTestBase("test/paragraph3/file_struct fun `regression - should not move loggers that depend on other variables from scope`() { fixAndCompare("LoggerOrderExpected.kt", "LoggerOrderTest.kt") } + + @Test + @Tag(WarningNames.BLANK_LINE_BETWEEN_PROPERTIES) + fun `should add new line before the comment`() { + fixAndCompare("CompanionObjectWithCommentExpected.kt", "CompanionObjectWithCommentTest.kt") + } } diff --git a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderRuleWarnTest.kt b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderRuleWarnTest.kt index 8a34208d2f..3334c25445 100644 --- a/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/ClassLikeStructuresOrderRuleWarnTest.kt @@ -196,6 +196,21 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO ) } + @Test + @Tag(WarningNames.BLANK_LINE_BETWEEN_PROPERTIES) + fun `should not trigger on EOL comment on the same line with property`() { + lintMethod( + """ + |class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) { + | companion object { + | private const val EGG_7_9_BUCKET_LABEL = "7-9" + | private const val DELAY = 15000L // 15s + | } + |} + """.trimMargin() + ) + } + @Test @Tag(WarningNames.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES) fun `should warn if order is incorrect and property with comment`() { @@ -244,7 +259,8 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO |object C { | private const val A = "value" | - | private const val LOG = "value" // Not a logger + | // Not a logger + | private const val LOG = "value" |} """.trimMargin() @@ -265,7 +281,6 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO val code = """ |object C { | private lateinit val A - | | private lateinit val LOG // Not a logger |} """.trimMargin() diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentExpected.kt new file mode 100644 index 0000000000..f8967112b6 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentExpected.kt @@ -0,0 +1,34 @@ +package com.saveourtool.diktat + +import io.micrometer.core.instrument.MeterRegistry +import org.springframework.stereotype.Component + +/** + * @param meterRegistry + * @param binRepository + */ +@Component +class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) { + companion object { + private const val EGG_4_5_BUCKET_LABEL = "4-5" + private const val EGG_3_BUCKET_LABEL = "3" + private const val EGG_OVER_10_BUCKET_LABEL = "10+" + private const val EGG_7_9_BUCKET_LABEL = "7-9" + private const val DELAY = 15000L + + // 15s + private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins" + private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs" + private const val ALL_ACTIVE_BINS_LABEL = "total" + private const val EGG_2_BUCKET_LABEL = "2" + private const val EGG_1_BUCKET_LABEL = "1" + private val numberOfEggsBuckets = setOf( + EGG_4_5_BUCKET_LABEL, + EGG_2_BUCKET_LABEL, + EGG_3_BUCKET_LABEL, + EGG_7_9_BUCKET_LABEL, + EGG_1_BUCKET_LABEL, + EGG_OVER_10_BUCKET_LABEL, + ALL_ACTIVE_BINS_LABEL) + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentTest.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentTest.kt new file mode 100644 index 0000000000..2775676e1c --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/file_structure/CompanionObjectWithCommentTest.kt @@ -0,0 +1,33 @@ +package com.saveourtool.diktat + +import io.micrometer.core.instrument.MeterRegistry +import org.springframework.stereotype.Component + +/** + * @param meterRegistry + * @param binRepository + */ +@Component +class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) { + companion object { + private const val EGG_4_5_BUCKET_LABEL = "4-5" + private const val EGG_3_BUCKET_LABEL = "3" + private const val EGG_OVER_10_BUCKET_LABEL = "10+" + private const val EGG_7_9_BUCKET_LABEL = "7-9" + private const val DELAY = 15000L + // 15s + private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins" + private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs" + private const val ALL_ACTIVE_BINS_LABEL = "total" + private const val EGG_2_BUCKET_LABEL = "2" + private const val EGG_1_BUCKET_LABEL = "1" + private val numberOfEggsBuckets = setOf( + EGG_4_5_BUCKET_LABEL, + EGG_2_BUCKET_LABEL, + EGG_3_BUCKET_LABEL, + EGG_7_9_BUCKET_LABEL, + EGG_1_BUCKET_LABEL, + EGG_OVER_10_BUCKET_LABEL, + ALL_ACTIVE_BINS_LABEL) + } +}