Skip to content

Commit

Permalink
WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES: fix the IOOBE (#1519)
Browse files Browse the repository at this point in the history
### What's done:

 * `const` properties named `log` or `logger` are no longer treated as loggers.
 * `lateinit` properties named `log` or `logger` are no longer treated as
   loggers.
 * Arbitrary properties containing `log` or `logger` in their names
   (e.g.: `loginName` or `LOGIN_NAME`) are no longer treated as loggers.
 * A diagnostic check added.
 * An IOOBE is no longer thrown.
 * Closes #1516.
  • Loading branch information
0x6675636b796f75676974687562 authored Sep 12, 2022
1 parent 2ae3500 commit 1756fc3
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,23 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : DiktatRule(
@Suppress("UnsafeCallOnNullableType")
private fun ASTNode.checkAndReorderBlocks(blocks: List<List<ASTNode>>) {
val classChildren = this.children().filter { it.elementType in childrenTypes }.toList()

check(blocks.size == classChildren.size) {
StringBuilder().apply {
append("`classChildren` has a size of ${classChildren.size} while `blocks` has a size of ${blocks.size}$NEWLINE")

append("`blocks`:$NEWLINE")
blocks.forEachIndexed { index, block ->
append("\t$index: ${block.firstOrNull()?.text}$NEWLINE")
}

append("`classChildren`:$NEWLINE")
classChildren.forEachIndexed { index, child ->
append("\t$index: ${child.text}$NEWLINE")
}
}
}

if (classChildren != blocks.map { it.first() }) {
blocks.filterIndexed { index, pair -> classChildren[index] != pair.first() }
.forEach { listOfChildren ->
Expand Down Expand Up @@ -180,9 +197,22 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : DiktatRule(
val allProperties = node.getAllChildrenWithType(PROPERTY)
val constProperties = allProperties.filterByModifier(CONST_KEYWORD)
val lateInitProperties = allProperties.filterByModifier(LATEINIT_KEYWORD)
val loggers = allProperties.filterByModifier(PRIVATE_KEYWORD).filter {
it.getIdentifierName()!!.text.contains(loggerPropertyRegex)
}
val loggers = allProperties.filterByModifier(PRIVATE_KEYWORD)
.filterNot { astNode ->
/*
* A `const` field named "logger" is unlikely to be a logger.
*/
astNode in constProperties
}
.filterNot { astNode ->
/*
* A `lateinit` field named "logger" is unlikely to be a logger.
*/
astNode in lateInitProperties
}
.filter { astNode ->
astNode.getIdentifierName()?.text?.matches(loggerPropertyRegex) ?: false
}
val properties = allProperties.filter { it !in lateInitProperties && it !in loggers && it !in constProperties }
return AllProperties(loggers, constProperties, properties, lateInitProperties)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ val KOTLIN = KtTokens.KEYWORDS
.map { line -> line.toString() }
.plus(KtTokens.SOFT_KEYWORDS.types.map { line -> line.toString() })

val loggerPropertyRegex = "(log|LOG|logger)".toRegex()
/**
* Either `log` or `logger`, case-insensitive.
*
* A name like `psychologist` or `LOGIN` won't be matched by this regular
* expression.
*/
val loggerPropertyRegex = "(?iu)^log(?:ger)?$".toRegex()

/**
* @return whether [this] string represents a Java keyword
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package org.cqfn.diktat.ruleset.chapter3

import org.cqfn.diktat.common.config.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.chapter3.spaces.describe
import org.cqfn.diktat.ruleset.constants.Warnings.BLANK_LINE_BETWEEN_PROPERTIES
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES
import org.cqfn.diktat.ruleset.rules.chapter3.ClassLikeStructuresOrderRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import generated.WarningNames
import org.assertj.core.api.Assertions.assertThat
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

Expand All @@ -16,6 +19,7 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO

// ===== order of declarations =====

@Language("kotlin")
private fun codeTemplate(keyword: String) = """
|$keyword Example {
| private val log = LoggerFactory.getLogger(Example.javaClass)
Expand Down Expand Up @@ -213,4 +217,69 @@ class ClassLikeStructuresOrderRuleWarnTest : LintTestBase(::ClassLikeStructuresO
""".trimMargin()
)
}

/**
* An exception to the "loggers on top" rule.
*
* See [#1516](https://github.com/saveourtool/diktat/issues/1516).
*/
@Test
@Tag(WarningNames.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES)
fun `logger-like const property should not be reordered`() {
val code = """
|object C {
| private const val A = "value"
|
| private const val LOG = "value" // Not a logger
|}
""".trimMargin()

val actualErrors = lintResult(code)
assertThat(actualErrors)
.describedAs("lint result for ${code.describe()}")
.isEmpty()
}

/**
* An exception to the "loggers on top" rule.
*
* See [#1516](https://github.com/saveourtool/diktat/issues/1516).
*/
@Test
@Tag(WarningNames.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES)
fun `logger-like lateinit property should not be reordered`() {
val code = """
|object C {
| private lateinit val A
|
| private lateinit val LOG // Not a logger
|}
""".trimMargin()

val actualErrors = lintResult(code)
assertThat(actualErrors)
.describedAs("lint result for ${code.describe()}")
.isEmpty()
}

/**
* An exception to the "loggers on top" rule.
*
* See [#1516](https://github.com/saveourtool/diktat/issues/1516).
*/
@Test
@Tag(WarningNames.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES)
fun `property with a name containing 'log' is not a logger`() {
val code = """
|object C {
| private val a = System.getProperty("os.name")
| private val loginName = LoggerFactory.getLogger({}.javaClass)
|}
""".trimMargin()

val actualErrors = lintResult(code)
assertThat(actualErrors)
.describedAs("lint result for ${code.describe()}")
.isEmpty()
}
}

0 comments on commit 1756fc3

Please sign in to comment.