loggers sometimes shouldn't be first (#1624)

* loggers sometimes shouldn't be first ### What's done: * Added simple dependency check when looking for logger that could be first property * Added test (#1602)
saveourtool · Mar 16, 2023 · 82ee915 · 82ee915
1 parent cb102a2
commit 82ee915
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 7 deletions.
diff --git a/...es/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt b/...es/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/ClassLikeStructuresOrderRule.kt
@@ -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 {
             /**
@@ -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 ->
                         /*
@@ -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()
+
                 val properties = allProperties.filter { it !in lateInitProperties && it !in loggers && it !in constProperties }
                 return AllProperties(loggers, constProperties, properties, lateInitProperties)
             }
+
+            @Suppress("TYPE_ALIAS")
+            private fun getLoggerDependencyNames(loggers: List<ASTNode>): Map<ASTNode, List<String>> = loggers.map { astNode ->
+                astNode to astNode.findAllDescendantsWithSpecificType(REFERENCE_EXPRESSION, false)
+            }.associate { (astNode, possibleDependencies) ->
+                astNode to possibleDependencies.map { it.text }
+            }
         }
     }
 

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
@@ -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

diff --git a/...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt b/...rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/ClassLikeStructuresOrderFixTest.kt
@@ -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")
+    }
 }
diff --git a/...les/src/test/resources/test/paragraph3/file_structure/DeclarationsInClassOrderExpected.kt b/...les/src/test/resources/test/paragraph3/file_structure/DeclarationsInClassOrderExpected.kt
@@ -32,4 +32,4 @@ class Example {
     }
 
     class UnusedNested { }
-}
+}
diff --git a/...t-rules/src/test/resources/test/paragraph3/file_structure/DeclarationsInClassOrderTest.kt b/...t-rules/src/test/resources/test/paragraph3/file_structure/DeclarationsInClassOrderTest.kt
@@ -32,4 +32,4 @@ class Example {
      * Dolor sit amet
      */
     private val BAZ = 44
-}
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/LoggerOrderExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/LoggerOrderExpected.kt
@@ -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 }
+}
diff --git a/diktat-rules/src/test/resources/test/paragraph3/file_structure/LoggerOrderTest.kt b/diktat-rules/src/test/resources/test/paragraph3/file_structure/LoggerOrderTest.kt
@@ -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 change
@@ Expand Up / @@ -32,4 +32,4 @@ class Example { @@
         }
         class UnusedNested { }
-    }
+    }