Add visibility-threshold to ComposeModifierMissing rule

compose-lint-checks/src/main/java/slack/lint/compose/ModifierMissingDetector.kt

@@ -8,12 +8,14 @@ import com.android.tools.lint.detector.api.Issue
 import com.android.tools.lint.detector.api.JavaContext
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.android.tools.lint.detector.api.StringOption
 import com.android.tools.lint.detector.api.TextFormat
 import org.jetbrains.kotlin.psi.KtFunction
 import org.jetbrains.kotlin.psi.psiUtil.isPublic
 import slack.lint.compose.util.Priorities
 import slack.lint.compose.util.definedInInterface
 import slack.lint.compose.util.emitsContent
+import slack.lint.compose.util.isInternal
 import slack.lint.compose.util.isOverride
 import slack.lint.compose.util.isPreview
 import slack.lint.compose.util.modifierParameter
@@ -30,6 +32,14 @@ constructor(
   companion object {
 
     val CONTENT_EMITTER_OPTION = ContentEmitterLintOption.newOption()
+    internal val VISIBILITY_THRESHOLD =
+      StringOption(
+        name = "visibility-threshold",
+        description = "Visibility threshold to check for Modifiers",
+        defaultValue = "only_public",
+        explanation =
+          "You can control the visibility of which composables to check for Modifiers. Possible values are: `only_public` (default), `public_and_internal` and `all`"
+      )
 
     val ISSUE =
       Issue.create(
@@ -45,25 +55,37 @@ constructor(
           severity = Severity.ERROR,
           implementation = sourceImplementation<ModifierMissingDetector>()
         )
-        .setOptions(listOf(CONTENT_EMITTER_OPTION))
+        .setOptions(listOf(CONTENT_EMITTER_OPTION, VISIBILITY_THRESHOLD))
   }
 
   override fun visitComposable(context: JavaContext, function: KtFunction) {
     // We want to find all composable functions that:
-    //  - are public
     //  - emit content
     //  - are not overridden or part of an interface
     //  - are not a @Preview composable
     if (
-      !function.isPublic ||
-        function.returnsValue ||
+      function.returnsValue ||
         function.isOverride ||
         function.definedInInterface ||
         function.isPreview
     ) {
       return
     }
 
+    // We want to check now the visibility to see whether it's allowed by the configuration
+    // Possible values:
+    // - only_public: will check for modifiers only on public composables
+    // - public_and_internal: will check for public and internal composables
+    // - all: will check all composables (public, internal, protected, private
+    val shouldCheck =
+      when (VISIBILITY_THRESHOLD.getValue(context.configuration)) {
+        "only_public" -> function.isPublic
+        "public_and_internal" -> function.isPublic || function.isInternal
+        "all" -> true
+        else -> function.isPublic
+      }
+    if (!shouldCheck) return
+
     // If there is a modifier param, we bail
     if (function.modifierParameter != null) return
 

compose-lint-checks/src/test/java/slack/lint/compose/ModifierMissingDetectorTest.kt

@@ -114,7 +114,7 @@ class ModifierMissingDetectorTest : BaseSlackLintTest() {
   }
 
   @Test
-  fun `non-public visibility Composables are ignored`() {
+  fun `non-public visibility Composables are ignored (by default)`() {
     @Language("kotlin")
     val code =
       """
@@ -145,6 +145,115 @@ class ModifierMissingDetectorTest : BaseSlackLintTest() {
     lint().files(kotlin(code)).allowCompilationErrors().run().expectClean()
   }
 
+  @Test
+  fun `public and internal visibility Composables are checked for 'public_and_internal' configuration`() {
+    @Language("kotlin")
+    val code =
+      """
+                @Composable
+                fun Something() {
+                    Row {
+                    }
+                }
+                @Composable
+                protected fun Something() {
+                    Column(modifier = Modifier.fillMaxSize()) {
+                    }
+                }
+                @Composable
+                internal fun Something() {
+                    SomethingElse {
+                        Box(modifier = Modifier.fillMaxSize()) {
+                        }
+                    }
+                }
+                @Composable
+                private fun Something() {
+                    Whatever(modifier = Modifier.fillMaxSize()) {
+                    }
+                }
+            """
+        .trimIndent()
+    lint()
+      .files(kotlin(code))
+      .allowCompilationErrors()
+      .configureOption(ModifierMissingDetector.VISIBILITY_THRESHOLD, "public_and_internal")
+      .run()
+      .expect(
+        """
+          src/test.kt:2: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          fun Something() {
+              ~~~~~~~~~
+          src/test.kt:12: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          internal fun Something() {
+                       ~~~~~~~~~
+          2 errors, 0 warnings
+        """
+          .trimIndent()
+      )
+  }
+
+  @Test
+  fun `all Composables are checked for 'all' configuration`() {
+    @Language("kotlin")
+    val code =
+      """
+                @Composable
+                fun Something() {
+                    Row {
+                    }
+                }
+                @Composable
+                protected fun Something() {
+                    Column(modifier = Modifier.fillMaxSize()) {
+                    }
+                }
+                @Composable
+                internal fun Something() {
+                    SomethingElse {
+                        Box(modifier = Modifier.fillMaxSize()) {
+                        }
+                    }
+                }
+                @Composable
+                private fun Something() {
+                    Whatever(modifier = Modifier.fillMaxSize()) {
+                    }
+                }
+            """
+        .trimIndent()
+
+    lint()
+      .files(kotlin(code))
+      .allowCompilationErrors()
+      .configureOption(ModifierMissingDetector.VISIBILITY_THRESHOLD, "all")
+      .run()
+      .expect(
+        """
+          src/test.kt:2: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          fun Something() {
+              ~~~~~~~~~
+          src/test.kt:7: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          protected fun Something() {
+                        ~~~~~~~~~
+          src/test.kt:12: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          internal fun Something() {
+                       ~~~~~~~~~
+          src/test.kt:19: Error: This @Composable function emits content but doesn't have a modifier parameter.
+          See https://slackhq.github.io/compose-lints/rules/#when-should-i-expose-modifier-parameters for more information. [ComposeModifierMissing]
+          private fun Something() {
+                      ~~~~~~~~~
+          4 errors, 0 warnings
+        """
+          .trimIndent()
+      )
+  }
+
   @Test
   fun `interface Composables are ignored`() {
     @Language("kotlin")

docs/rules.md

@@ -246,6 +246,20 @@ More info: [Always provide a Modifier parameter](https://chris.banes.dev/posts/a
 
 Related rule: [`ComposeModifierMissing`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/ModifierMissingDetector.kt)
 
+!!! note "Configuration"
+By default, this rule will only check for modifiers in public methods. However, you can configure the threshold via using `visibility-threshold` option in `lint.xml`.
+
+    ```xml
+    <issue id="ComposeModifierMissing">
+       <option name="visibility-threshold" value="only_public" />
+    </issue>
+    ```
+
+Possible values are:
+* `only_public`: (default) Will check for missing modifiers only for public composables.
+* `public_and_internal`: Will check for missing modifiers in both public and internal composables.
+* `all`: Will check for missing modifiers in all composables.
+
 ### Don't re-use modifiers
 
 Modifiers which are passed in are designed so that they should be used by a single layout node in the composable function. If the provided modifier is used by multiple composables at different levels, unwanted behaviour can happen.