Why does IntelliJ suggest to use the elvis operator in a test and skip the assertions?-CodePudding

I have this test case in IntelliJ IDEA 2021.2.3 (Community Edition) Build #IC-212.5457.46, built on October 12, 2021:

@Test
fun mcve() {
    val exception = assertThrows<IllegalArgumentException> {
        throw java.lang.IllegalArgumentException("just for texting")
    }
    val msg = exception.message!!
    assertTrue(msg.contains("testing"), "Should have 'testing' but got '$msg'")
}

I use msg because in real life the expression is longer and I want to keep my assertions shorter.

IntelliJ now suggests to Replace '!!' with '?: return' and make the line look like this:

val msg = exception.message ?: return

This would mean that if there was no message, the test would pass, but it should only pass if there was a message and it contained "testing".

Why does IntelliJ suggest this, when it's different from the original code's behavior? Am I missing something about Kotlin here?

CodePudding user response：

That does seem wrong in the context of a test, it should be more explicit about why it says that. But there is one thing to consider. If your !! expression is wrong, and the exception message is actually null, all you will get is an NPE, not a false assert.

To clarify, Exception.message is nullable, that is how the Exception class is defined. You do know that the message is initialised to "just for texting", but the compiler can't know that for sure, something else in another thread might have changed it for example, or the exception that it received might not have a message at all.

That means that from the point of view of the compiler msg.contains("testing") will never run if your assumption is wrong, so there is no point in going forward, so might as well return out of it.

If you want to check it properly, then you should either add another assertion for msg not null,

val msg = exception.message
assertTrue(msg != null, "whatever message")
assertTrue(msg.contains("testing"), "Should have 'testing' but got '$msg'")

Or leave msg as String? and take advantage of ? in combination with == true and have a single assertion.

val msg = exception.message
assertTrue(msg?.contains("testing") == true, "Should have 'testing' but got '$msg'")

CodePudding user response：

What if whatever exception is thrown has a null message? (I know in this code that can't happen, but the assertThrows method can't provide that guarantee to the compiler.) Using !! will crash the test in that case. Since you're not trying to handle that potential null in your code, it looks like the IDE is suggesting you ignore it in a safer way.

You might want to do this instead:

// leave it as a nullable type
val msg = exception.message
// account for the possible null (condition can evaluate to true, false or null)
assertTrue(msg?.contains("testing") == true, "Should have 'testing' but got '$msg'")

or just suppress the warning if you know the exception is going to have a non-null message (but if you know exactly what's going to be thrown, why do you need to test it?)