diff --git a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/Case.kt b/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/Case.kt index edffa87a0..785d19bac 100644 --- a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/Case.kt +++ b/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"), diff --git a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ProtectedMemberInFinalClassSpec.kt b/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ProtectedMemberInFinalClassSpec.kt index fe59491a3..b3b0b120a 100644 --- a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ProtectedMemberInFinalClassSpec.kt +++ b/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() } } }) diff --git a/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassNegative.kt b/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassNegative.kt deleted file mode 100644 index 39498c04b..000000000 --- a/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassNegative.kt +++ /dev/null @@ -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() {} -} diff --git a/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassPositive.kt b/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassPositive.kt deleted file mode 100644 index 9dd29d553..000000000 --- a/detekt-rules/src/test/resources/cases/ProtectedMemberInFinalClassPositive.kt +++ /dev/null @@ -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