Skip to content

Cleanup: reduce the number of warnings during a build #4547

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.

Sign up for GitHub

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 5 commits into from
Nov 12, 2025
Merged

Cleanup: reduce the number of warnings during a build #4547

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3591801
Cleanup: reduce the number of warnings during a build
dkhalanskyjb Oct 16, 2025
44927b6
Remove the `public` from test source sets
dkhalanskyjb Oct 30, 2025
4b881f3
Fix new warnings after a rebase
dkhalanskyjb Oct 30, 2025
b450ca9
Remove one more unnecessary type boundary
dkhalanskyjb Oct 30, 2025
039441e
Add a newline
dkhalanskyjb Nov 12, 2025
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
1 change: 1 addition & 0 deletions buildSrc/src/main/kotlin/configure-compilation-conventions.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ configure(subprojects) {
"kotlinx.cinterop.ExperimentalForeignApi",
"kotlinx.cinterop.UnsafeNumber",
"kotlin.experimental.ExperimentalNativeApi",
"kotlin.native.concurrent.ObsoleteWorkersApi",
)
}
addExtraCompilerFlags(project)
Expand Down
4 changes: 1 addition & 3 deletions kotlinx-coroutines-core/common/test/AsyncLazyTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED") // KT-21913

package kotlinx.coroutines

