Cleanup tests in ProtectedMemberInFinalClass (#2752)

* Cleanup tests in ProtectedMemberInFinalClass

* Remove extra `reports` comments

* Improve the test description

Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>

detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/Case.kt

@@ -16,8 +16,6 @@ enum class Case(val file: String) {
     EqualsAlwaysReturnsTrueOrFalseNegative("/cases/EqualsAlwaysReturnsTrueOrFalseNegative.kt"),
     ExceptionRaisedInMethodsNegative("/cases/ExceptionRaisedInMethodsNegative.kt"),
     ExceptionRaisedInMethodsPositive("/cases/ExceptionRaisedInMethodsPositive.kt"),
-    FinalClassNegative("/cases/ProtectedMemberInFinalClassNegative.kt"),
-    FinalClassPositive("/cases/ProtectedMemberInFinalClassPositive.kt"),
     FunctionReturningConstantPositive("/cases/FunctionReturningConstantPositive.kt"),
     FunctionReturningConstantNegative("/cases/FunctionReturningConstantNegative.kt"),
     IteratorImplNegative("/cases/IteratorImplNegative.kt"),

detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ProtectedMemberInFinalClassSpec.kt

@@ -1,9 +1,9 @@
 package io.gitlab.arturbosch.detekt.rules.style
 
 import io.gitlab.arturbosch.detekt.api.Config
-import io.gitlab.arturbosch.detekt.rules.Case
-import io.gitlab.arturbosch.detekt.test.lint
-import org.assertj.core.api.Assertions.assertThat
+import io.gitlab.arturbosch.detekt.api.SourceLocation
+import io.gitlab.arturbosch.detekt.test.assertThat
+import io.gitlab.arturbosch.detekt.test.compileAndLint
 import org.spekframework.spek2.Spek
 import org.spekframework.spek2.style.specification.describe
 
@@ -12,14 +12,196 @@ class ProtectedMemberInFinalClassSpec : Spek({
 
     describe("check all variants of protected visibility modifier in final class") {
 
-        it("reports protected visibility") {
-            val path = Case.FinalClassPositive.path()
-            assertThat(subject.lint(path)).hasSize(13)
+        it("reports a protected field in a final class") {
+            val code = """
+                class Foo {
+                    protected var i1 = 0
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(2, 5)
         }
 
-        it("does not report protected visibility") {
-            val path = Case.FinalClassNegative.path()
-            assertThat(subject.lint(path)).isEmpty()
+        it("reports a protected constructor in a final class") {
+            val code = """
+                class Foo {
+                    var i1: Int = 0
+                    protected constructor(i1: Int) : super() {
+                        this.i1 = i1
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(3, 5)
+        }
+
+        it("reports a protected function in a final class") {
+            val code = """
+                class Foo {
+                    protected fun function() {}
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(2, 5)
+        }
+
+        it("reports an inner class with a protected field in a final class") {
+            val code = """
+                class Foo {
+                    inner class InnerClass2 {
+                        protected val i = 0
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(3, 9)
+        }
+
+        it("reports a protected inner class with a protected field in a final class") {
+            val code = """
+                class Fee {
+                    protected inner class YetAnotherInnerClass {
+                        protected val i = 0
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(2)
+            assertThat(findings).hasSourceLocations(
+                SourceLocation(2, 5),
+                SourceLocation(3, 9)
+            )
+        }
+
+        it("reports a protected companion object in a final class") {
+            val code = """
+                class Foo {
+                    protected companion object {
+                        protected class A {
+                            protected var x = 0
+                        }
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(3)
+            assertThat(findings).hasSourceLocations(
+                SourceLocation(2, 5),
+                SourceLocation(2, 5),
+                SourceLocation(4, 13)
+            )
+        }
+
+        it("reports a protected companion object in an nested class") {
+            val code = """
+                abstract class Foo {
+                    protected companion object {
+                        protected class A {
+                            protected var x = 0
+                        }
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(4, 13)
+        }
+
+        it("reports a protected field object in a final inner class") {
+            val code = """
+                open class OpenClass {
+                    inner class InnerClass {
+                        protected val i = 0
+                    }
+                }
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(3, 9)
+        }
+
+        it("reports a protected primary constructor in a final class") {
+            val code = """
+                class FinalClassWithProtectedConstructor protected constructor()
+            """
+            val findings = subject.compileAndLint(code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(1, 42)
+        }
+    }
+
+    describe("check valid occurrences of protected that should not be reported") {
+
+        it("does not report non-protected members in final class") {
+            val code = """
+                abstract class BaseClass
+                class Foo : BaseClass() {
+                    private val i = 0
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        it("does not report overridden fields") {
+            val code = """
+                abstract class BaseClass {
+                    protected abstract val abstractProp : Int
+                }
+                class Foo : BaseClass() {
+                    // should not report protected = private visibility
+                    protected override val abstractProp = 0
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        it("does not report overridden functions") {
+            val code = """
+                abstract class BaseClass {
+                    protected abstract fun abstractFunction()
+                }
+                class Foo : BaseClass() {
+                    // should not report protected = private visibility
+                    protected override fun abstractFunction() {
+                    }
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        it("does not report protected definitions in abstract class") {
+            val code = """
+                abstract class BaseClass {
+                    protected abstract val abstractProp: Int
+                    protected abstract fun abstractFunction()
+                
+                    protected object InnerObject
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        it("does not report protected definitions in sealed class") {
+            val code = """
+                sealed class SealedClass {
+                    protected fun a() {}
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        it("does not report protected definitions in enum class") {
+            val code = """
+                enum class EnumClass {
+                    ;   
+                    protected fun foo() {}
+                }
+            """
+            assertThat(subject.compileAndLint(code)).isEmpty()
         }
     }
 })

detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassNegative.kt

@@ -1,33 +0,0 @@
-@file:Suppress("unused", "RedundantVisibilityModifier", "ProtectedInFinal", "ConvertSecondaryConstructorToPrimary")
-
-package cases
-
-class NoProtectedMembersInFinalClass : BaseClass() {
-
-    private val i = 0
-
-    // should not report protected = private visibility
-    protected override val abstractProp = 0
-
-    // should not report protected = private visibility
-    protected override fun abstractFunction() {
-    }
-}
-
-abstract class BaseClass {
-
-    protected abstract val abstractProp: Int
-    protected abstract fun abstractFunction()
-
-    protected object InnerObject
-}
-
-sealed class SealedClass {
-
-    protected fun a() {}
-}
-
-enum class EnumClass {
-    ;
-    protected fun foo() {}
-}

detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassPositive.kt

@@ -1,48 +0,0 @@
-@file:Suppress("unused", "RedundantVisibilityModifier", "ProtectedInFinal", "ConvertSecondaryConstructorToPrimary")
-
-package cases
-
-class ProtectedMemberInFinalClassPositive {
-
-    protected var i1 = 0 // reports 1
-
-    protected constructor(i1: Int) : super() { // reports 1
-        this.i1 = i1
-    }
-
-    protected fun function() {} // reports 1
-
-    protected inner class InnerClass1 { // reports 1
-        protected val i = 0 // reports 1
-    }
-
-    inner class InnerClass2 {
-        protected val i = 0 // reports 1
-    }
-
-    protected object InnerObject // reports 1
-
-    protected companion object { // reports 1
-        protected class A { // reports 1
-            protected var x = 0 // reports 1
-        }
-    }
-}
-
-abstract class ClassWithAbstractCompanionMembers {
-
-    protected companion object {
-        protected class A {
-            protected var x = 0 // reports 1
-        }
-    }
-}
-
-open class OpenClass {
-
-    inner class InnerClass {
-        protected val i = 0 // reports 1
-    }
-}
-
-class FinalClassWithProtectedConstructor protected constructor() // reports 1