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.

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

Jump to bottom

Add Kotlin contracts to exposed Kotlin API #3259

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Add Kotlin contracts to exposed Kotlin API #3259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e3b3ac3
Added contracts for kotlin assertions
awelless Apr 7, 2023
edbe73f
Merge branch 'main' into main
awelless Aug 10, 2023
252cad2
Mark new kotlin assetion methods as experimental
awelless Apr 23, 2023
1d5811b
Adjust assertThrows contract
awelless Jun 3, 2023
19922d6
Add contract for fail
awelless Jun 3, 2023
eccb259
Don't return a value from assertNotNull
awelless Aug 25, 2023
e427978
Increase API version
awelless Aug 25, 2023
544b4e1
Add assertInstanceOf methods
awelless Aug 25, 2023
437d29e
assertInstanceOf methods accept nullable values
awelless Aug 25, 2023
43c08ea
Test for assertInstanceOf nullable upcast
awelless Aug 27, 2023
ff9fd39
Specify lambda InvocationKind explicitly for assertThrows methods
awelless Aug 27, 2023
5a70965
Rename variable names in new assertion examples
awelless Sep 1, 2023
666f73f
Add API annotation for new fail assertion
awelless Sep 1, 2023
9920cf9
Make the comments be sentences
awelless Sep 1, 2023
4d21486
Adjust contract for assertTimeoutPreemptively
awelless Sep 1, 2023
30b9b78
Removed contracts for methods where lambdas may throw exceptions
awelless Sep 8, 2023
41f00c0
Add more tests for methods with contracts
awelless Sep 23, 2023
69e9c38
Merge branch 'main' into main
awelless Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 65 additions & 0 deletions junit-jupiter-api/src/main/kotlin/org/junit/jupiter/api/Assertions.kt
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

I think fun fail(message: (() -> String)?) should also have a contract added to it?

Also, can contracts be added to vararg executables: () -> Unit?

Copy link
Author

Choose a reason for hiding this comment

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

callsInPlace supports only non-nullable arguments.
It would be great to have something like:

fun fail(message: (() -> String)?): Nothing {
    contract {
        if (message != null) {
            callsInPlace(message, EXACTLY_ONCE)
        }
    }

    Assertions.fail<Nothing>(message)
}

but if is not allowed in a contract definition.

Similar thing applies to vararg executables: () -> Unit

Copy link

Choose a reason for hiding this comment

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

Could the if be partially resolved like this? So that more (most) calls end up using the contract description for analysis?

// Covers method references `fail(::foo)`, inline lambdas `fail { }`.
fun fail(message: () -> String): Nothing {
    contract {
        callsInPlace(message, EXACTLY_ONCE)
    }
    Assertions.fail<Nothing>(message)
}
// Covers backwards compatibility and potential edge cases like `fail(foo.bar)` where bar is a nullable functional type.
fun fail(message: (() -> String)?): Nothing {
    Assertions.fail<Nothing>(message)
}

Copy link
Author

@awelless awelless May 27, 2023 •
edited

Choose a reason for hiding this comment

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

The method signatures are same from JVM perspective.

Platform declaration clash: The following declarations have the same JVM signature (fail(Lkotlin/jvm/functions/Function0;)Ljava/lang/Void;):
    fun fail(message: (() -> String)?): Nothing defined in org.junit.jupiter.api in file Assertions.kt
    fun fail(message: () -> String): Nothing defined in org.junit.jupiter.api in file Assertions.kt

Copy link

Choose a reason for hiding this comment

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

Huh, I didn't get that when I tried. Anyway, @JvmName usually helps. The question is if this is a feasible solution to get contract in for most, and still keep supporting other edge cases. What's the use case for nullable lambda here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, creating a separate method with a contract and non-nullable lambda allows kotlin compiler to carry out contract checks when it's sure the passed lambda is not null.
I assume, lambda is made nullable to resemble existing java api where messageSupplier can be nullable

Copy link
Author

Choose a reason for hiding this comment

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

callsInPlace for fail method is also specified with InvocationKind.UNKNOWN .
If we set it to EXACTLY_ONCE the following code compiles:

val expectedValue = "value"
val value: String

try {
    fail {
        value = expectedValue
        "message"
    }
} catch (_: AssertionFailedError) {
    value = "another value"
}

assertEquals(expectedValue, value)

Here we reassign value albeit it is val. I reckon it isn't desirable to allow this code to compile

Expand Up @@ -231,6 +231,71 @@ fun assertNotNull(actual: Any?, messageSupplier: () -> String) {
Assertions.assertNotNull(actual, messageSupplier)
}

/**
* Example usage:
* ```kotlin
* val string: Any = ...
*
* assertInstanceOf<String>(string)
*
* // compiler smart casts string to a String object
* assertTrue(string.isNotEmpty())
* ```
* @see Assertions.assertInstanceOf
*/
@API(since = "5.11", status = EXPERIMENTAL)
inline fun <reified T : Any> assertInstanceOf(actual: Any) {
awelless marked this conversation as resolved.
Show resolved Hide resolved
contract {
returns() implies (actual is T)
}

Assertions.assertInstanceOf(T::class.java, actual)
}

/**
* Example usage:
* ```kotlin
* val string: Any = ...
*
* assertInstanceOf<String>(string, "Should be a String")
*
* // compiler smart casts string to a String object
* assertTrue(string.isNotEmpty())
* ```
* @see Assertions.assertInstanceOf
*/
@API(since = "5.11", status = EXPERIMENTAL)
inline fun <reified T : Any> assertInstanceOf(actual: Any, message: String) {
contract {
returns() implies (actual is T)
}

Assertions.assertInstanceOf(T::class.java, actual, message)
}

/**
* Example usage:
* ```kotlin
* val string: Any = ...
*
* assertInstanceOf<String>(string) { "Should be a String" }
*
* // compiler smart casts string to a String object
awelless marked this conversation as resolved.
Show resolved Hide resolved
* assertTrue(string.isNotEmpty())
* ```
* @see Assertions.assertInstanceOf
*/
@API(since = "5.11", status = EXPERIMENTAL)
inline fun <reified T : Any> assertInstanceOf(actual: Any, noinline messageSupplier: () -> String) {
contract {
returns() implies (actual is T)

callsInPlace(messageSupplier, AT_MOST_ONCE)
}

Assertions.assertInstanceOf(T::class.java, actual, messageSupplier)
}

/**
* Example usage:
* ```kotlin
Expand Down
8 changes: 8 additions & 0 deletions junit-jupiter-engine/src/test/kotlin/org/junit/jupiter/api/KotlinAssertionsTests.kt
View file
Open in desktop
Expand Up @@ -228,6 +228,14 @@ class KotlinAssertionsTests {
// nullableString?.isEmpty()
}

@Test
fun `assertInstanceOf with compiler smart cast`() {
val string: Any = "string"

assertInstanceOf<String>(string)
assertFalse(string.isEmpty()) // smart cast to a String object
}

@Test
fun `assertDoesNotThrow with value initialization in lambda`() {
val value: Int
Expand Down