Fix duplicate emissions for multicontent (#411)

CHANGELOG.md

@@ -1,6 +1,12 @@
 Changelog
 =========
 
+**Unreleased**
+--------------
+
+- **Fix**: Don't report duplicate errors about multiple content emitters.
+- **Enhancement**: Report the function name for readability in `ComposeContentEmitterReturningValues`.
+
 1.3.1
 -----
 

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

@@ -23,6 +23,7 @@ import slack.lint.compose.util.emitsContent
 import slack.lint.compose.util.findChildrenByClass
 import slack.lint.compose.util.hasReceiverType
 import slack.lint.compose.util.isComposable
+import slack.lint.compose.util.returnsUnitOrVoid
 import slack.lint.compose.util.sourceImplementation
 import slack.lint.compose.util.unwrapParenthesis
 
@@ -43,8 +44,8 @@ constructor(
           briefDescription = "Composable functions should emit XOR return",
           explanation =
             """
-            Composable functions should either emit content into the composition or return a value, but not both.\
-            If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.\
+            Composable functions should either emit content into the composition or return a value, but not both. \
+            If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller. \
             See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information.
         """,
           category = Category.PRODUCTIVITY,
@@ -99,26 +100,32 @@ constructor(
         val composables =
           file
             .findChildrenByClass<KtFunction>()
-            .filter { it.toUElementOfType<UMethod>()?.isComposable == true }
+            .filter {
+              val method = it.toUElementOfType<UMethod>() ?: return@filter false
+              method.isComposable && !method.returnsUnitOrVoid(context.evaluator)
+            }
             // We don't want to analyze composables that are extension functions, as they might be
             // things like
             // BoxScope which are legit, and we want to avoid false positives.
-            .filter { it.hasBlockBody() }
             // We want only methods with a body
+            .filter { it.hasBlockBody() }
             .filterNot { it.hasReceiverType }
+            .toList()
+
+        if (composables.isEmpty()) return
 
         // Now we want to get the count of direct emitters in them: the composables we know for a
         // fact that output UI
         val composableToEmissionCount = composables.associateWith { it.directUiEmitterCount }
 
-        // We can start showing errors, for composables that emit more than once (from the list of
+        // We can start showing errors, for composables that emit at all (from the list of
         // known composables)
-        val directEmissionsReported = composableToEmissionCount.filterValues { it > 1 }.keys
+        val directEmissionsReported = composableToEmissionCount.keys
         for (composable in directEmissionsReported) {
           context.report(
             ISSUE,
             composable,
-            context.getLocation(composable),
+            context.getNameLocation(composable),
             ISSUE.getExplanation(TextFormat.TEXT),
           )
         }
@@ -156,7 +163,7 @@ constructor(
             context.report(
               ISSUE,
               composable,
-              context.getLocation(composable),
+              context.getNameLocation(composable),
               ISSUE.getExplanation(TextFormat.TEXT),
             )
           }

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

@@ -81,6 +81,20 @@ abstract class BaseComposeLintTest : LintDetectorTest() {
       """
           .trimIndent()
       ),
+      kotlin(
+        """
+          package androidx.compose.ui
+
+          import androidx.compose.runtime.Composable
+          import androidx.compose.ui.Modifier
+
+          @Composable
+          fun Text(text: String, modifier: Modifier = Modifier) {
+
+          }
+      """
+          .trimIndent()
+      ),
     )
 
   /**

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

@@ -28,6 +28,8 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
     @Language("kotlin")
     val code =
       """
+        import androidx.compose.runtime.Composable
+
         @Composable
         fun Something() {
             val something = rememberWhatever()
@@ -48,6 +50,8 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
     @Language("kotlin")
     val code =
       """
+        import androidx.compose.runtime.Composable
+
         @Composable
         fun ColumnScope.Something() {
             Text("Hi")
@@ -63,55 +67,20 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
     lint().files(kotlin(code)).allowCompilationErrors().run().expectClean()
   }
 
-  @Test
-  fun `errors when a Composable function has more than one UI emitter at the top level`() {
-    @Language("kotlin")
-    val code =
-      """
-        import androidx.compose.runtime.Composable
-
-        @Composable
-        fun Something() {
-            Text("Hi")
-            Text("Hola")
-        }
-        @Composable
-        fun Something() {
-            Spacer16()
-            Text("Hola")
-        }
-      """
-        .trimIndent()
-    lint()
-      .files(*commonStubs, kotlin(code))
-      .run()
-      .expect(
-        """
-          src/test.kt:3: Error: Composable functions should either emit content into the composition or return a value, but not both.If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
-          @Composable
-          ^
-          src/test.kt:8: Error: Composable functions should either emit content into the composition or return a value, but not both.If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
-          @Composable
-          ^
-          2 errors, 0 warnings
-        """
-          .trimIndent()
-      )
-  }
-
   @Test
   fun `errors when a Composable function has more than one indirect UI emitter at the top level`() {
     @Language("kotlin")
     val code =
       """
         import androidx.compose.runtime.Composable
+        import androidx.compose.ui.Text
 
         @Composable
         fun Something1() {
             Something2()
         }
         @Composable
-        fun Something2() {
+        fun Something2(): String {
             Text("Hola")
             Something3()
         }
@@ -124,23 +93,31 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
             Banana()
         }
         @Composable
-        fun Something5() {
+        fun Something5(): String {
             Something3()
             Something4()
         }
+        @Composable
+        fun Potato() {
+            Text("Potato")
+        }
+        @Composable
+        fun Banana() {
+            Text("Banana")
+        }
       """
         .trimIndent()
     lint()
       .files(*commonStubs, kotlin(code))
       .run()
       .expect(
         """
-          src/test.kt:7: Error: Composable functions should either emit content into the composition or return a value, but not both.If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
-          @Composable
-          ^
-          src/test.kt:20: Error: Composable functions should either emit content into the composition or return a value, but not both.If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
-          @Composable
-          ^
+          src/test.kt:9: Error: Composable functions should either emit content into the composition or return a value, but not both. If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller. See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
+          fun Something2(): String {
+              ~~~~~~~~~~
+          src/test.kt:22: Error: Composable functions should either emit content into the composition or return a value, but not both. If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller. See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
+          fun Something5(): String {
+              ~~~~~~~~~~
           2 errors, 0 warnings
         """
           .trimIndent()
@@ -153,12 +130,14 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
     val code =
       """
         import androidx.compose.runtime.Composable
+        import androidx.compose.ui.Text
 
         @Composable
-        fun Something() {
+        fun Something(): String {
             Text("Hi")
             Text("Hola")
             Something2()
+            return "hi"
         }
         @Composable
         fun Something2() {
@@ -171,12 +150,30 @@ class ContentEmitterReturningValuesDetectorTest : BaseComposeLintTest() {
       .run()
       .expect(
         """
-          src/test.kt:3: Error: Composable functions should either emit content into the composition or return a value, but not both.If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller.See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
-          @Composable
-          ^
+          src/test.kt:5: Error: Composable functions should either emit content into the composition or return a value, but not both. If a composable should offer additional control surfaces to its caller, those control surfaces or callbacks should be provided as parameters to the composable function by the caller. See https://slackhq.github.io/compose-lints/rules/#do-not-emit-content-and-return-a-result for more information. [ComposeContentEmitterReturningValues]
+          fun Something(): String {
+              ~~~~~~~~~
           1 errors, 0 warnings
         """
           .trimIndent()
       )
   }
+
+  // /~https://github.com/slackhq/compose-lints/issues/339
+  @Test
+  fun `multiple emitters are not a warning in this lint`() {
+    @Language("kotlin")
+    val code =
+      """
+        import androidx.compose.runtime.Composable
+
+        @Composable
+        fun Test(modifier: Modifier = Modifier) {
+            Text(text = "TextOne")
+            Text(text = "TextTwo")
+        }
+      """
+        .trimIndent()
+    lint().files(*commonStubs, kotlin(code)).run().expectClean()
+  }
 }