Investigate Kotlin 2.1 compiler warnings for contracts

[marcphilipp]

The Kotlin contracts introduced in #1866 cause the following warnings after bumping apiVersion and languageVersion to 2.1 on the marc/jdk17 branch.

w: junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt:414:9 Wrong invocation kind 'EXACTLY_ONCE' for 'executable: () -> R' specified, the actual invocation kind is 'AT_MOST_ONCE'.
w: junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt:442:9 Wrong invocation kind 'EXACTLY_ONCE' for 'executable: () -> R' specified, the actual invocation kind is 'AT_MOST_ONCE'.
w: junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt:466:9 Wrong invocation kind 'EXACTLY_ONCE' for 'executable: () -> R' specified, the actual invocation kind is 'AT_MOST_ONCE'.
w: junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt:490:9 Wrong invocation kind 'EXACTLY_ONCE' for 'executable: () -> R' specified, the actual invocation kind is 'AT_MOST_ONCE'.

These warnings can be reproduced by running ./gradlew :junit-jupiter-api:compileKotlin --rerun

They are currently not treated as errors:

junit5/junit-jupiter-api/junit-jupiter-api.gradle.kts

Lines 26 to 29 in 4ba2f60

	compileKotlin {
	// https://github.com/junit-team/junit5/issues/4371
	compilerOptions.allWarningsAsErrors = false
	}

Deliverables

• Resolve the warnings or suppress them if they're false positives

[awelless]

At first glance the errors look like false positives.
It seem the reason is that all these lambda calls are wrapped into a try-catch block, which makes the compiler assume a passed lambda may not return.

This KT issue perfectly describes the reason of the error in the evaluateAndWrap method.

assertThrows is more interesting. Either the compiler doesn't understand that any Throwable is rethrown immediately or it doesn't support contract interop with Java at all.

[marcphilipp]

So you think we can safely suppress these warnings? If so, what's the "category" to use with the @Suppress annotation?

[awelless]

I investigated this issue more in detail.

First, it's possible to suppress them with @Suppress("WRONG_INVOCATION_KIND") on the method level.

Second, I perhaps wouldn't do it.
When it comes to assertThrows callsInPlace docs require the function to be inline, which isn't the case there. So it'd be our responsibility to guarantee this contract.
The error in evaluateAndWrap looks valid. If you take this method in isolation, it's possible for executable() to throw, while the method itself doesn't throw anything. So, it's possible to end up with an invalid state, and EXACTLY_ONCE is not guaranteed there.

There was an example of a potential change to assertDoesNotThrow, but it doesn't support suspend functions.

[marcphilipp]

@awelless Thanks for taking a closer look!

If you take this method in isolation, it's possible for executable() to throw, while the method itself doesn't throw anything. So, it's possible to end up with an invalid state, and EXACTLY_ONCE is not guaranteed there.

But isn't executable "invoked exactly one time" even if it throws? Or does EXACTLY_ONCE mean that it's invoked completely, i.e. without throwing an exception?

IIUC, you wouldn't suppress the warnings but relax the contract to use AT_MOST_ONCE for executable parameters, right?

[marcphilipp]

As can be seen in #4481, relaxing the contracts for Executable causes some cases (val initialization) to now be a compiler error.

@awelless Do you have time to take a look at the PR?

[awelless]

@marcphilipp
Yes, I'd start with marking the invocations to be AT_MOST_ONCE. And then see if something can be improved either on the kotlin compiler or juint side.

I'll be able to do a review near the weekend, not earlier.