import kotlinx.coroutines.testing.*
Expand Down Expand Up @@ -109,7 +107,7 @@ class AsyncLazyTest : TestBase() {
assertTrue(!d.isActive && !d.isCompleted)
try {
d.await() // will throw IOException
} catch (e: TestException) {
} catch (_: TestException) {
assertTrue(!d.isActive && d.isCompleted && d.isCancelled)
expect(4)
}
Expand Down
20 changes: 7 additions & 13 deletions kotlinx-coroutines-core/common/test/AsyncTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED", "UNREACHABLE_CODE", "USELESS_IS_CHECK") // KT-21913

package kotlinx.coroutines

import kotlinx.coroutines.testing.*
Expand Down Expand Up @@ -82,11 +80,10 @@ class AsyncTest : TestBase() {
val deferred = async(NonCancellable) {
val decomposed = async(NonCancellable) {
throw TestException()
1
}
try {
decomposed.await()
} catch (e: TestException) {
} catch (_: TestException) {
42
}
}
Expand All @@ -101,29 +98,27 @@ class AsyncTest : TestBase() {
val decomposed = async { // inherits parent job!
expect(3)
throw TestException()
1
}
try {
decomposed.await()
} catch (e: TestException) {
} catch (_: TestException) {
expect(4) // Should catch this exception, but parent is already cancelled
42
}
}
try {
// This will fail
assertEquals(42, deferred.await())
} catch (e: TestException) {
} catch (_: TestException) {
finish(5)
}
}

@Test
fun testParallelDecompositionUncaughtExceptionWithInheritedParent() = runTest(expected = { it is TestException }) {
val deferred = async(NonCancellable) {
val deferred = async<Int>(NonCancellable) {
val decomposed = async {
throw TestException()
1
}

decomposed.await()
Expand All @@ -135,10 +130,9 @@ class AsyncTest : TestBase() {

@Test
fun testParallelDecompositionUncaughtException() = runTest(expected = { it is TestException }) {
val deferred = async(NonCancellable) {
val deferred = async<Int>(NonCancellable) {
val decomposed = async {
throw TestException()
1
}

decomposed.await()
Expand All @@ -158,7 +152,7 @@ class AsyncTest : TestBase() {
deferred.cancel()
try {
deferred.await()
} catch (e: TestException) {
} catch (_: TestException) {
finish(3)
}
}
Expand Down Expand Up @@ -229,7 +223,7 @@ class AsyncTest : TestBase() {
try {
expect(1)
deferred.await()
} catch (e: CancellationException) {
} catch (_: CancellationException) {
finish(3)
}
}
Expand Down
3 changes: 2 additions & 1 deletion kotlinx-coroutines-core/common/test/AtomicCancellationCommonTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,14 @@ class AtomicCancellationCommonTest : TestBase() {
finish(4)
}

@Suppress("DEPRECATION")
Copy link
Contributor

@murfel murfel Oct 27, 2025

Choose a reason for hiding this comment

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

Could you add // Mutex.onLock, since there's no hint neither on github, nor in Idea after the suppress has been added

Copy link
Contributor

@murfel murfel Oct 27, 2025

Choose a reason for hiding this comment

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

Here and in other entries of @Suppress("DEPRECATION")

Copy link
Collaborator Author

@dkhalanskyjb dkhalanskyjb Oct 30, 2025

Choose a reason for hiding this comment

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

Such comments can become outdated when additional APIs used in the test get deprecated. I'd rather not risk misleading the reader. The more reliable method to learn which particular API needs the suppression is to temporarily remove the annotation.

Copy link
Contributor

@murfel murfel Nov 3, 2025

Choose a reason for hiding this comment

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

I see, but without the comment, there's no way to know which suppress the author meant when they wrote that suppress.

(Either way, would be nice to have all suppresses in the codebase either commented, or un-commented. Not necessarily by your PR, of course.)

Copy link
Collaborator Author

@dkhalanskyjb dkhalanskyjb Nov 12, 2025

Choose a reason for hiding this comment

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

I don't see how the authorial intent matters here. The suppression affects the compilation process only, not the behaviors observable by the user, so rigor here feels like a waste of effort to me.

@Test
fun testSelectLockCancellable() = runTest {
expect(1)
val mutex = Mutex(true) // locked mutex
val job = launch(start = CoroutineStart.UNDISPATCHED) {
expect(2)
select<String> { // suspends
select { // suspends
mutex.onLock {
expect(4)
"OK"
Expand Down
36 changes: 17 additions & 19 deletions kotlinx-coroutines-core/common/test/CancellableContinuationHandlersTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED") // KT-21913

package kotlinx.coroutines

import kotlinx.coroutines.testing.*
Expand All @@ -11,15 +9,15 @@ class CancellableContinuationHandlersTest : TestBase() {

@Test
fun testDoubleSubscription() = runTest({ it is IllegalStateException }) {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.invokeOnCancellation { finish(1) }
c.invokeOnCancellation { expectUnreached() }
}
}

@Test
fun testDoubleSubscriptionAfterCompletion() = runTest {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.resume(Unit)
// First invokeOnCancellation is Ok
c.invokeOnCancellation { expectUnreached() }
Expand All @@ -31,7 +29,7 @@ class CancellableContinuationHandlersTest : TestBase() {
@Test
fun testDoubleSubscriptionAfterCompletionWithException() = runTest {
assertFailsWith<TestException> {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.resumeWithException(TestException())
// First invokeOnCancellation is Ok
c.invokeOnCancellation { expectUnreached() }
Expand All @@ -44,23 +42,23 @@ class CancellableContinuationHandlersTest : TestBase() {
@Test
fun testDoubleSubscriptionAfterCancellation() = runTest {
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.cancel()
c.invokeOnCancellation {
assertIs<CancellationException>(it)
expect(1)
}
assertFailsWith<IllegalStateException> { c.invokeOnCancellation { expectUnreached() } }
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
finish(2)
}
}

@Test
fun testSecondSubscriptionAfterCancellation() = runTest {
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
// Set IOC first
c.invokeOnCancellation {
assertNull(it)
Expand All @@ -72,7 +70,7 @@ class CancellableContinuationHandlersTest : TestBase() {
// then try to install another one
assertFailsWith<IllegalStateException> { c.invokeOnCancellation { expectUnreached() } }
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
finish(3)
}
}
Expand All @@ -83,7 +81,7 @@ class CancellableContinuationHandlersTest : TestBase() {
val job = launch(start = CoroutineStart.UNDISPATCHED) {
// will be cancelled during dispatch
assertFailsWith<CancellationException> {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
cont = c
// Set IOC first -- not called (completed)
c.invokeOnCancellation {
Expand All @@ -105,39 +103,39 @@ class CancellableContinuationHandlersTest : TestBase() {
expect(6)
// then try to install another one after we've done dispatching it
assertFailsWith<IllegalStateException> {
cont!!.invokeOnCancellation { expectUnreached() }
cont.invokeOnCancellation { expectUnreached() }
}
finish(7)
}

@Test
fun testDoubleSubscriptionAfterCancellationWithCause() = runTest {
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.cancel(AssertionError())
c.invokeOnCancellation {
require(it is AssertionError)
expect(1)
}
assertFailsWith<IllegalStateException> { c.invokeOnCancellation { expectUnreached() } }
}
} catch (e: AssertionError) {
} catch (_: AssertionError) {
finish(2)
}
}

@Test
fun testDoubleSubscriptionMixed() = runTest {
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.invokeOnCancellation {
require(it is IndexOutOfBoundsException)
expect(1)
}
c.cancel(IndexOutOfBoundsException())
assertFailsWith<IllegalStateException> { c.invokeOnCancellation { expectUnreached() } }
}
} catch (e: IndexOutOfBoundsException) {
} catch (_: IndexOutOfBoundsException) {
finish(2)
}
}
Expand All @@ -148,11 +146,11 @@ class CancellableContinuationHandlersTest : TestBase() {
) {
expect(1)
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
c.invokeOnCancellation { throw AssertionError() }
c.cancel()
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
expect(2)
}
finish(3)
Expand All @@ -171,13 +169,13 @@ class CancellableContinuationHandlersTest : TestBase() {
val s = MySegment()
expect(1)
try {
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
expect(2)
c as CancellableContinuationImpl<*>
c.invokeOnCancellation(s, 0)
c.cancel()
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
expect(3)
}
expect(4)
Expand Down
26 changes: 12 additions & 14 deletions kotlinx-coroutines-core/common/test/CancellableContinuationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED") // KT-21913

package kotlinx.coroutines

import kotlinx.coroutines.testing.*
Expand All @@ -13,18 +11,18 @@ class CancellableContinuationTest : TestBase() {
val job = launch {
try {
expect(2)
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
continuation = c
}
} catch (e: TestException) {
} catch (_: TestException) {
expect(3)
}
}
expect(1)
yield()
continuation!!.resumeWithException(TestException())
yield()
assertFailsWith<IllegalStateException> { continuation!!.resumeWithException(TestException()) }
assertFailsWith<IllegalStateException> { continuation.resumeWithException(TestException()) }
job.join()
finish(4)
}
Expand All @@ -34,7 +32,7 @@ class CancellableContinuationTest : TestBase() {
var continuation: Continuation<Unit>? = null
val job = launch {
expect(2)
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
continuation = c
}
expect(3)
Expand All @@ -43,7 +41,7 @@ class CancellableContinuationTest : TestBase() {
yield()
continuation!!.resume(Unit)
job.join()
assertFailsWith<IllegalStateException> { continuation!!.resumeWithException(TestException()) }
assertFailsWith<IllegalStateException> { continuation.resumeWithException(TestException()) }
finish(4)
}

Expand All @@ -52,7 +50,7 @@ class CancellableContinuationTest : TestBase() {
var continuation: Continuation<Unit>? = null
val job = launch {
expect(2)
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
continuation = c
}
expect(3)
Expand All @@ -61,12 +59,12 @@ class CancellableContinuationTest : TestBase() {
yield()
continuation!!.resume(Unit)
job.join()
assertFailsWith<IllegalStateException> { continuation!!.resume(Unit) }
assertFailsWith<IllegalStateException> { continuation.resume(Unit) }
finish(4)
}

/**
* Cancelling outer job may, in practise, race with attempt to resume continuation and resumes
* Cancelling the outer job may, in practice, race with an attempt to resume continuation and resumes
* should be ignored. Here suspended coroutine is cancelled but then resumed with exception.
*/
@Test
Expand All @@ -75,10 +73,10 @@ class CancellableContinuationTest : TestBase() {
val job = launch {
try {
expect(2)
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
continuation = c
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
expect(3)
}
}
Expand All @@ -100,10 +98,10 @@ class CancellableContinuationTest : TestBase() {
val job = launch {
try {
expect(2)
suspendCancellableCoroutine<Unit> { c ->
suspendCancellableCoroutine { c ->
continuation = c
}
} catch (e: CancellationException) {
} catch (_: CancellationException) {
expect(3)
}
}
Expand Down
Loading