Skip to content
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

Add Kotlin contracts to exposed Kotlin API #1866

Open
4 tasks
JLLeitschuh opened this issue Apr 17, 2019 · 15 comments · May be fixed by #3259
Open
4 tasks

Add Kotlin contracts to exposed Kotlin API #1866

JLLeitschuh opened this issue Apr 17, 2019 · 15 comments · May be fixed by #3259

Comments

@JLLeitschuh
Copy link
Contributor

Foreward

First off, I want to thank the Junit 5 team from being so willing to officially support Kotlin as a first-class citizen in the Junit 5 library. It has been absolutely wonderful being able to use my own contributions in all of my Kotlin projects.

Feature Request

I believe that this API can be further enhanced with the new Kotlin 1.3 feature, Contracts.

Contracts are making guarantees to the compiler that various methods have certain characteristics.

Here's an example from the Kotlin Std-Lib:

/**
 * Throws an [IllegalStateException] if the [value] is null. Otherwise
 * returns the not null value.
 *
 * @sample samples.misc.Preconditions.failCheckWithLazyMessage
 */
@kotlin.internal.InlineOnly
public inline fun <T : Any> checkNotNull(value: T?): T {
    contract {
        returns() implies (value != null)
    }
    return checkNotNull(value) { "Required value was null." }
}

Before Kotlin Contracts, the following code wouldn't have compiled:

fun validateString(aString: String?): String {
    checkNotNull(aString)
    return aString
}

I believe that JUnit 5 has a few places where these contracts would be valuable.

Examples

assertNotNull

@ExperimentalContracts
fun <T: Any> assertNonNull(actual: T?, message: String): T {
    contract {
        returns() implies (actual != null)
    }
    Assertions.assertNotNull(actual, message)
    return actual!!
}

The above would allow something like this:

val exception = assertThrows<IllegalStateException { /** whatever **/}
val message = exception.message
assertNotNull(message)
assertTrue(message.contains("some expected substring")) 

Alternatively, it would also allow for this sort of use case:

val message = assertNotNull(exception.message)

assertThrows / assertDoesNotThrow

Since the callable passed to assertThrows is only ever called once, we can expose that in the contract.

@ExperimentalContracts
inline fun <reified T : Throwable> assertThrows(noinline message: () -> String, noinline executable: () -> Unit): T {
    contract {
        callsInPlace(executable, InvocationKind.EXACTLY_ONCE)
    }
    return Assertions.assertThrows(T::class.java, Executable(executable), Supplier(message))
}

Similar

This would for something like this:

val something: Int
val somethingElse: String
assertDoesNotThrow {
    something = somethingThatDoesntThrow()
    somethingElse = gettingSomethingElse()
}

Caveats

Kotlin Contracts are only supported in Kotlin 1.3 and higher.
This would require a discussion regarding what version of Kotlin the Junit 5 team want's to officially support.

Deliverables

  • Add Kotlin method assertNotNull
  • Add Contracts to Kotlin method assertNotNull
  • Add Contracts to Kotlin method assertThrows
  • Add Contracts to Kotlin method assertDoesNotThrow
@stale
Copy link

stale bot commented May 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label May 13, 2021
@jbduncan
Copy link
Contributor

This sounds like a useful usability feature!

@stale stale bot removed the status: stale label May 13, 2021
@marcphilipp marcphilipp removed this from the 5.8 Backlog milestone Jun 19, 2021
@stale
Copy link

stale bot commented Jun 19, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label Jun 19, 2022
@ephemient
Copy link

Please do this.

@stale
Copy link

stale bot commented Jul 11, 2022

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

@TWiStErRob
Copy link

TWiStErRob commented Sep 19, 2022

@junit-team What is missing to move this forward?

