Skip to content

Adjust Gradle configuration to gradually introduce warnings as errors… #3466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
}
21 changes: 17 additions & 4 deletions kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm typically strongly against that, but this whole file is removed in fast-channels branch, so there are no issues with it here


package kotlinx.coroutines.channels

import kotlinx.atomicfu.*
Expand All @@ -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> {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -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) }
}
}

Expand Down
7 changes: 4 additions & 3 deletions kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<*>) {
Expand Down
3 changes: 2 additions & 1 deletion kotlinx-coroutines-core/common/src/channels/Broadcast.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
1 change: 1 addition & 0 deletions kotlinx-coroutines-core/common/src/channels/Channel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 7 additions & 1 deletion kotlinx-coroutines-core/jvm/src/channels/Actor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions kotlinx-coroutines-core/jvm/src/debug/AgentPremain.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
/*
Expand Down
3 changes: 0 additions & 3 deletions kotlinx-coroutines-core/jvm/src/internal/MainDispatchers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
29 changes: 29 additions & 0 deletions kotlinx-coroutines-core/jvm/test/NoParamAssertionsTest.kt
Original file line number Diff line number Diff line change
@@ -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)
}
}
6 changes: 3 additions & 3 deletions kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -173,7 +173,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
}
}
}
event.dispatcher.processEvent(event.time, event.marker)
event.dispatcher.processEvent(event.marker)
}
}

Expand Down
2 changes: 1 addition & 1 deletion kotlinx-coroutines-test/common/src/TestDispatcher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 '" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions reactive/kotlinx-coroutines-reactor/src/ReactorFlow.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions reactive/kotlinx-coroutines-rx3/src/RxMaybe.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down