Flow Delegate usage detector

David Liu
Commit 7ca8b272da872e0c2881a5e36573d392487772b0 7ca8b272 1 parent 89750fc0
livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt
livekit-android-sdk/src/main/java/io/livekit/android/room/participant/Participant.kt
livekit-android-sdk/src/main/java/io/livekit/android/room/track/TrackPublication.kt
livekit-android-sdk/src/main/java/io/livekit/android/util/MutableStateFlowDelegate.kt → livekit-android-sdk/src/main/java/io/livekit/android/util/FlowDelegate.kt
livekit-lint/src/main/java/io/livekit/lint/FlowDelegateUsageDetector.kt
livekit-lint/src/main/java/io/livekit/lint/IssueRegistry.kt
livekit-lint/src/main/java/io/livekit/lint/MediaTrackEqualsDetector.kt
livekit-lint/src/test/java/io/livekit/lint/FlowDelegateUsageDetectorTest.kt
--- a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt
查看文件 @7ca8b27
+++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt
查看文件 @7ca8b27
@@ -16,6 +16,7 @@ import io.livekit.android.events.RoomEvent
 import io.livekit.android.renderer.TextureViewRenderer
 import io.livekit.android.room.participant.*
 import io.livekit.android.room.track.*
+import io.livekit.android.util.FlowObservable
 import io.livekit.android.util.LKLog
 import io.livekit.android.util.flow
 import io.livekit.android.util.flowDelegate
