CanBeNonNullable false positive

[TWiStErRob]

Steps to Reproduce

import javax.xml.stream.XMLStreamWriter

inline fun XMLStreamWriter.attribute(name: String, value: Any) {
	writeAttribute(name, value.toString())
}

inline fun <T : Any> XMLStreamWriter.optionalAttribute(
	name: String,
	value: T?,
	transformation: (T) -> String
) {
	value?.let { attribute(name, transformation(it)) }
}

Expected Behavior

No flagging.

Observed Behavior

XMLStreamWriterDSL.kt:80:70: The nullable parameter 'value' can be made non-nullable. [CanBeNonNullable]

Context

The whole point of this method is to hide the nullability in a named nice fun. The method does the null check, so that it can be just called, and the lambda doesn't have to deal with a null-check. It allows this change:

which actually fixes NestedScopeFunctions.

I'm not entirely sure this is a false positive, I could be mis-interpreting what CanBeNonNullable does.

Your Environment

• Version of detekt used: 1.21.0
• Version of Gradle used (if applicable): 7.5.1
• Operating System and version: Windows
• Link to your project (if it's a public repository): code is not live yet.

[BraisGabin]

This is an expected behavior. The function does nothing when the value is null so it should not allow null values in the first place.

If you really want to keep the function signature I think that a @Suppress is the way to go.

[TWiStErRob]

Thanks. If I make it non-null then I can remove the method, but then I need to put the null check outside of every usage.
Should I be doing this? (I'm trying to find a general way to handle this, because it's quite a recurring pattern)

if(documentation != null)
   attribute("documentation", documentation.toAsciiString())

[TWiStErRob]

(feel free to move this to a discussion)

[BraisGabin]

Yes, that's the idea, to move the check up instead of inside the function. Don't have functions that do "nothing".

But as I said, if that have sense for your use case you can always @Suppress the issue in your case. I mean, the parameter could be non-nullable But even it "could" you prefer to keep it nullable.

[TWiStErRob]

@BraisGabin added your explanation to the rule: #5332

(We follow the same pattern at work, btw, ifs outside.)