@JLLeitschuh I think these might also be deliverables for full coverage:

  • assertNull to get Kotlin compilation protection for val actual = ...; assertNull(actual); actual.foo()
    (.foo() would be a 100% NPE with current solution and it wouldn't compile with contracts)
  • assertInstanceOf (similar to non-null, but with implies (actualValue is T))

@awelless
Copy link

Is anyone working on this issue?

@JLLeitschuh
Copy link
Contributor Author

Not currently, but I figure at this point it would be safe to do so as Kotlin 1.3 is pretty EOL as of 2 years ago: https://endoflife.date/kotlin

@JLLeitschuh
Copy link
Contributor Author

Also, this library currently supports Kotlin versions 1.3 and above, so it would now be safe to add:

tasks.withType<KotlinCompile>().configureEach {
kotlinOptions {
apiVersion = "1.3"
languageVersion = "1.3"
allWarningsAsErrors = false
}
}

If someone wants to open a pull request to add support for this, I'm more than happy to review it

@awelless
Copy link

Okay.
I could start working on it. However, it might take some time

@TWiStErRob
Copy link

@JLLeitschuh how can this be tested with automation? Because contracts should induce Kotlin compilation failures. I guess the positive cases of smart casts as results of contract can be tested:

val foo: String? = ...
assertNotNull(foo)
assertEquals(1, foo.size)

(if the contracts stop working, the above test will not compile)

@JLLeitschuh
Copy link
Contributor Author

As you suggested, I would use the compiler as your validation

awelless added a commit to awelless/junit5 that referenced this issue Apr 7, 2023
Added assertNull and assertNotNull methods with contracts.
Added contracts for assertThrows and assertDoesNotThrow methods.

assertInstanceOf can be implemented only with kotlin 1.4, because
refined generics
[are not supported](https://youtrack.jetbrains.com/issue/KT-28298)
in contracts for kotlin 1.3 yet.

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Apr 17, 2023
@awelless awelless linked a pull request Apr 17, 2023 that will close this issue
6 tasks
awelless added a commit to awelless/junit5 that referenced this issue Apr 23, 2023
Kotlin assertNull and assertNotNull methods are marked as
experimental.

Issue: junit-team#1866
@awelless
Copy link

I think these might also be deliverables for full coverage:
...
* assertInstanceOf (similar to non-null, but with implies (actualValue is T))

Contracts with reified generics are supported only since kotlin 1.4. Therefore, assertInstanceOf can't be implemented for now

@TWiStErRob
Copy link

I wondered if the that error is suppressed will it emit 1.4 compatible bytecode for the contract? Therefore making it possible to use the same jar binary from both 1.3 and 1.4.

so I tested it... but doesn't seem so

(For reference the Kotlin version is declared in kotlin-library-conventions.gradle.kts)

// kotlinc -language-version 1.3 -api-version 1.3 -d assertIsInstance.jar -opt-in=kotlin.contracts.ExperimentalContracts assertIsInstance.kt
package test
import kotlin.contracts.contract
inline fun <reified T> assertIsInstance(value: Any?) {
    contract {
        returns() implies (value is @Suppress("ERROR_IN_CONTRACT_DESCRIPTION") T)
    }
    require(T::class.java.isInstance(value))
}
// kotlinc -language-version 1.4 -api-version 1.4 -cp assertIsInstance.jar usage.kt
import test.assertIsInstance
fun main() {
    val cs: CharSequence = ""
    assertIsInstance<String>(cs)
    val str: String = cs
    println(str)
}
// usage.kt:6:23: error: type mismatch: inferred type is CharSequence but String was expected
//    val str: String = cs
//                      ^

If the assertIsInstance.jar is compiled with 1.4, then usage.kt compiles as expected.

Funny thing: if assertIsInstance.jar is compiled with 1.4, but usage is compiled with 1.3, it works! So the 1.3 target understands the 1.4 contract, if it's correctly encoded in the .class file in the first place. In the end this information is really for the compiler to know which functions/types to bind to during compilation. It probably works this way because the actual kotlinc compiler I'm using is 1.7.10.

awelless added a commit to awelless/junit5 that referenced this issue Jun 3, 2023
Lambda invoked in assertThrows is marked as being called UNKNOWN number
of times, since contracts do nothing with exception suppression

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Jun 3, 2023
Created another fail method with a non-nullable lambda parameter

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 10, 2023
Added assertNull and assertNotNull methods with contracts.
Added contracts for assertThrows and assertDoesNotThrow methods.

assertInstanceOf can be implemented only with kotlin 1.4, because
refined generics
[are not supported](https://youtrack.jetbrains.com/issue/KT-28298)
in contracts for kotlin 1.3 yet.

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 10, 2023
Kotlin assertNull and assertNotNull methods are marked as
experimental.

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 10, 2023
Lambda invoked in assertThrows is marked as being called UNKNOWN number
of times, since contracts do nothing with exception suppression

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 10, 2023
Created another fail method with a non-nullable lambda parameter

Issue: junit-team#1866
@awelless
Copy link

Since kotlin version has been bumped to 1.6, it should be possible to implement assertInstanceOf with a contract specified

awelless added a commit to awelless/junit5 that referenced this issue Aug 25, 2023
Because of kotlin smart casts, there is no need to return a
non-nullable value from `assertNotNull` methods

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 25, 2023
Api version is increased to 5.11 for `assertNull` and `assertNotNull`
methods

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 25, 2023
Added `assertInstanceOf` assertions with upcasting contracts

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Aug 25, 2023
awelless added a commit to awelless/junit5 that referenced this issue Aug 27, 2023
awelless added a commit to awelless/junit5 that referenced this issue Aug 27, 2023
awelless added a commit to awelless/junit5 that referenced this issue Sep 1, 2023
awelless added a commit to awelless/junit5 that referenced this issue Sep 1, 2023
awelless added a commit to awelless/junit5 that referenced this issue Sep 1, 2023
awelless added a commit to awelless/junit5 that referenced this issue Sep 1, 2023
Invocation kind for the executable is UNKNOWN for
assertTimeoutPreemptively methods

Issue: junit-team#1866
awelless added a commit to awelless/junit5 that referenced this issue Sep 8, 2023
awelless added a commit to awelless/junit5 that referenced this issue Sep 23, 2023
@sbrannen sbrannen changed the title Add Kotlin Contracts to Exposed Kotlin API Add Kotlin contracts to exposed Kotlin API Dec 16, 2023
@marcphilipp marcphilipp added this to the 5.11 M1 milestone Dec 18, 2023
@marcphilipp marcphilipp modified the milestones: 5.11 M1, 5.12 Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants