Update RedundantVisibilityModifierRule to find redundant internal modifiers (#3092)

* Report redundant internal modifier on declarations in private or local objects and classes

* Enable RedundantVisibilityModifierRule for detekt self-scans

* Use simpler test example

Co-authored-by: Nicola Corti <corti.nico@gmail.com>

Co-authored-by: Nicola Corti <corti.nico@gmail.com>

config/detekt/detekt.yml

@@ -137,6 +137,8 @@ style:
     active: true
   ProtectedMemberInFinalClass:
     active: true
+  RedundantVisibilityModifierRule:
+    active: true
   SpacingBetweenPackageAndImports:
     active: true
   UnnecessaryAbstractClass:

detekt-metrics/src/main/kotlin/io/github/detekt/metrics/processors/util/LLOC.kt

@@ -23,7 +23,7 @@ object LLOC {
         private var escape: Boolean = false
 
         @Suppress("LoopWithTooManyJumpStatements")
-        internal fun run(): Int {
+        fun run(): Int {
             for (line in lines) {
 
                 val trimmed = line.trim()

detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/NestedBlockDepth.kt

@@ -58,9 +58,9 @@ class NestedBlockDepth(
 
     private class FunctionDepthVisitor(val threshold: Int) : DetektVisitor() {
 
-        internal var depth = 0
-        internal var maxDepth = 0
-        internal var isTooDeep = false
+        var depth = 0
+        var maxDepth = 0
+        var isTooDeep = false
         private fun inc() {
             depth++
             if (depth >= threshold) {

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/EqualsWithHashCodeExist.kt

@@ -59,7 +59,7 @@ class EqualsWithHashCodeExist(config: Config = Config.empty) : Rule(config) {
     private val queue = ArrayDeque<ViolationHolder>(MAXIMUM_EXPECTED_NESTED_CLASSES)
 
     private data class ViolationHolder(var equals: Boolean = false, var hashCode: Boolean = false) {
-        internal fun violation() = equals && !hashCode || !equals && hashCode
+        fun violation() = equals && !hashCode || !equals && hashCode
     }
 
     override fun visitFile(file: PsiFile?) {

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/RedundantVisibilityModifierRule.kt

@@ -8,13 +8,17 @@ import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.rules.isInternal
 import io.gitlab.arturbosch.detekt.rules.isOverride
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtModifierListOwner
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.KtProperty
+import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
+import org.jetbrains.kotlin.psi.psiUtil.isPrivate
 
 /**
  * This rule checks for redundant visibility modifiers.
@@ -38,9 +42,7 @@ class RedundantVisibilityModifierRule(config: Config = Config.empty) : Rule(conf
     override val issue: Issue = Issue(
         "RedundantVisibilityModifierRule",
         Severity.Style,
-        "Checks for redundant visibility modifiers. " +
-            "Public is the default visibility for classes. " +
-            "The public modifier is redundant.",
+        "Checks for redundant visibility modifiers.",
         Debt.FIVE_MINS
     )
 
@@ -59,6 +61,22 @@ class RedundantVisibilityModifierRule(config: Config = Config.empty) : Rule(conf
         }
     }
 
+    override fun visitDeclaration(declaration: KtDeclaration) {
+        super.visitDeclaration(declaration)
+        if (
+            declaration.isInternal() &&
+            declaration.containingClassOrObject?.let { it.isLocal || it.isPrivate() } == true
+        ) {
+            report(
+                CodeSmell(
+                    issue,
+                    Entity.from(declaration),
+                    "The `internal` modifier on ${declaration.name} is redundant and should be removed."
+                )
+            )
+        }
+    }
+
     private inner class ClassVisitor : DetektVisitor() {
         override fun visitClass(klass: KtClass) {
             super.visitClass(klass)

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

@@ -127,5 +127,27 @@ class RedundantVisibilityModifierRuleSpec : Spek({
             """
             assertThat(subject.compileAndLint(code)).isEmpty()
         }
+
+        it("reports internal modifier on nested class in private object") {
+            val code = """
+                private object A {
+
+                    internal class InternalClass
+
+                }
+            """
+            assertThat(subject.compileAndLint(code)).hasSize(1)
+        }
+
+        it("reports internal modifier on function declaration in private object") {
+            val code = """
+                private object A {
+
+                    internal fun internalFunction() {}
+
+                }
+            """
+            assertThat(subject.compileAndLint(code)).hasSize(1)
+        }
     }
 })