@@ -60,23 +61,18 @@ constructor(
     @Deprecated("Use events instead.")
     var listener: RoomListener? = null
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+    @get:FlowObservable
     var sid: Sid? by flowDelegate(null)
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+
+    @get:FlowObservable
     var name: String? by flowDelegate(null)
         private set
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+
+    @get:FlowObservable
     var state: State by flowDelegate(State.DISCONNECTED)
         private set
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+
+    @get:FlowObservable
     var metadata: String? by flowDelegate(null)
         private set
@@ -88,17 +84,16 @@ constructor(
     lateinit var localParticipant: LocalParticipant
         private set
+
     private var mutableRemoteParticipants by flowDelegate(emptyMap<String, RemoteParticipant>())
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+
+    @get:FlowObservable
     val remoteParticipants: Map<String, RemoteParticipant>
         get() = mutableRemoteParticipants
     private var mutableActiveSpeakers by flowDelegate(emptyList<Participant>())
-    /**
-     * Changes can be observed by using [io.livekit.android.util.flow]
-     */
+
+    @get:FlowObservable
     val activeSpeakers: List<Participant>
         get() = mutableActiveSpeakers
@@ -109,7 +104,6 @@ constructor(
         }
         coroutineScope = CoroutineScope(defaultDispatcher + SupervisorJob())
         state = State.CONNECTING
-        ::state.flow
         val response = engine.join(url, token, options)
         LKLog.i { "Connected to server, server version: ${response.serverVersion}, client version: ${Version.CLIENT_VERSION}" }
--- a/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/Participant.kt
查看文件 @7ca8b27
+++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/Participant.kt
查看文件 @7ca8b27
@@ -7,6 +7,7 @@ import io.livekit.android.room.track.LocalTrackPublication
 import io.livekit.android.room.track.RemoteTrackPublication
 import io.livekit.android.room.track.Track
 import io.livekit.android.room.track.TrackPublication
+import io.livekit.android.util.FlowObservable
 import io.livekit.android.util.flow
 import io.livekit.android.util.flowDelegate
 import kotlinx.coroutines.CoroutineDispatcher
@@ -32,23 +33,27 @@ open class Participant(
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var participantInfo: LivekitModels.ParticipantInfo? by flowDelegate(null)
         private set
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var identity: String? by flowDelegate(identity)
         internal set
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var audioLevel: Float by flowDelegate(0f)
         internal set
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var isSpeaking: Boolean by flowDelegate(false) { newValue, oldValue ->
         if (newValue != oldValue) {
             listener?.onSpeakingChanged(this)
@@ -60,6 +65,7 @@ open class Participant(
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var metadata: String? by flowDelegate(null) { newMetadata, oldMetadata ->
         if (newMetadata != oldMetadata) {
             listener?.onMetadataChanged(this, oldMetadata)
@@ -72,6 +78,7 @@ open class Participant(
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var connectionQuality by flowDelegate(ConnectionQuality.UNKNOWN)
         internal set
@@ -93,11 +100,13 @@ open class Participant(
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     var tracks by flowDelegate(emptyMap<String, TrackPublication>())
         protected set
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     val audioTracks by flowDelegate(
         stateFlow = ::tracks.flow
             .map { it.filterValues { publication -> publication.kind == Track.Kind.AUDIO } }
@@ -106,6 +115,7 @@ open class Participant(
     /**
      * Changes can be observed by using [io.livekit.android.util.flow]
      */
+    @get:FlowObservable
     val videoTracks by flowDelegate(
         stateFlow = ::tracks.flow
             .map {
--- a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/TrackPublication.kt
查看文件 @7ca8b27
+++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/TrackPublication.kt
查看文件 @7ca8b27
 package io.livekit.android.room.track
 import io.livekit.android.room.participant.Participant
+import io.livekit.android.util.FlowObservable
 import io.livekit.android.util.flowDelegate
 import livekit.LivekitModels
 import java.lang.ref.WeakReference
@@ -10,6 +11,7 @@ open class TrackPublication(
     track: Track?,
     participant: Participant
 ) {
+    @get:FlowObservable
     open var track: Track? by flowDelegate(track)
         internal set
     var name: String
--- a/livekit-android-sdk/src/main/java/io/livekit/android/util/MutableStateFlowDelegate.kt → livekit-android-sdk/src/main/java/io/livekit/android/util/FlowDelegate.kt
查看文件 @7ca8b27
+++ b/livekit-android-sdk/src/main/java/io/livekit/android/util/MutableStateFlowDelegate.kt → livekit-android-sdk/src/main/java/io/livekit/android/util/FlowDelegate.kt
查看文件 @7ca8b27
@@ -62,7 +62,16 @@ internal val <T> KProperty0<T>.delegate: Any?
 val <T> KProperty0<T>.flow: StateFlow<T>
     get() = delegate as StateFlow<T>
-class MutableStateFlowDelegate<T>
+/**
+ * Indicates that the target property changes can be observed with [flow].
+ */
+@Target(AnnotationTarget.CLASS, AnnotationTarget.PROPERTY_GETTER)
+@Retention(AnnotationRetention.BINARY)
+@MustBeDocumented
+annotation class FlowObservable
+
+@FlowObservable
+internal class MutableStateFlowDelegate<T>
 internal constructor(
     private val flow: MutableStateFlow<T>,
     private val onSetValue: ((newValue: T, oldValue: T) -> Unit)? = null
@@ -82,7 +91,8 @@ internal constructor(
     }
 }
-class StateFlowDelegate<T>
+@FlowObservable
+internal class StateFlowDelegate<T>
 internal constructor(
     private val flow: StateFlow<T>
 ) : StateFlow<T> by flow {
--- a/livekit-lint/src/main/java/io/livekit/lint/FlowDelegateUsageDetector.kt
查看文件 @7ca8b27
+++ b/livekit-lint/src/main/java/io/livekit/lint/FlowDelegateUsageDetector.kt
查看文件 @7ca8b27
@@ -19,11 +19,14 @@
 package io.livekit.lint
 import com.android.tools.lint.detector.api.*
+import com.intellij.psi.PsiClassType
 import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiField
 import com.intellij.psi.PsiMethod
 import org.jetbrains.uast.UCallableReferenceExpression
 import org.jetbrains.uast.UReferenceExpression
 import org.jetbrains.uast.kotlin.KotlinUQualifiedReferenceExpression
+import org.jetbrains.uast.tryResolve
 /** Checks related to DiffUtil computation. */
 class FlowDelegateUsageDetector : Detector(), SourceCodeScanner {
@@ -32,20 +35,33 @@ class FlowDelegateUsageDetector : Detector(), SourceCodeScanner {
         // Check if we're actually trying to access the flow delegate
         val referencedMethod = referenced as? PsiMethod ?: return
-        if (referenced.name != "getFlow" || referencedMethod.containingClass?.qualifiedName != "io.livekit.android.util.FlowObservableKt") {
+        if (referenced.name != GET_FLOW || referencedMethod.containingClass?.qualifiedName != FLOW_DELEGATE) {
             return
         }
-        // This should get the property we're trying to receive the flow from.
+        // This should get the getter we're trying to receive the flow from.
+        val parent = reference.uastParent as? KotlinUQualifiedReferenceExpression
+        val rec = parent?.receiver
+        val resolve = rec?.tryResolve()
+        println("parent: $parent, rec: $rec, resolve: $resolve")
         val receiver = ((reference.uastParent as? KotlinUQualifiedReferenceExpression)
             ?.receiver as? UCallableReferenceExpression)
-            ?: return
+            ?.resolve()
-        // This should get the original class associated with the property.
-        val className = receiver.qualifierType?.canonicalText
-        val psiClass = if (className != null) context.evaluator.findClass(className) else null
-        val psiField = psiClass?.findFieldByName("${receiver.callableName}\$delegate", true)
-        val isAnnotated = psiField?.hasAnnotation("io.livekit.android.util.FlowObservable") ?: false
+        val isAnnotated = when (receiver) {
+            is PsiMethod -> {
+                println("${receiver.name},  ${receiver.annotations.fold("") { total, next -> "$total, ${next.text}" }}")
+                receiver.hasAnnotation(FLOW_OBSERVABLE_ANNOTATION)
+            }
+            is PsiField -> {
+                val receiverClass = (receiver.type as? PsiClassType)?.resolve()
+                println("${receiverClass},  ${receiverClass?.annotations?.fold("") { total, next -> "$total, ${next.text}" }}")
+                receiverClass?.hasAnnotation(FLOW_OBSERVABLE_ANNOTATION) ?: false
+            }
+            else -> {
+                false
+            }
+        }
         if (!isAnnotated) {
             val message = DEFAULT_MSG
@@ -60,8 +76,16 @@ class FlowDelegateUsageDetector : Detector(), SourceCodeScanner {
     companion object {
+        // The name of the method for the flow accessor
+        private const val GET_FLOW = "getFlow"
+
+        // The containing file and implicitly generated class
+        private const val FLOW_DELEGATE = "io.livekit.android.util.FlowDelegateKt"
+
+        private const val FLOW_OBSERVABLE_ANNOTATION = "io.livekit.android.util.FlowObservable"
+
         private const val DEFAULT_MSG =
-            "Incorrect flow property usage: Only properties marked with the @FlowObservable annotation can be observed using `io.livekit.android.util.flow`."
+            "Incorrect flow property usage: Only properties marked with the @FlowObservable annotation can be observed using `io.livekit.android.util.flow`. Improper usage will result in a NullPointerException."
         private val IMPLEMENTATION =
             Implementation(FlowDelegateUsageDetector::class.java, Scope.JAVA_FILE_SCOPE)
--- a/livekit-lint/src/main/java/io/livekit/lint/IssueRegistry.kt
查看文件 @7ca8b27
+++ b/livekit-lint/src/main/java/io/livekit/lint/IssueRegistry.kt
查看文件 @7ca8b27
 package io.livekit.lint
 import com.android.tools.lint.client.api.IssueRegistry
+import com.android.tools.lint.client.api.Vendor
 import com.android.tools.lint.detector.api.CURRENT_API
 import com.android.tools.lint.detector.api.Issue
 import com.google.auto.service.AutoService
@@ -11,6 +12,12 @@ class IssueRegistry : IssueRegistry() {
     override val api: Int = CURRENT_API
+    override val vendor: Vendor = Vendor(
+        vendorName = "LiveKit",
+        identifier = "io.livekit.android",
+        feedbackUrl = "https://github.com/livekit/client-sdk-android",
+    )
+
     override val issues: List<Issue>
-        get() = listOf(MediaTrackEqualsDetector.ISSUE)
+        get() = listOf(MediaTrackEqualsDetector.ISSUE, FlowDelegateUsageDetector.ISSUE)
 }
--- a/livekit-lint/src/main/java/io/livekit/lint/MediaTrackEqualsDetector.kt
查看文件 @7ca8b27
+++ b/livekit-lint/src/main/java/io/livekit/lint/MediaTrackEqualsDetector.kt
查看文件 @7ca8b27
 @file:Suppress("UnstableApiUsage") // We know that Lint API's aren't final.
-
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package io.livekit.lint
 import com.android.tools.lint.client.api.UElementHandler
--- a/livekit-lint/src/test/java/io/livekit/lint/FlowDelegateUsageDetectorTest.kt
查看文件 @7ca8b27
+++ b/livekit-lint/src/test/java/io/livekit/lint/FlowDelegateUsageDetectorTest.kt
查看文件 @7ca8b27
@@ -14,6 +14,7 @@ class FlowDelegateUsageDetectorTest {
             .allowMissingSdk()
             .files(
                 flowAccess(),
+                stateFlow(),
                 kotlin(
                     """
                     package foo
@@ -21,7 +22,7 @@ class FlowDelegateUsageDetectorTest {
                     import io.livekit.android.util.flow
                     import io.livekit.android.util.flowDelegate
                     class Example {
-                        @field:FlowObservable
+                        @get:FlowObservable
                         val value: Int by flowDelegate(0)
                         fun foo() {
                             ::value.flow
@@ -36,11 +37,12 @@ class FlowDelegateUsageDetectorTest {
     }
     @Test
-    fun nonAnnotatedFlowAccess() {
+    fun thisColonAccess() {
         lint()
             .allowMissingSdk()
             .files(
                 flowAccess(),
+                stateFlow(),
                 kotlin(
                     """
                     package foo
@@ -48,6 +50,7 @@ class FlowDelegateUsageDetectorTest {
                     import io.livekit.android.util.flow
                     import io.livekit.android.util.flowDelegate
                     class Example {
+                        @get:FlowObservable
                         val value: Int by flowDelegate(0)
                         fun foo() {
                             this::value.flow
@@ -58,18 +61,143 @@ class FlowDelegateUsageDetectorTest {
             )
             .issues(FlowDelegateUsageDetector.ISSUE)
             .run()
+            .expectClean()
+    }
+
+    @Test
+    fun otherClassAccess() {
+        lint()
+            .allowMissingSdk()
+            .files(
+                flowAccess(),
+                stateFlow(),
+                kotlin(
+                    """
+                    package foo
+                    import io.livekit.android.util.FlowObservable
+                    import io.livekit.android.util.flow
+                    import io.livekit.android.util.flowDelegate
+                    class FlowContainer {
+                        @get:FlowObservable
+                        val value: Int by flowDelegate(0)
+                    }
+                    class Example {
+                        fun foo() {
+                            val container = FlowContainer()
+                            container::value.flow
+                            return
+                        }
+                    }"""
+                ).indented()
+            )
+            .issues(FlowDelegateUsageDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun parenthesesClassAccess() {
+        lint()
+            .allowMissingSdk()
+            .files(
+                flowAccess(),
+                stateFlow(),
+                kotlin(
+                    """
+                    package foo
+                    import io.livekit.android.util.FlowObservable
+                    import io.livekit.android.util.flow
+                    import io.livekit.android.util.flowDelegate
+                    class FlowContainer {
+                        @get:FlowObservable
+                        val value: Int by flowDelegate(0)
+                    }
+                    class Example {
+                        fun foo() {
+                            val container = FlowContainer()
+                            (container)::value.flow
+                            return
+                        }
+                    }"""
+                ).indented()
+            )
+            .issues(FlowDelegateUsageDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun roundaboutAccess() {
+        lint()
+            .allowMissingSdk()
+            .files(
+                flowAccess(),
+                stateFlow(),
+                kotlin(
+                    """
+                    package foo
+                    import io.livekit.android.util.FlowObservable
+                    import io.livekit.android.util.flow
+                    import io.livekit.android.util.flowDelegate
+                    class FlowContainer {
+                        var value: Int by flowDelegate(0)
+                        @get:FlowObservable
+                        val otherValue: Int
+                            get() = value
+                    }
+                    class Example {
+                        fun foo() {
+                            val container = FlowContainer()
+                            container::otherValue.flow
+                            return
+                        }
+                    }"""
+                ).indented()
+            )
+            .issues(FlowDelegateUsageDetector.ISSUE)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun nonAnnotatedFlowAccess() {
+        lint()
+            .allowMissingSdk()
+            .files(
+                flowAccess(),
+                stateFlow(),
+                kotlin(
+                    """
+                    package foo
+                    import io.livekit.android.util.FlowObservable
+                    import io.livekit.android.util.flow
+                    import io.livekit.android.util.flowDelegate
+                    class Example {
+                        val value: Int = 0
+                        fun foo() {
+                            this::value.flow
+                            return
+                        }
+                    }"""
+                ).indented()
+            )
+            .issues(FlowDelegateUsageDetector.ISSUE)
+            .run()
             .expectErrorCount(1)
     }
 }
 fun flowAccess(): TestFile {
     return kotlin(
+        "io/livekit/android/util/FlowDelegate.kt",
         """
         package io.livekit.android.util
         import kotlin.reflect.KProperty
         import kotlin.reflect.KProperty0
-        
+        import kotlinx.coroutines.flow.StateFlow
+        import kotlinx.coroutines.flow.MutableStateFlow
+
         internal val <T> KProperty0<T>.delegate: Any?
             get() { getDelegate() }
@@ -77,11 +205,12 @@ fun flowAccess(): TestFile {
         val <T> KProperty0<T>.flow: StateFlow<T>
             get() = delegate as StateFlow<T>
-        @Target(AnnotationTarget.PROPERTY)
+        @Target(AnnotationTarget.PROPERTY_GETTER)
         @Retention(AnnotationRetention.SOURCE)
         @MustBeDocumented
         annotation class FlowObservable
+        @FlowObservable
         class MutableStateFlowDelegate<T>
         internal constructor(
             private val flow: MutableStateFlow<T>,
@@ -105,7 +234,16 @@ fun flowAccess(): TestFile {
         ): MutableStateFlowDelegate<T> {
             return MutableStateFlowDelegate(MutableStateFlow(initialValue), onSetValue)
         }
+    """
+    ).indented()
+        .within("src")
+}
+fun stateFlow(): TestFile {
+
+    return kotlin(
+        """
+        package kotlinx.coroutines.flow
         interface StateFlow<out T> {
             val value: T
         }