Get rid of suppressions for "invisible_reference" and "invisible_member"

For accessing internal declarations from other modules (e.g. access `rib-base` internals from `rib-test`), we are currently suppressing compiler errors with `@file:Suppress("invisible_reference", "invisible_member")` A better approach is to: 1. Create an `internal` opt-in annotation. 2. Make the "accessible to friend modules" component `public` 3. Mark the (now public) component with the opt-in annotation. 4. Opt-in to the annotation from `build.gradle`. Because the new annotation is `internal`, it cannot be normally accessed from external modules. But Gradle can see it if it's part of the source set. This makes the internal visibility of those friend-modules APIs even stricter, since consumers now cannot just suppress `"invisible_reference"` and `"invisible_member"` to directly access the friend-module API. Plus, we get rid of the hacky suppressions in our codebase.
uber · Sep 22, 2023 · adc3b0d · adc3b0d
1 parent 0043217
commit adc3b0d
Show file tree

Hide file tree

Showing 27 changed files with 202 additions and 48 deletions.
diff --git a/android/libraries/rib-android/build.gradle → ...id/libraries/rib-android/build.gradle.kts b/android/libraries/rib-android/build.gradle → ...id/libraries/rib-android/build.gradle.kts
@@ -19,7 +19,17 @@ plugins {
 }
 
 android {
-    namespace "com.uber.rib.android"
+    namespace = "com.uber.rib.android"
+}
+
+kotlin {
+    sourceSets {
+        configureEach {
+            languageSettings {
+                optIn("com.uber.rib.core.internal.CoreFriendModuleApi")
+            }
+        }
+    }
 }
 
 dependencies {

diff --git a/android/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt b/android/libraries/rib-android/src/main/kotlin/com/uber/rib/core/RibActivity.kt
@@ -13,8 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@file:Suppress("invisible_reference", "invisible_member")
-
 package com.uber.rib.core
 
 import android.content.Intent
@@ -45,7 +43,7 @@ import kotlinx.coroutines.flow.SharedFlow
 import kotlinx.coroutines.rx2.asObservable
 
 /** Base implementation for all VIP [android.app.Activity]s. */
-abstract class RibActivity :
+public abstract class RibActivity :
   CoreAppCompatActivity(),
   ActivityStarter,
   LifecycleScopeProvider<ActivityLifecycleEvent>,
@@ -55,7 +53,7 @@ abstract class RibActivity :
   private val _lifecycleFlow =
     MutableSharedFlow<ActivityLifecycleEvent>(1, 0, BufferOverflow.DROP_OLDEST)
 
-  open val lifecycleFlow: SharedFlow<ActivityLifecycleEvent>
+  public open val lifecycleFlow: SharedFlow<ActivityLifecycleEvent>
     get() = _lifecycleFlow
 
   @Volatile private var _lifecycleObservable: Observable<ActivityLifecycleEvent>? = null
@@ -64,7 +62,7 @@ abstract class RibActivity :
 
   private val _callbacksFlow =
     MutableSharedFlow<ActivityCallbackEvent>(0, 1, BufferOverflow.DROP_OLDEST)
-  open val callbacksFlow: SharedFlow<ActivityCallbackEvent>
+  public open val callbacksFlow: SharedFlow<ActivityCallbackEvent>
     get() = _callbacksFlow
 
   @Volatile private var _callbacksObservable: Observable<ActivityCallbackEvent>? = null
@@ -215,7 +213,7 @@ abstract class RibActivity :
    * @return the [Interactor] when the activity has alive.
    * @throws IllegalStateException if the activity has not been created or has been destroyed.
    */
-  open val interactor: Interactor<*, *>
+  public open val interactor: Interactor<*, *>
     get() =
       if (router != null) {
         router?.interactor as Interactor<*, *>
@@ -232,7 +230,7 @@ abstract class RibActivity :
    */
   protected abstract fun createRouter(parentViewGroup: ViewGroup): ViewRouter<*, *>
 
-  companion object {
+  public companion object {
     /**
      * Figures out which corresponding next lifecycle event in which to unsubscribe, for Activities.
      */

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/FlowAsScope.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/FlowAsScope.kt
@@ -19,6 +19,7 @@ package com.uber.rib.core
 
 import com.uber.autodispose.lifecycle.LifecycleEndedException
 import com.uber.autodispose.lifecycle.LifecycleNotStartedException
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import io.reactivex.CompletableSource
 import kotlin.coroutines.CoroutineContext
 import kotlin.coroutines.EmptyCoroutineContext
@@ -36,7 +37,8 @@ import kotlinx.coroutines.rx2.rxCompletable
  * 2. [LifecycleEndedException], if the last emitted event is in the end (inclusive) or beyond
  *    [range].
  */
-internal fun <T : Comparable<T>> SharedFlow<T>.asScopeCompletable(
+@CoreFriendModuleApi
+public fun <T : Comparable<T>> SharedFlow<T>.asScopeCompletable(
   range: ClosedRange<T>,
   context: CoroutineContext = EmptyCoroutineContext,
 ): CompletableSource {

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Interactor.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Interactor.kt
@@ -20,6 +20,7 @@ import androidx.annotation.VisibleForTesting
 import com.uber.autodispose.lifecycle.CorrespondingEventsFunction
 import com.uber.autodispose.lifecycle.LifecycleEndedException
 import com.uber.rib.core.RibEvents.triggerRibActionAndEmitEvents
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import com.uber.rib.core.lifecycle.InteractorEvent
 import io.reactivex.CompletableSource
 import io.reactivex.Observable
@@ -39,12 +40,15 @@ import kotlinx.coroutines.rx2.asObservable
  */
 public abstract class Interactor<P : Any, R : Router<*>>() : InteractorType, RibActionEmitter {
   @Inject public lateinit var injectedPresenter: P
-  internal var actualPresenter: P? = null
+
+  @CoreFriendModuleApi public var actualPresenter: P? = null
   private val _lifecycleFlow = MutableSharedFlow<InteractorEvent>(1, 0, BufferOverflow.DROP_OLDEST)
   public open val lifecycleFlow: SharedFlow<InteractorEvent>
     get() = _lifecycleFlow
 
   @Volatile private var _lifecycleObservable: Observable<InteractorEvent>? = null
+
+  @OptIn(CoreFriendModuleApi::class)
   private val lifecycleObservable
     get() = ::_lifecycleObservable.setIfNullAndGet { lifecycleFlow.asObservable() }
 
@@ -54,6 +58,7 @@ public abstract class Interactor<P : Any, R : Router<*>>() : InteractorType, Rib
   public open var router: R by routerDelegate
     protected set
 
+  @OptIn(CoreFriendModuleApi::class)
   protected constructor(presenter: P) : this() {
     this.actualPresenter = presenter
   }
@@ -67,6 +72,7 @@ public abstract class Interactor<P : Any, R : Router<*>>() : InteractorType, Rib
 
   final override fun peekLifecycle(): InteractorEvent? = lifecycleFlow.replayCache.lastOrNull()
 
+  @OptIn(CoreFriendModuleApi::class)
   final override fun requestScope(): CompletableSource =
     lifecycleFlow.asScopeCompletable(lifecycleRange)
 
@@ -149,13 +155,15 @@ public abstract class Interactor<P : Any, R : Router<*>>() : InteractorType, Rib
     return getPresenter()
   }
 
-  internal fun setRouterInternal(router: Router<*>) {
+  @CoreFriendModuleApi
+  public fun setRouterInternal(router: Router<*>) {
     if (routerDelegate != null) {
       this.router = router as R
     }
   }
 
   /** @return the currently attached presenter if there is one */
+  @OptIn(CoreFriendModuleApi::class)
   @VisibleForTesting
   private fun getPresenter(): P {
     val presenter: P? =
@@ -172,6 +180,7 @@ public abstract class Interactor<P : Any, R : Router<*>>() : InteractorType, Rib
     return presenter
   }
 
+  @OptIn(CoreFriendModuleApi::class)
   @VisibleForTesting
   internal fun setPresenter(presenter: P) {
     actualPresenter = presenter

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/LazyBackingProperty.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/LazyBackingProperty.kt
@@ -15,6 +15,7 @@
  */
 package com.uber.rib.core
 
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import kotlin.reflect.KMutableProperty0
 
 /**
@@ -30,8 +31,9 @@ import kotlin.reflect.KMutableProperty0
  *
  * To properly support concurrency, the backing mutable property should be [Volatile].
  */
-internal inline fun <T : Any> KMutableProperty0<T?>.setIfNullAndGet(
+@CoreFriendModuleApi
+public inline fun <T : Any> KMutableProperty0<T?>.setIfNullAndGet(
   initializer: () -> T,
 ): T = get() ?: synchronized(Lock) { get() ?: initializer().also { set(it) } }
 
-private object Lock
+@CoreFriendModuleApi public object Lock
diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Presenter.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Presenter.kt
@@ -17,6 +17,7 @@ package com.uber.rib.core
 
 import androidx.annotation.CallSuper
 import com.uber.autodispose.ScopeProvider
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import com.uber.rib.core.lifecycle.PresenterEvent
 import io.reactivex.CompletableSource
 import io.reactivex.Observable
@@ -39,6 +40,8 @@ public abstract class Presenter : ScopeProvider, RibActionEmitter {
     get() = _lifecycleFlow
 
   @Volatile private var _lifecycleObservable: Observable<PresenterEvent>? = null
+
+  @OptIn(CoreFriendModuleApi::class)
   private val lifecycleObservable
     get() = ::_lifecycleObservable.setIfNullAndGet { lifecycleFlow.asObservable() }
 
@@ -70,6 +73,7 @@ public abstract class Presenter : ScopeProvider, RibActionEmitter {
   /** @return an observable of this controller's lifecycle events. */
   public fun lifecycle(): Observable<PresenterEvent> = lifecycleObservable
 
+  @OptIn(CoreFriendModuleApi::class)
   final override fun requestScope(): CompletableSource =
     lifecycleFlow.asScopeCompletable(lifecycleRange)
 

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Router.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/Router.kt
@@ -20,6 +20,7 @@ import androidx.annotation.CallSuper
 import androidx.annotation.MainThread
 import androidx.annotation.VisibleForTesting
 import com.uber.rib.core.RibEvents.triggerRibActionAndEmitEvents
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import java.util.Locale
 import java.util.concurrent.CopyOnWriteArrayList
 
@@ -40,7 +41,8 @@ protected constructor(
     get() = interactor as Interactor<*, *>
 
   /** @return the Tag. */
-  internal var tag: String? = null
+  @CoreFriendModuleApi
+  public var tag: String? = null
     private set
   private var savedInstanceState: Bundle? = null
   private var isLoaded = false
@@ -58,6 +60,7 @@ protected constructor(
     (component as? InteractorBaseComponent<Interactor<*, *>>)?.inject(interactorGeneric)
   }
 
+  @OptIn(CoreFriendModuleApi::class)
   protected open fun attachToInteractor() {
     interactorGeneric.setRouterInternal(this)
   }
@@ -99,6 +102,7 @@ protected constructor(
    * @param childRouter the [Router] to be attached.
    * @param tag an identifier to namespace saved instance state [Bundle] objects.
    */
+  @OptIn(CoreFriendModuleApi::class)
   @MainThread
   public open fun attachChild(childRouter: Router<*>, tag: String) {
     for (child in children) {
@@ -146,6 +150,7 @@ protected constructor(
    *
    * @param childRouter the [Router] to be detached.
    */
+  @OptIn(CoreFriendModuleApi::class)
   @MainThread
   public open fun detachChild(childRouter: Router<*>) {
     val isChildRemoved = children.remove(childRouter)
@@ -185,6 +190,7 @@ protected constructor(
     dispatchAttach(savedInstanceState, javaClass.name)
   }
 
+  @OptIn(CoreFriendModuleApi::class)
   @CallSuper
   @Initializer
   public open fun dispatchAttach(savedInstanceState: Bundle?, tag: String) {
@@ -218,14 +224,16 @@ protected constructor(
    *
    * @return Children.
    */
-  internal open fun getChildren(): List<Router<*>> {
+  @CoreFriendModuleApi
+  public open fun getChildren(): List<Router<*>> {
     return children
   }
 
   internal fun saveInstanceStateInternal(outState: Bundle) {
     saveInstanceState(outState)
   }
 
+  @OptIn(CoreFriendModuleApi::class)
   protected open fun saveInstanceState(outState: Bundle) {
     val interactorSavedInstanceState = Bundle()
     interactorGeneric.onSaveInstanceStateInternal(interactorSavedInstanceState)

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/WorkerBinder.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/WorkerBinder.kt
@@ -19,6 +19,7 @@ import androidx.annotation.VisibleForTesting
 import com.uber.autodispose.ScopeProvider
 import com.uber.autodispose.lifecycle.LifecycleScopeProvider
 import com.uber.rib.core.RibEvents.triggerRibActionAndEmitEvents
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import com.uber.rib.core.lifecycle.InteractorEvent
 import com.uber.rib.core.lifecycle.PresenterEvent
 import com.uber.rib.core.lifecycle.WorkerEvent
@@ -157,6 +158,7 @@ public object WorkerBinder {
     }
   }
 
+  @OptIn(CoreFriendModuleApi::class)
   @JvmStatic
   @VisibleForTesting
   @Deprecated(
@@ -226,6 +228,7 @@ public object WorkerBinder {
    * @param workerLifecycle the worker lifecycle event provider
    * @param worker the class that wants to be informed when to start and stop doing work
    */
+  @OptIn(CoreFriendModuleApi::class)
   @JvmStatic
   @Deprecated(
     message =

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/WorkerScopeProvider.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/WorkerScopeProvider.kt
@@ -16,14 +16,16 @@
 package com.uber.rib.core
 
 import com.uber.autodispose.ScopeProvider
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import com.uber.rib.core.lifecycle.WorkerEvent
 import io.reactivex.CompletableSource
 import io.reactivex.Observable
 
 /** [ScopeProvider] for [Worker] instances. */
 public open class WorkerScopeProvider internal constructor(delegate: ScopeProvider) :
   ScopeProvider by delegate {
-  internal constructor(lifecycle: Observable<WorkerEvent>) : this(lifecycle.asScopeProvider())
+  @CoreFriendModuleApi
+  public constructor(lifecycle: Observable<WorkerEvent>) : this(lifecycle.asScopeProvider())
 }
 
 private fun Observable<*>.asScopeProvider() = asCompletableSource().asScopeProvider()

diff --git a/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/internal/CoreFriendModuleApi.kt b/android/libraries/rib-base/src/main/kotlin/com/uber/rib/core/internal/CoreFriendModuleApi.kt
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2023. Uber Technologies
+ *
+ * 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 com.uber.rib.core.internal
+
+/*
+ * Methods that are visible only to rib-base friend modules.
+ *
+ * Anything marked with this annotation is not intended for public use.
+ */
+@RequiresOptIn(level = RequiresOptIn.Level.ERROR) internal annotation class CoreFriendModuleApi
diff --git a/android/libraries/rib-base/src/test/kotlin/com/uber/rib/core/LazyBackingPropertyTest.kt b/android/libraries/rib-base/src/test/kotlin/com/uber/rib/core/LazyBackingPropertyTest.kt
@@ -16,6 +16,7 @@
 package com.uber.rib.core
 
 import com.google.common.truth.Truth.assertThat
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import io.reactivex.Observable
 import kotlinx.coroutines.channels.produce
 import kotlinx.coroutines.channels.toList
@@ -26,6 +27,8 @@ import org.mockito.kotlin.mock
 
 class LazyBackingPropertyTest {
   @Volatile private var _expensiveObject: ExpensiveObject? = null
+
+  @OptIn(CoreFriendModuleApi::class)
   private val expensiveObject
     get() = ::_expensiveObject.setIfNullAndGet { ExpensiveObject() }
 
@@ -47,6 +50,8 @@ class LazyBackingPropertyTest {
 
 private open class ClassToBeMocked {
   @Volatile private var _prop: Observable<Int>? = null
+
+  @OptIn(CoreFriendModuleApi::class)
   val prop: Observable<Int>
     get() = ::_prop.setIfNullAndGet { Observable.just(1, 2, 3) }
 }

diff --git a/android/libraries/rib-base/src/test/kotlin/com/uber/rib/core/WorkerScopeProviderTest.kt b/android/libraries/rib-base/src/test/kotlin/com/uber/rib/core/WorkerScopeProviderTest.kt
@@ -17,6 +17,7 @@ package com.uber.rib.core
 
 import com.jakewharton.rxrelay2.BehaviorRelay
 import com.jakewharton.rxrelay2.Relay
+import com.uber.rib.core.internal.CoreFriendModuleApi
 import com.uber.rib.core.lifecycle.WorkerEvent
 import io.reactivex.observers.TestObserver
 import org.junit.Before
@@ -27,6 +28,7 @@ class WorkerScopeProviderTest {
   private val lifecycle: Relay<WorkerEvent> = BehaviorRelay.create()
   private lateinit var testObserver: TestObserver<Any>
 
+  @OptIn(CoreFriendModuleApi::class)
   @Before
   fun setUp() {
     testObserver = TestObserver()