Error prone should enforce @Value.Default on defaults in install configurations

[Jolyon-S]

What happened?

Omitting @Value.Default on a method with default in an install config interface will compile but will fail if that field is ever set, e.g.

@JsonProperty("blah")
default boolean myProperty() {
    return false;
}

will throw on startup if blah is set.

What did you want to happen?

Error prone should check that, in the install configuration, anything default has a @Value.Default annotation.

[iamdanfox]

Seems reasonable, although I think there are some edge cases we'd want to handle:

• default methods with parameters
• @Value.Check methods
• methods which should really be annotated @Value.Derived / @Value.Lazy or @Value.Auxiliary, e.g.

@Value.Immutable
public interface CertificateParams {
    PublicKey subjectPublicKey();

    PrivateKey issuerPrivateKey();

    X500Name issuerName();

    X500Name subject();

    Set<Hostname> subjectAlternativeHostnames();

    default BigInteger serial() {
        return new BigInteger(20, new SecureRandom());
    }

    default Date notBefore() {
        Calendar calendar = Calendar.getInstance();
        calendar.setTimeInMillis(Instant.now().toEpochMilli());
        return calendar.getTime();
    }

    default Date notAfter() {
        Calendar calendar = Calendar.getInstance();
        calendar.setTimeInMillis(Instant.now().toEpochMilli());
        calendar.add(Calendar.YEAR, 10);
        return calendar.getTime();
    }

    static ImmutableCertificateParams.Builder builder() {
        return ImmutableCertificateParams.builder();
    }
}

We generally try not to couple our static analysis to specific internal libs (e.g. WC and its concept of runtime/install config), as we ideally want these to be applicable to all java codebases, including gotham!

[Jolyon-S]

I think that's pretty reasonable, but I'm mostly flagging because the cost of getting this wrong is that a host refuses to start outright (because it doesn't recognise the parameter). Fortunately we tested this on internal-test-stack but this could have caused a more serious issue if we did not (and it wasn't caught in code review). So just wondering how we can make this better other than best-effort.

[carterkozak]

defaultAsDefault = true on the immutables style annotation makes this sort of thing easier, however it doesn't allow default interface methods to be used without storing a value. The Lazy annotation can be used on default methods to lazily compute the value the first time it's requested, but the value will be stored rather than computed each invocation.