Skip to content

Commit

Permalink
Fixed bug in WRONG_DECLARATIONS_ORDER rule (#1881)
Browse files Browse the repository at this point in the history
- Fixed new line, which appeared before line with comment.
- Added warn and fix tests.

Closes #1865
  • Loading branch information
diphtongue authored Dec 20, 2023
1 parent de959e2 commit 74d62a9
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,19 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : 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) {
return
}

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))
Expand All @@ -100,7 +105,7 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,8 @@ class WhiteSpaceRule(configRules: List<RulesConfig>) : 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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`() {
Expand Down Expand Up @@ -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()

Expand All @@ -265,7 +281,6 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO
val code = """
|object C {
| private lateinit val A
|
| private lateinit val LOG // Not a logger
|}
""".trimMargin()
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 74d62a9

Please sign in to comment.