Search code examples
intellij-ideajava-8compiler-warningssuppress-warnings

Can I silence warnings when USING my method?


I'm writing a library method that emulates ?: in Java.

To be fair, I'm not sure how useful this is and I'm just testing it out personally for now.

/**
 * Shorthand for {@link #nullCoalescing(Object, Supplier, Object)}
 */
@NotNull
public static <RawType, BackupType> BackupType nc(
        @Nullable RawType possiblyNullValueContainer,
        @NotNull Supplier<BackupType> ifNotNull,
        @NotNull BackupType nonNullBackup) {
    return nullCoalescing(possiblyNullValueContainer, ifNotNull, nonNullBackup);
}

/**
 * Null Coalescing method: If {@code possiblyNullValueContainer} is {@code null}, {@code nonNullBackup} is returned;
 * else, {@code ifNotNull} is invoked and its result returned. The only caveat occurs when {@code
 * possiblyNullValueContainer} is a non-null {@link Optional}, wherein its {@link Optional#isPresent() isPresent()}
 * method is checked after discovering that it itself is not null.
 *
 * @param possiblyNullValueContainer The possibly-null value to check, or an {@link Optional} whose internal value
 *                                   is to be checked.
 * @param ifNotNull                  If {@code possiblyNullValueContainer} is not {@code null}, this is invoked and
 *                                   its result returned. The intended use is that this references a method that can
 *                                   be called on the aforementioned object, like {@code () ->
 *                                   possiblyNullValueContainer.toString()}. If this is {@code null}, you're just
 *                                   being silly and {@code null} will be returned.
 * @param nonNullBackup              If {@code possiblyNullValueContainer} is {@code null}, this is returned.
 * @param <RawType>                  The type of value to check for nullability
 * @param <BackupType>               The type of the backup values, which should not be null.
 *
 * @return Pretty much {@code possiblyNullValueContainer ? ifNotNull() : nonNullBackup}
 */
@NotNull
@SuppressWarnings("unchecked") // manually type-checked
public static <RawType, BackupType> BackupType nullCoalescing(
        @Nullable RawType possiblyNullValueContainer,
        @NotNull Supplier<BackupType> ifNotNull,
        @NotNull BackupType nonNullBackup) {
    if (null == possiblyNullValueContainer) {
        return nonNullBackup;
    } else if (possiblyNullValueContainer instanceof Optional) {
        // If they pass us an Optional, they must want us to check if it has a value inside rather than itself
        Optional nonNullOpt = (Optional) possiblyNullValueContainer;
        if (!nonNullOpt.isPresent()) {
            return nonNullBackup;
        }
    }

    if (null == ifNotNull) {
        Logger.getGlobal().severe("What are you doing?!");
        return null;
    } else {
        return ifNotNull.get();
    }
}

This is used like such:

// kinda like int i = value ?: 0
int i = nc(value, () -> value.intValue(), 0)

// less useful, I know, but still supported
String s = nc(optional , () -> optional.get(), "simple");

Which works fine. The problem comes when I use the full method name, like:

long l = nullCoalescing(value, () -> value.longValue(), 0L)

and I get this warning:

Unboxing of 'nullCoalescing(value, () -> value.longValue(), 0L)' may produce 'java.lang.NullPointerException'

Which is clearly bubkus, because I manually checked each and every line in that method to ensure no NPE can be thrown while using it, and even sanity-checked by running tests against it. So, how do I stop this warning from appearing whenever someone uses my nullCoalescing method?

Note that this does not happen when using nc (presumably because it doesn't drill any deeper?).


Solution

  • The problem stems from the fact that you are capturing the nullable value and unconditionally invoking a method on that reference within the lambda expression. The fact that the function will only be evaluated when value is not null is not visible to the audit tool, as you are checking the possiblyNullValueContainer parameter, not the captured value (they contain the same reference, but that seems to exceed the audit’s capabilities.

    I recommend a different design anyway:

    @NotNull
    public static <RawType, BackupType> BackupType nullCoalescing(
            @Nullable RawType possiblyNullValueContainer,
            @NotNull Function<RawType,BackupType> ifNotNull,
            @NotNull BackupType nonNullBackup) {
    
        Objects.requireNonNull(ifNotNull);// don't provide fall-backs for illegal use
        Objects.requireNonNull(nonNullBackup); // be explicit
    
        if (null == possiblyNullValueContainer) {
            return nonNullBackup;
        }
        else if (possiblyNullValueContainer instanceof Optional) {
            // If they pass us an Optional,
            // they want us to check if it has a value inside rather than itself
            Optional<?> nonNullOpt = (Optional)possiblyNullValueContainer;
            if (!nonNullOpt.isPresent()) {
                return nonNullBackup;
            }
        }
        return ifNotNull.apply(possiblyNullValueContainer);
    }
    

    By using a Function which receives the reference which has been checked against null, every analyzing tool should recognize that the lambda expression is accessing a non-null value. Even tools not diving into the method won’t complain, as when using it like

    long l = nullCoalescing(value, nonNull -> nonNull.longValue(), 0L);
    

    the lambda expression doesn’t access a captured nullable value, but the lambda expression’s parameter only. As a bonus point, it’ll be potentially more efficient as a non-capturing lambda expression will be represented by a single instance rather than creating new instances on each capturing (as of the current implementation).

    You may even use a simplified form, e.g.

    long l = nullCoalescing(value, nonNull -> nonNull, 0L);
    

    or

    long l = nullCoalescing(value, Long::longValue, 0L);
    

    By the way, the code above solves two other issues as well. By inserting a single <?>, you can get rid of the @SuppressWarnings("unchecked") and generally, it is not a good idea to handle an illegal state by logging and returning null in a method annotated with @NotNull, making the situation even worse than before…