Javadoc conventions in the presence of default methods

[Brian Goetz]

We've tried this thread a few times without success, so let's try it again.

There have been a number of cases where its not obvious how to document 
default methods.  After some analysis, this appears to be just another 
case where the complexity was present in Java from day 1, and default 
methods simply bring it to the fore because the confusing cases are 
expected to come up more often.  The following applies equally well to 
methods in abstract classes (or concrete classes) as to defaults.

There are lots of things we might want to document about a method in an 
API.  Historically we've framed them as either being "specification" 
(e.g., necessary postconditions) or "implementation notes" (e.g., hints 
that give the user an idea what's going on under the hood.)  But really, 
there are four boxes (and we've been cramming them into two):

   { API, implementation } x { specification, notes }

(We sometimes use the terms normative/informative to describe the 
difference between specification/notes.)

As background, here are some example uses for default methods which vary 
in their "expected prevalence of overriding".  I think the variety of 
use cases here have contributed to the confusion on how to document 
implementation characteristics.  (Note that all of these have analogues 
in abstract classes too, one can find examples in Abstract{List,Map,Set}.)

1.  Optional methods.  This is when the default implementation is barely 
conformant, such as the following from Iterator:

     public default void remove() {
         throw new UnsupportedOperationException("remove");
     }

It adheres to its contract, because the contract is explicitly weak, but 
any class that cares about removal will definitely want to override it.

2.  Methods with *reasonable* defaults but which might well be 
overridden by implementations that care enough.  For example, again from 
Iterator:

     default void forEach(Consumer<? super E> consumer) {
         while (hasNext())
             consumer.accept(next());
     }

This implementation is perfectly fine for most implementations, but some 
classes (e.g., ArrayList) might have the chance to do better, if their 
maintainers are sufficiently motivated to do so.  The new methods on Map 
(e.g., putIfAbsent) are also in this bucket.

3.  Methods where its pretty unlikely anyone will ever override them, 
such as this method from Predicate:

     public default Predicate<T> and(Predicate<? super T> p) {
         Objects.requireNonNull(p);
         return (T t) -> test(t) && p.test(t);
     }

These are all common enough cases.  The primary reason that the Javadoc 
needs to provide some information about the implementation, separate 
from the API specification, is so that those who would extend these 
classes or interfaces can know which methods they need to / want to 
override.  It should be clear from the doc that anyone who implements 
Iterator MUST implement remove() if they want removal to happen, CAN 
override forEach if they think it will result in better performance, and 
almost certainly don't need to override Predicate.and().


The question is made more complicated by the prevalent use of the 
ambiguous phrase "this implementation."  We often use "this 
implementation" to describe both normative and informative aspects of 
the implementation, and readers are left to guess which.  (Does "this 
implementation" mean all versions of Oracle's JDK forever?  The current 
version in Oracle's JDK?  All versions of all JDKs?  The implementation 
in a specific class?  Could IBM's JDK throw a different exception from 
UOE from the default of Iterator.remove()?  What happens when the doc is 
@inheritDoc'ed into a subclass that overrides the method?  Etc.  The 
phrase is too vague to be useful, and this vagueness has been the 
subject of many bug report.)

I think one measure of success of this effort should be "can we replace 
all uses of 'this implementation' with something that is more 
informative and fits neatly within the model."


As said earlier, there are four boxes.  Here are some descriptions of 
what belongs in each box.

1.  API specification.  This is the one we know and love; a description 
that applies equally to all valid implementations of the method, 
including preconditions, postconditions, etc.

2.  API notes.  Commentary, rationale, or examples pertaining to the API.

3.  Implementation specification.  This is where we say what it means to 
be a valid default implementation (or an overrideable implementation in 
a class), such as "throws UOE."  Similarly this is where we'd describe 
what the default for putIfAbsent does.  It is from this box that the 
would-be-implementer gets enough information to make a sensible decision 
as to whether or not to override.

4.  Implementation notes.  Informative notes about the implementation, 
such as performance characteristics that are specific to the 
implementation in this class in this JDK in this version, and might 
change.  These things are allowed to vary across platforms, vendors and 
versions.

Once we recognize that these are the four boxes, I think everything gets 
simpler.


Strawman Proposal
-----------------

As a strawman proposal, here's one way to explicitly label the four 
boxes: add three new Javadoc tags, @apinote, @implspec, and @implnote. 
(The remaining box, API Spec, needs no new tag, since that's how Javadoc 
is used already.)  @impl{spec,note} can apply equally well to a concrete 
method in a class or a default method in an interface.

(Rule of engagement: bikeshedding the names will be interpreted as a 
waiver to ever say anything about the model or the semantics.  So you 
may bikeshed, but it must be your last word on the topic.)

/**
   * ... API specifications ...
   *
   * @apinote
   * ... API notes ...
   *
   * @implspec
   * ... implementation specification ...
   *
   * @implnote
   * ... implementation notes ...
   *
   * @param ...
   * @return ...
   */

Applying this to some existing Javadoc, take AbstractMap.putAll:

     Copies all of the mappings from the specified map to this map
     (optional operation). The effect of this call is equivalent to
     that of calling put(k, v) on this map once for each mapping from
     key k to value v in the specified map. The behavior of this
     operation is undefined if the specified map is modified while
     the operation is in progress.

     This implementation iterates over the specified map's
     entrySet() collection, and calls this map's put operation
     once for each entry returned by the iteration.

The first paragraph is API specification and the second is 
implementation *specification*, as users expect the implementation in 
AbstractMap, regardless of version or vendor, to behave this way.  The 
change here would be to replace "This implementation" with @implspec, 
and the ambiguity over "this implementation" goes away.

The doc for Iterator.remove could be:

  /**
   * Removes from the underlying collection the last element returned by
   * this iterator (optional operation). This method can be called only
   * once per call to next(). The behavior of an iterator is unspecified
   * if the underlying collection is modified while the iteration is in
   * progress in any way other than by calling this method.
   *
   * @implspec
   * The default implementation must throw UnsupportedOperationException.
   *
   * @implnote
   * For purposes of efficiency, the same UnsupportedOperationException
   * instance is always thrown. [*]
   */

[*] We don't really intend to implement it this way; this is just an 
example of an @implnote.


The doc for Map.putIfAbsent could be:

   /**
    * If the specified key is not already associated with a value,
associates
    * it with the given value.
    *
    * @implspec
    * Th default behaves as if:
    * <pre> {@code
    * if (!map.containsKey(key))
    *   return map.put(key, value);
    * else
    *   return map.get(key);
    * } </pre>
    *
    * @implnote
    * This default implementation is implemented essentially as described
    * in the API note. This operation is not atomic. Atomicity, if desired,
    * must be provided by a subclass implementation.
    */


Secondary: one can build on this to eliminate some common inheritance 
anomalies by making these inheritable separately, where @inheritDoc is 
interpreted as "inherit the stuff from the corresponding section."  This 
is backward compatible because these sections do not yet exist in old 
docs.  SO to inherit API spec and implementation spec, you would do:

  /**
   * {@inheritDoc}
   * @implspec
   * {@inheritDoc}
   * ...
   */