armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Explicitly add `@Nullable` to the return type of overriding methods

Open ikhoon opened this issue 2 years ago • 1 comments

Armeria does not conventionally add @Nullable annotation to the return type of overriding methods if the methods also return a nullable value. We assume @Nullable annotation is inherited into sub-methods. https://github.com/line/armeria/blob/496a3f63621b68b73fbd626878c811c2acf3d007/core/src/main/java/com/linecorp/armeria/common/annotation/Nullable.java#L35

Kotlin understands JSR-305 @Nullable annotation and produces explicit nullable types platform types. https://kotlinlang.org/docs/java-interop.html#nullability-annotations If a method does not have a @Nullable annotation to the return type, the type is regarded as a non-null type. For example:

// In Java
interface SuperInterface {
    @Nullable
    String optionalValue();
}

interface SubInterface extends SuperInterface {
    // Note that @Nullable annotation is omitted
    @Override
    String optionalValue();
}

// In Kotlin
class NullableApiImplTest: SubInterface {
    // Compile error. Kotlin regards `optionalValue()` as a non-null method.
    override fun optionalValue(): String? {
       return null;
    }
}

For better compatibility with Kotlin and other lint tools such as NullAway, we should explicitly add @Nullable. If @Nullable is present in a super method but not declared in the child method, @Nullable is needed to specify in the child method.

ikhoon avatar Sep 12 '23 08:09 ikhoon

Honestly this sounds like more of an issue of Kotlin not supporting @Inherited.

Having said this, I think adding @Nullable also to inherited methods is fine as long as we have a lint rule which somehow detects this.

jrhee17 avatar Sep 26 '23 01:09 jrhee17