Adjust Gradle configuration and introduce allWarningsAsErrors (#3466)

* Also, add tests that verify our disabled assertions
* Fix nullability in AgentPremain that used to work by accident (because we disabled those assertions)
* Fix all corresponding warnings


Co-authored-by: Dmitry Khalanskiy <[email protected]>

Author: qwwdfsad

build.gradle

@@ -168,18 +168,7 @@ configure(subprojects.findAll { !sourceless.contains(it.name) && it.name != core
 apply plugin: "bom-conventions"
 
 // Configure subprojects with Kotlin sources
-configure(subprojects.findAll { !sourceless.contains(it.name) }) {
-    // Use atomicfu plugin, it also adds all the necessary dependencies
-    apply plugin: 'kotlinx-atomicfu'
-
-    // Configure options for all Kotlin compilation tasks
-    tasks.withType(AbstractKotlinCompile).all {
-        kotlinOptions.freeCompilerArgs += OptInPreset.optInAnnotations.collect { "-Xopt-in=" + it }
-        kotlinOptions.freeCompilerArgs += "-progressive"
-        // Remove null assertions to get smaller bytecode on Android
-        kotlinOptions.freeCompilerArgs += ["-Xno-param-assertions", "-Xno-receiver-assertions", "-Xno-call-assertions"]
-    }
-}
+apply plugin: "configure-compilation-conventions"
 
 if (build_snapshot_train) {
     println "Hacking test tasks, removing stress and flaky tests"

buildSrc/src/main/kotlin/configure-compilation-conventions.gradle.kts

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+import org.jetbrains.kotlin.gradle.tasks.*
+
+configure(subprojects) {
+    if (name in sourceless) return@configure
+    apply(plugin = "kotlinx-atomicfu")
+    val projectName = name
+    tasks.withType(KotlinCompile::class).all {
+        val isMainTaskName = name == "compileKotlin" || name == "compileKotlinJvm"
+        kotlinOptions {
+            if (isMainTaskName) {
+                allWarningsAsErrors = true
+            }
+            val newOptions =
+                listOf(
+                    "-progressive", "-Xno-param-assertions", "-Xno-receiver-assertions",
+                    "-Xno-call-assertions"
+                ) + optInAnnotations.map { "-opt-in=$it" }
+            freeCompilerArgs = freeCompilerArgs + newOptions
+        }
+    }
+}

kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt

@@ -2,6 +2,8 @@
  * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
  */
 
+@file:Suppress("UNCHECKED_CAST", "UNUSED_PARAMETER")
+
 package kotlinx.coroutines.channels
 
 import kotlinx.atomicfu.*
@@ -16,6 +18,7 @@ import kotlin.jvm.*
 /**
  * Abstract send channel. It is a base class for all send channel implementations.
  */
+@Suppress("UNCHECKED_CAST", "UNUSED_PARAMETER")
 internal abstract class AbstractSendChannel<E>(
     @JvmField protected val onUndeliveredElement: OnUndeliveredElement<E>?
 ) : SendChannel<E> {
@@ -122,7 +125,12 @@ internal abstract class AbstractSendChannel<E>(
         return sendSuspend(element)
     }
 
-    @Suppress("DEPRECATION", "DEPRECATION_ERROR")
+    @Suppress("DEPRECATION_ERROR")
+    @Deprecated(
+        level = DeprecationLevel.ERROR,
+        message = "Deprecated in the favour of 'trySend' method",
+        replaceWith = ReplaceWith("trySend(element).isSuccess")
+    ) // see super()
     override fun offer(element: E): Boolean {
         // Temporary migration for offer users who rely on onUndeliveredElement
         try {
@@ -705,6 +713,11 @@ internal abstract class AbstractChannel<E>(
             onCancellationConstructor = onUndeliveredElementReceiveCancellationConstructor
         )
 
+    @Deprecated(
+        message = "Deprecated in favor of onReceiveCatching extension",
+        level = DeprecationLevel.ERROR,
+        replaceWith = ReplaceWith("onReceiveCatching")
+    ) // See super()
     override val onReceiveOrNull: SelectClause1<E?>
         get() = SelectClause1Impl<E?>(
             clauseObject = this,
@@ -726,7 +739,7 @@ internal abstract class AbstractChannel<E>(
         if (selectResult is Closed<*>) throw selectResult.receiveException
         else selectResult as E
 
-    private fun processResultSelectReceiveCatching(ignoredParam: Any?, selectResult: Any?): Any? =
+    private fun processResultSelectReceiveCatching(ignoredParam: Any?, selectResult: Any?): Any =
         if (selectResult is Closed<*>) ChannelResult.closed(selectResult.closeCause)
         else ChannelResult.success(selectResult as E)
 
@@ -735,8 +748,8 @@ internal abstract class AbstractChannel<E>(
         else selectResult as E
 
     private val onUndeliveredElementReceiveCancellationConstructor: OnCancellationConstructor? = onUndeliveredElement?.let {
-        { select: SelectInstance<*>, ignoredParam: Any?, element: Any? ->
-            { cause: Throwable -> if (element !is Closed<*>) onUndeliveredElement.callUndeliveredElement(element as E, select.context) }
+        { select: SelectInstance<*>, _: Any?, element: Any? ->
+            { _: Throwable -> if (element !is Closed<*>) onUndeliveredElement.callUndeliveredElement(element as E, select.context) }
         }
     }
 

kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt

@@ -158,9 +158,10 @@ internal open class ArrayChannel<E>(
                     // Too late, already cancelled, but we removed it from the queue and need to notify on undelivered element.
                     // The only exception is when this "send" operation is an `onSend` clause that has to be re-registered
                     // in the corresponding `select` invocation.
-                    val send = send!!
-                    if (!(send is SendElementSelectWithUndeliveredHandler<*> && send.trySelectResult == REREGISTER))
-                        send.undeliveredElement()
+                    send!!.apply {
+                        if (!(this is SendElementSelectWithUndeliveredHandler<*> && trySelectResult == REREGISTER))
+                            undeliveredElement()
+                    }
                 }
             }
             if (replacement !== POLL_FAILED && replacement !is Closed<*>) {

kotlinx-coroutines-core/common/src/channels/Broadcast.kt

@@ -47,10 +47,11 @@ public fun <E> ReceiveChannel<E>.broadcast(
     start: CoroutineStart = CoroutineStart.LAZY
 ): BroadcastChannel<E> {
     val scope = GlobalScope + Dispatchers.Unconfined + CoroutineExceptionHandler { _, _ -> }
+    val channel = this
     // We can run this coroutine in the context that ignores all exceptions, because of `onCompletion = consume()`
     // which passes all exceptions upstream to the source ReceiveChannel
     return scope.broadcast(capacity = capacity, start = start, onCompletion = { cancelConsumed(it) }) {
-        for (e in this@broadcast) {
+        for (e in channel) {
             send(e)
         }
     }

kotlinx-coroutines-core/common/src/channels/Channel.kt

@@ -360,6 +360,7 @@ public interface ReceiveChannel<out E> {
      *
      * @suppress **Deprecated**: in favor of onReceiveCatching extension.
      */
+    @Suppress("DEPRECATION_ERROR")
     @Deprecated(
         message = "Deprecated in favor of onReceiveCatching extension",
         level = DeprecationLevel.ERROR,

kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt

@@ -207,6 +207,7 @@ internal class DispatchedContinuation<in T>(
 
     // We inline it to save an entry on the stack in cases where it shows (unconfined dispatcher)
     // It is used only in Continuation<T>.resumeCancellableWith
+    @Suppress("NOTHING_TO_INLINE")
     inline fun resumeCancellableWith(
         result: Result<T>,
         noinline onCancellation: ((cause: Throwable) -> Unit)?
@@ -235,7 +236,7 @@ internal class DispatchedContinuation<in T>(
     }
 
     // inline here is to save us an entry on the stack for the sake of better stacktraces
-
+    @Suppress("NOTHING_TO_INLINE")
     inline fun resumeCancelled(state: Any?): Boolean {
         val job = context[Job]
         if (job != null && !job.isActive) {
@@ -247,6 +248,7 @@ internal class DispatchedContinuation<in T>(
         return false
     }
 
+    @Suppress("NOTHING_TO_INLINE")
     inline fun resumeUndispatchedWith(result: Result<T>) {
         withContinuationContext(continuation, countOrElement) {
             continuation.resumeWith(result)

kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt

@@ -167,7 +167,6 @@ internal fun <T> DispatchedTask<T>.dispatch(mode: Int) {
     }
 }
 
-@Suppress("UNCHECKED_CAST")
 internal fun <T> DispatchedTask<T>.resume(delegate: Continuation<T>, undispatched: Boolean) {
     // This resume is never cancellable. The result is always delivered to delegate continuation.
     val state = takeState()

kotlinx-coroutines-core/jvm/src/channels/Actor.kt

@@ -162,7 +162,12 @@ private class LazyActorCoroutine<E>(
         return super.send(element)
     }
 
-    @Suppress("DEPRECATION", "DEPRECATION_ERROR")
+    @Suppress("DEPRECATION_ERROR")
+    @Deprecated(
+        level = DeprecationLevel.ERROR,
+        message = "Deprecated in the favour of 'trySend' method",
+        replaceWith = ReplaceWith("trySend(element).isSuccess")
+    ) // See super()
     override fun offer(element: E): Boolean {
         start()
         return super.offer(element)
@@ -181,6 +186,7 @@ private class LazyActorCoroutine<E>(
         return closed
     }
 
+    @Suppress("UNCHECKED_CAST")
     override val onSend: SelectClause2<E, SendChannel<E>> get() = SelectClause2Impl(
         clauseObject = this,
         regFunc = LazyActorCoroutine<*>::onSendRegFunction as RegistrationFunction,

kotlinx-coroutines-core/jvm/src/debug/AgentPremain.kt

@@ -26,6 +26,7 @@ internal object AgentPremain {
     }.getOrNull() ?: DebugProbesImpl.enableCreationStackTraces
 
     @JvmStatic
+    @Suppress("UNUSED_PARAMETER")
     fun premain(args: String?, instrumentation: Instrumentation) {
         AgentInstallationType.isInstalledStatically = true
         instrumentation.addTransformer(DebugProbesTransformer)
@@ -36,13 +37,13 @@ internal object AgentPremain {
 
     internal object DebugProbesTransformer : ClassFileTransformer {
         override fun transform(
-            loader: ClassLoader,
+            loader: ClassLoader?,
             className: String,
             classBeingRedefined: Class<*>?,
             protectionDomain: ProtectionDomain,
             classfileBuffer: ByteArray?
         ): ByteArray? {
-            if (className != "kotlin/coroutines/jvm/internal/DebugProbesKt") {
+            if (loader == null || className != "kotlin/coroutines/jvm/internal/DebugProbesKt") {
                return null
             }
             /*

kotlinx-coroutines-core/jvm/src/internal/MainDispatchers.kt

@@ -98,9 +98,6 @@ private class MissingMainCoroutineDispatcher(
     override fun limitedParallelism(parallelism: Int): CoroutineDispatcher =
         missing()
 
-    override suspend fun delay(time: Long) =
-        missing()
-
     override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle =
         missing()
 

kotlinx-coroutines-core/jvm/test/NoParamAssertionsTest.kt

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+package kotlinx.coroutines
+
+import kotlinx.coroutines.*
+import org.junit.Test
+import kotlin.test.*
+
+
+class NoParamAssertionsTest : TestBase() {
+    // These tests verify that we haven't omitted "-Xno-param-assertions" and "-Xno-receiver-assertions"
+
+    @Test
+    fun testNoReceiverAssertion() {
+        val function: (ThreadLocal<Int>, Int) -> ThreadContextElement<Int> = ThreadLocal<Int>::asContextElement
+        @Suppress("UNCHECKED_CAST")
+        val unsafeCasted = function as ((ThreadLocal<Int>?, Int) -> ThreadContextElement<Int>)
+        unsafeCasted(null, 42)
+    }
+
+    @Test
+    fun testNoParamAssertion() {
+        val function: (ThreadLocal<Any>, Any) -> ThreadContextElement<Any> = ThreadLocal<Any>::asContextElement
+        @Suppress("UNCHECKED_CAST")
+        val unsafeCasted = function as ((ThreadLocal<Any?>?, Any?) -> ThreadContextElement<Any>)
+        unsafeCasted(ThreadLocal.withInitial { Any() }, null)
+    }
+}

kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt

@@ -97,7 +97,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
             currentTime = event.time
             event
         }
-        event.dispatcher.processEvent(event.time, event.marker)
+        event.dispatcher.processEvent(event.marker)
         return true
     }
 
@@ -132,7 +132,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
             val event = synchronized(lock) {
                 events.removeFirstIf { it.time <= timeMark } ?: return
             }
-            event.dispatcher.processEvent(event.time, event.marker)
+            event.dispatcher.processEvent(event.marker)
         }
     }
 
@@ -173,7 +173,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
                     }
                 }
             }
-            event.dispatcher.processEvent(event.time, event.marker)
+            event.dispatcher.processEvent(event.marker)
         }
     }
 

kotlinx-coroutines-test/common/src/TestDispatcher.kt

@@ -23,7 +23,7 @@ public abstract class TestDispatcher internal constructor() : CoroutineDispatche
     public abstract val scheduler: TestCoroutineScheduler
 
     /** Notifies the dispatcher that it should process a single event marked with [marker] happening at time [time]. */
-    internal fun processEvent(time: Long, marker: Any) {
+    internal fun processEvent(marker: Any) {
         check(marker is Runnable)
         marker.run()
     }

kotlinx-coroutines-test/jvm/src/migration/TestBuildersDeprecated.kt

@@ -159,7 +159,7 @@ public fun runTestWithLegacyScope(
     context: CoroutineContext = EmptyCoroutineContext,
     dispatchTimeoutMs: Long = DEFAULT_DISPATCH_TIMEOUT_MS,
     testBody: suspend TestCoroutineScope.() -> Unit
-): TestResult {
+) {
     if (context[RunningInRunTest] != null)
         throw IllegalStateException("Calls to `runTest` can't be nested. Please read the docs on `TestResult` for details.")
     val testScope = TestBodyCoroutine(createTestCoroutineScope(context + RunningInRunTest))

kotlinx-coroutines-test/jvm/src/migration/TestCoroutineDispatcher.kt

@@ -61,6 +61,10 @@ public class TestCoroutineDispatcher(public override val scheduler: TestCoroutin
         scheduler.registerEvent(this, 0, block, context) { false }
 
     /** @suppress */
+    @Deprecated(
+        "Please use a dispatcher that is paused by default, like `StandardTestDispatcher`.",
+        level = DeprecationLevel.WARNING
+    )
     override suspend fun pauseDispatcher(block: suspend () -> Unit) {
         val previous = dispatchImmediately
         dispatchImmediately = false
@@ -72,11 +76,19 @@ public class TestCoroutineDispatcher(public override val scheduler: TestCoroutin
     }
 
     /** @suppress */
+    @Deprecated(
+        "Please use a dispatcher that is paused by default, like `StandardTestDispatcher`.",
+        level = DeprecationLevel.WARNING
+    )
     override fun pauseDispatcher() {
         dispatchImmediately = false
     }
 
     /** @suppress */
+    @Deprecated(
+        "Please use a dispatcher that is paused by default, like `StandardTestDispatcher`.",
+        level = DeprecationLevel.WARNING
+    )
     override fun resumeDispatcher() {
         dispatchImmediately = true
     }

kotlinx-coroutines-test/jvm/src/migration/TestCoroutineExceptionHandler.kt

@@ -42,6 +42,7 @@ public interface UncaughtExceptionCaptor {
 /**
  * An exception handler that captures uncaught exceptions in tests.
  */
+@Suppress("DEPRECATION")
 @Deprecated(
     "Deprecated for removal without a replacement. " +
         "It may be to define one's own `CoroutineExceptionHandler` if you just need to handle '" +

kotlinx-coroutines-test/jvm/src/migration/TestCoroutineScope.kt

@@ -86,6 +86,7 @@ private class TestCoroutineScopeImpl(
     /** These jobs existed before the coroutine scope was used, so it's alright if they don't get cancelled. */
     private val initialJobs = coroutineContext.activeJobs()
 
+    @Deprecated("Please call `runTest`, which automatically performs the cleanup, instead of using this function.")
     override fun cleanupTestCoroutines() {
         val delayController = coroutineContext.delayController
         val hasUnfinishedJobs = if (delayController != null) {

reactive/kotlinx-coroutines-reactor/src/ReactorFlow.kt

@@ -32,8 +32,7 @@ private class FlowAsFlux<T : Any>(
     private val flow: Flow<T>,
     private val context: CoroutineContext
 ) : Flux<T>() {
-    override fun subscribe(subscriber: CoreSubscriber<in T>?) {
-        if (subscriber == null) throw NullPointerException()
+    override fun subscribe(subscriber: CoreSubscriber<in T>) {
         val hasContext = !subscriber.currentContext().isEmpty
         val source = if (hasContext) flow.flowOn(subscriber.currentContext().asCoroutineContext()) else flow
         subscriber.onSubscribe(FlowSubscription(source, subscriber, context))

reactive/kotlinx-coroutines-rx3/src/RxMaybe.kt

@@ -43,6 +43,7 @@ private class RxMaybeCoroutine<T>(
 ) : AbstractCoroutine<T>(parentContext, false, true) {
     override fun onCompleted(value: T) {
         try {
+            @Suppress("NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS") // KT-54201
             if (value == null) subscriber.onComplete() else subscriber.onSuccess(value)
         } catch (e: Throwable) {
             handleUndeliverableException(e, context)