Preconditions (for records, or otherwise)

[Brian Goetz]

> Hello,
>
> I'd claim it's an uncontroversial best practice that method and 
> constructor parameters should be aggressively *checked for validity*, 
> especially when that data is stored and used later (when knowledge of 
> where that bad value came from has vanished).

Yes, and especially so for immutable objects (which we'd like to nudge 
towards somehow, such as making `final` the default for record fields 
and requiring an explicit `non-final`.)

I think what you're getting at is that failing to validate in records 
will become an attractive nuisance; because we've stripped away all the 
boilerplate, reconstructing a place to hang validation code will be on 
the wrong side of the activation energy leap, and anything we can do to 
reduce the cost of responsible validation is worth doing.  I tend to 
agree; if people perceive validation as inconvenient, they won't do it, 
and our data will be of lower quality.  Which is not the goal ;)

> One thing I've been pushing for as a result is that the design of 
> records really, really should not impose a disproportionate penalty to 
> adding the first bit of validation. If to check that a number is 
> positive I have to change
>
>  record Foo(int num, String unrelated) {}
>
> to
>
>  record Foo(int num, String unrelated) {
>    Foo(int num, String unrelated) {
>      if (num <= 0) ...;
>      default.this(num, unrelated);
>    }
>  }
>
> ... then I'd say that the cost of listing my fields three times 
> instead of once is too great, and the user may not bother. For 
> records, the right amount of repetition really is no repetition.

In general, I am wary about arguments that sound like catering to Billy 
Boilerplate, but in this case I agree -- validation is so important, we 
don't want to give people any excuses to not do it.

First, let's catalog the repetition.

I think we can surely eliminate the `default.this(num, unrelated)` call; 
the parser production can tell the difference between a constructor body 
that contains an explicit constructor call (super/this/default.this) and 
one that does not.  So if the user provides a constructor without an 
explicit constructor call, we can just fill in the default 
initialization.  (There are arguments in favor of both putting it at the 
beginning and at the end; I'll take that up separately.)  That gets us 
down to:

  record Foo(int num, String unrelated) {
    Foo(int num, String unrelated) {
      if (num <= 0) ...;
    }
  }

Which is slightly better.  The remaining repetition is the argument list 
of the constructor is repeated.  On the one hand, this seems at first to 
be incompressible: Java members have signatures.  But if we're willing 
to do something special for records, we can take advantage of the fact 
that a record _must_ have a constructor (the "primary constructor") that 
matches the record signature, and let the user leave this out if they 
are declaring an explicit primary constructor:

  record Foo(int num, String unrelated) {
    Foo{
      if (num <= 0) ...;
    }
  }

or

  record Foo(int num, String unrelated) {
    primary-constructor {
      if (num <= 0) ...;
    }
  }

or similar could be a shorthand for declaring the full primary 
constructor signature.  I think that gets you down to your minimum, 
without losing much in the way of readability.

> We've discussed addressing this in either a *records-specific* or a 
> generalized way. The former is (imho) the least we can do to satisfy 
> "first do no harm". This could be a matter of saying that a record's 
> primary constructor gets to have various uninteresting boilerplate be 
> *inferred* (though if we had a way to also get around the traditional 
> annoyance of parameters and fields both being in scope at the same 
> time with the same names, that might be even better). So I'd like to 
> figure out what that would look like. (As a side product, maybe this 
> solution solves the question of "where does a constructor annotation 
> go?".)

The above is a record-specific approach, which takes advantage of the 
fact that a record must have a constructor that matches the record 
signature.

I am a little more wary of dealing with the "pun" between the fields and 
the constructor arguments in a record-specific way, since I'd like for 
there to be a simple mechanical refactoring between records and classes 
that could be records.

If the compiler is going to fill in the super call / field 
initialization, we have two choices: fill them in at the beginning, or 
at the end, of the constructor.  Both have pros and cons.

General: we can lift the restriction of "no statements before 
this/super" (for years, we couldn't, until some verifier improvements 
made this possible).  In that case, statements before the constructor 
call have `this` as DU, so you can't do field access in that part of the 
ctor, you can only work on the arguments:

     Foo(int x, int y) {
         if (x < y) throw ...   // `this` is DU
         this(...);                   // now `this` is DA
         more stuff
     }

If we are presented with

    Foo(int num, String unrelated) {
      if (num <= 0) ...;
    }

we could desugar that into

    Foo(int num, String unrelated) {
      if (num <= 0) ...;
      default.this(num, unrelated);
    }

or
    Foo(int num, String unrelated) {
      default.this(num, unrelated);
      if (num <= 0) ...;
    }

Checking preconditions after you create the object feels wrong to me, so 
the former speaks to me.  It also has the benefit that you can normalize 
the fields without having to explicitly provide the default call:

    Foo(int num, String unrelated) {
      if (num <= 0) num = 0;
      // implicit default, writes normalized num to this.num
    }

The only time you'd need an explicit default under this interpretation 
is if you had additional fields to initialize in the constructor.

Putting the implicit default first is more like how things work now, but 
it means you can only check the values after you write the fields, and 
you can't normalize them without an explicit default.

> Lastly... hey, what about just a *library* like Guava's Preconditions 
> class? I made that thing, and it is extremely popular here. It also 
> gives extraordinarily small benefit. Yeah, it lets you express your 
> expectation positively instead of negatively. It lets you create a 
> message with %s. That's about it. Yawn.

I'd be in favor; seems a pretty big return-on-complexity.  The closest 
thing we have in JDK now is Objects.requireNonNull(), which I use a lot 
but is obviously a one-trick pony.

Other ideas along these lines include:
  - A statement, analogous to assert, but which is unconditional 
("validate");
  - Compiler and documentation heuristics for Preconditions, where, if a 
method begins with a block of preconditions, they are propagated into 
the documentation;
  - Better Javadoc support for describing preconditions, so they don't 
have to be spelled out longhand.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20180309/2b105d1d/attachment-0001.html>