Repository: detekt/detekt (kotlin)

Issue:
ClassOrdering rule prints incorrect message
<!-- The bug you're experiencing might have already be reported! -->
<!-- Please search in the [issues](https://github.com/detekt/detekt/issues) before creating one. -->

## Expected Behavior
<!-- Tell us what should happen -->

## Observed Behavior
<!-- Tell us what happens instead of the expected behavior -->

## Steps to Reproduce
<!-- Provide a link to a live example, or an unambiguous set of steps to -->
<!-- reproduce this bug. Include code to reproduce, if relevant -->
Example class that can triggers this behavior
```
class ExampleClass {

    var var1 = 1

    fun bar() {

    }

    var var2 = 2

    init {
    }

    companion object {
    }
}
```

## Context
<!-- How has this issue affected you? What are you trying to accomplish? -->
<!-- Providing context helps us come up with a solution that is most useful in the real world -->
Sometimes ClassOrdering Rule print you incorrect messages. For example in code above it types "ClassOrdering: method `bar()` should be declared before companion object." despite bar() already declared before companion object. The correct error here is "method `bar()` should be declared after property declarations and initializer blocks"

## Your Environment
<!-- Include as many relevant details about the environment you experienced the bug in -->
* Version of detekt used: 1.23.6
* Version of Gradle used (if applicable):
* Gradle scan link (add `--scan` option when running the gradle task):
* Operating System and version:
* Link to your project (if it's a public repository):

Pull request:
the violating element in case of reporting the `before` msg

Fixes https://github.com/detekt/detekt/issues/7407

Patch:
diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrdering.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrdering.kt
index 8cb3b2614c81..3f13eea54a8c 100644
--- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrdering.kt
+++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrdering.kt
@@ -12,8 +12,6 @@ import org.jetbrains.kotlin.psi.KtObjectDeclaration
 import org.jetbrains.kotlin.psi.KtProperty
 import org.jetbrains.kotlin.psi.KtSecondaryConstructor
 
-private typealias DeclarationToSectionPair = Pair<KtDeclaration, Section>
-
 /**
  * This rule ensures class contents are ordered as follows as recommended by the Kotlin
  * [Coding Conventions](https://kotlinlang.org/docs/coding-conventions.html#class-layout):
@@ -59,22 +57,39 @@ class ClassOrdering(config: Config) : Rule(
     override fun visitClassBody(classBody: KtClassBody) {
         super.visitClassBody(classBody)
 
-        val sectionList = classBody.declarations.filterNotNull()
-        if (sectionList.isEmpty()) return
-        val (violationDeclarationToSectionPair, listOfIncreasingSection) = getMinimalNumberOfViolations(sectionList)
-            ?: return
-        violationDeclarationToSectionPair.forEach { (declaration, section) ->
-            val (directionMsg, anchorSection) = listOfIncreasingSection.find { it.priority > section.priority }?.let {
-                "before" to it
-            } ?: run {
-                "after" to listOfIncreasingSection.findLast { it.priority < section.priority }
-            }
+        val declarations = classBody.declarations.filterNotNull()
+        if (declarations.isEmpty()) return
+        val (violatingDeclarationWithSections, increasingDeclarationWithSections) = getMinimalNumberOfViolations(
+            declarations
+        ) ?: return
+        violatingDeclarationWithSections.forEach { (violatingDeclaration, violatingSection) ->
+            val increasingDeclarationsBeforeViolatingElement =
+                declarations.takeWhile { it != violatingDeclaration }
+            val increasingDeclarationSectionBeforeViolatingElement =
+                increasingDeclarationWithSections.takeWhile {
+                    it.declaration in increasingDeclarationsBeforeViolatingElement
+                }
+            // for finding section from which violatingSection should be before we are only
+            // taking declarations which is already before the violatingSection
+            val (directionMsg, anchorSection) = increasingDeclarationSectionBeforeViolatingElement
+                .find {
+                    it.section.priority > violatingSection.priority
+                }
+                ?.let {
+                    "before" to it
+                }
+                ?: run {
+                    "after" to
+                        increasingDeclarationWithSections
+                            .findLast { it.section.priority < violatingSection.priority }
+                }
             anchorSection ?: return@forEach
             val message =
-                "${declaration.toDescription()} should be declared $directionMsg ${anchorSection.toDescription()}."
+                "${violatingDeclaration.toDescription()} should be declared $directionMsg " +
+                    "${anchorSection.section.toDescription()}."
             report(
                 CodeSmell(
-                    entity = Entity.from(declaration),
+                    entity = Entity.from(violatingDeclaration),
                     message = message,
                     references = listOf(Entity.from(classBody))
                 )
@@ -84,10 +99,13 @@ class ClassOrdering(config: Config) : Rule(
 
     private fun getMinimalNumberOfViolations(
         declarations: List<KtDeclaration>,
-    ): Pair<List<DeclarationToSectionPair>, List<Section>>? {
+    ): Pair<List<DeclarationWithSection>, List<DeclarationWithSection>>? {
         val declarationWithSectionList = declarations.mapNotNull { declaration ->
             declaration.toSection()?.let {
-                declaration to it
+                DeclarationWithSection(
+                    declaration,
+                    it
+                )
             }
         }
         val dp = IntArray(declarationWithSectionList.size) {
@@ -98,7 +116,8 @@ class ClassOrdering(config: Config) : Rule(
         }
         for (i in dp.indices) {
             for (j in 0..<i) {
-                if (declarationWithSectionList[i].second.priority >= declarationWithSectionList[j].second.priority &&
+                if (declarationWithSectionList[i].section.priority >=
+                    declarationWithSectionList[j].section.priority &&
                     dp[i] < dp[j] + 1
                 ) {
                     dp[i] = dp[j] + 1
@@ -118,8 +137,13 @@ class ClassOrdering(config: Config) : Rule(
             } while (index != oldIndex)
         }.reversed()
         return declarationWithSectionList.minus(listOfIncreasingSection.toSet()) to
-            listOfIncreasingSection.map { it.second }
+            listOfIncreasingSection
     }
+
+    private data class DeclarationWithSection(
+        val declaration: KtDeclaration,
+        val section: Section,
+    )
 }
 
 private fun KtDeclaration.toDescription(): String = when {


Test patch:
diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrderingSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrderingSpec.kt
index 58845a60b6cc..24fcda346c3d 100644
--- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrderingSpec.kt
+++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ClassOrderingSpec.kt
@@ -137,7 +137,7 @@ class ClassOrderingSpec {
         val findings = subject.compileAndLint(code)
         assertThat(findings).hasSize(1)
         assertThat(findings[0].message).isEqualTo(
-            "secondary constructor should be declared before method declarations."
+            "secondary constructor should be declared after property declarations and initializer blocks."
         )
     }
 
@@ -164,7 +164,7 @@ class ClassOrderingSpec {
         val findings = subject.compileAndLint(code)
         assertThat(findings).hasSize(1)
         assertThat(findings[0].message)
-            .isEqualTo("method `returnX()` should be declared before companion object.")
+            .isEqualTo("method `returnX()` should be declared after secondary constructors.")
     }
 
     @Test
@@ -297,9 +297,9 @@ class ClassOrderingSpec {
     }
 
     @Test
-    fun `does report before issue when loweset sections comes after longest change`() {
+    fun `does report before issue when lowest sections comes after longest change`() {
         val code = """
-            class SingleMisorderAtFirst(private val x: String) {
+            class SingleMisOrderAtFirst(private val x: String) {
                 companion object {
                     const val IMPORTANT_VALUE = 3
                 }
@@ -322,4 +322,34 @@ class ClassOrderingSpec {
         assertThat(findings[0].message)
             .isEqualTo("companion object should be declared after method declarations.")
     }
+
+    @Test
+    fun `#7407 does report before issue when there is not lower section above violating section`() {
+        val code = """
+            class ExampleClass {
+
+                var var1 = 1
+
+                fun bar() {
+            
+                }
+
+                var var2 = 2
+
+                init {
+                    /* no-op */
+                }
+ 
+                companion object {
+                    /* no-op */
+                }
+            }
+
+        """.trimIndent()
+
+        val findings = subject.compileAndLint(code)
+        assertThat(findings).hasSize(1)
+        assertThat(findings[0].message)
+            .isEqualTo("method `bar()` should be declared after property declarations and initializer blocks.")
+    }
 }