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

loggers sometimes shouldn't be first #1624

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,11 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : DiktatRule(
* @property properties all other properties
* @property lateInitProperties `lateinit var`s
*/
private data class AllProperties(val loggers: List<ASTNode>,
val constProperties: List<ASTNode>,
val properties: List<ASTNode>,
val lateInitProperties: List<ASTNode>
private data class AllProperties(
val loggers: List<ASTNode>,
val constProperties: List<ASTNode>,
val properties: List<ASTNode>,
val lateInitProperties: List<ASTNode>
) {
companion object {
/**
Expand All @@ -197,6 +198,7 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : DiktatRule(
val allProperties = node.getAllChildrenWithType(PROPERTY)
val constProperties = allProperties.filterByModifier(CONST_KEYWORD)
val lateInitProperties = allProperties.filterByModifier(LATEINIT_KEYWORD)
val referencesFromSameScope = allProperties.mapNotNull { it.getIdentifierName()?.text }
val loggers = allProperties.filterByModifier(PRIVATE_KEYWORD)
.filterNot { astNode ->
/*
Expand All @@ -213,9 +215,27 @@ class ClassLikeStructuresOrderRule(configRules: List<RulesConfig>) : DiktatRule(
.filter { astNode ->
astNode.getIdentifierName()?.text?.matches(loggerPropertyRegex) ?: false
}
.let {
getLoggerDependencyNames(it)
}
.filter { (_, dependencyReferences) ->
dependencyReferences.all {
it !in referencesFromSameScope
}
}
.keys
.toList()
Fixed Show fixed Hide fixed


val properties = allProperties.filter { it !in lateInitProperties && it !in loggers && it !in constProperties }
return AllProperties(loggers, constProperties, properties, lateInitProperties)
}

private fun getLoggerDependencyNames(loggers: List<ASTNode>): Map<ASTNode, List<String>> = loggers.map { astNode ->
Fixed Show fixed Hide fixed
astNode to astNode.findAllDescendantsWithSpecificType(REFERENCE_EXPRESSION, false)
}.associate { (astNode, possibleDependencies) ->
astNode to possibleDependencies.map { it.text }
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ fun ASTNode.getAllChildrenWithType(elementType: IElementType): List<ASTNode> =
/**
* Generates a sequence of this ASTNode's children in reversed order
*
* @return a reevrsed sequence of children
* @return a reversed sequence of children
*/
fun ASTNode.reversedChildren(): Sequence<ASTNode> = sequence {
var node = lastChildNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ class ClassLikeStructuresOrderFixTest : FixTestBase("test/paragraph3/file_struct
fun `regression - should not remove enum members`() {
fixAndCompare("OrderWithEnumsExpected.kt", "OrderWithEnumsTest.kt")
}

@Test
@Tags(Tag(WarningNames.WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES))
fun `regression - should not move loggers that depend on other variables from scope`() {
fixAndCompare("LoggerOrderExpected.kt", "LoggerOrderTest.kt")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ class Example {
}

class UnusedNested { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ class Example {
* Dolor sit amet
*/
private val BAZ = 44
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package test.paragraph3.file_structure

import org.junit.platform.commons.logging.LoggerFactory

class LoggerOrderExample {
private val logger = getLogger("string template")
private val logger = getLogger("""string template""")
private val logger = getLogger("""
string template
""")
private val logger = getLogger(this.javaClass)
private val logger = getLogger(this.javaClass.name)
private val logger = getLogger(javaClass)
private val logger = getLogger(javaClass.name)
private val logger = getLogger(Foo::class.java)
private val logger = getLogger(Foo::class.java.name)
private val logger = getLogger(Foo::class)
private val logger = getLogger(Foo::class.name)
private val logger = getLogger<Foo>()
private val logger = getLogger({}.javaClass)
private val a = "a"
private val b = "b"
private val c = "c"
private val d = "d"
private val e = "e"
private val f = "f"
private val g = "g"
private val h = "h"
private val i = "i"
private val j = "j"
private val k = "k"
private val l = "l"
private val m = "m"
private val logger = LoggerFactory.getLogger(m)
private val n = "n"
private val logger = n.getLogger()
private val o = "o"
private val logger = o { get() }
private val p = "p"
private val logger = p::class.java.enclosingClass
private val q = "q"
private val logger = q::class.java
private val r = "r"
private val logger = r::javaClass
private val s = "s"
private val t = "t"
private val logger = getLogger("$s$t")
private val u = "u"
private val v = "v"
private val w = "w"
private val x = "x"
private val y = "y"
private val z = "z"
private val logger = getLogger("$s$t$u$v$w") { x + y + z }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package test.paragraph3.file_structure

import org.junit.platform.commons.logging.LoggerFactory

class LoggerOrderExample {
private val logger = getLogger("string template")
private val a = "a"
private val logger = getLogger("""string template""")
private val b = "b"
private val logger = getLogger("""
string template
""")
private val c = "c"
private val logger = getLogger(this.javaClass)
private val d = "d"
private val logger = getLogger(this.javaClass.name)
private val e = "e"
private val logger = getLogger(javaClass)
private val f = "f"
private val logger = getLogger(javaClass.name)
private val g = "g"
private val logger = getLogger(Foo::class.java)
private val h = "h"
private val logger = getLogger(Foo::class.java.name)
private val i = "i"
private val logger = getLogger(Foo::class)
private val j = "j"
private val logger = getLogger(Foo::class.name)
private val k = "k"
private val logger = getLogger<Foo>()
private val l = "l"
private val logger = getLogger({}.javaClass)
private val m = "m"
private val logger = LoggerFactory.getLogger(m)
private val n = "n"
private val logger = n.getLogger()
private val o = "o"
private val logger = o { get() }
private val p = "p"
private val logger = p::class.java.enclosingClass
private val q = "q"
private val logger = q::class.java
private val r = "r"
private val logger = r::javaClass
private val s = "s"
private val t = "t"
private val logger = getLogger("$s$t")
private val u = "u"
private val v = "v"
private val w = "w"
private val x = "x"
private val y = "y"
private val z = "z"
private val logger = getLogger("$s$t$u$v$w") { x + y + z }
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class TestComparatorUnit(
} else {
// fixme: actualResult is separated by KtLint#determineLneSeparator, should be split by it here too
actualResult.split("\n")
}
}.also { println(it.joinToString("\n")) }
Fixed Show fixed Hide fixed

val expectedFileContent = readFile(expectedFile)

Expand Down