Using @implSpec in AbstractMap, AbstractCollection, AbstractList, etc

[Martin Buchholz]

As the subject says, these changes should be applied to all of the
so-called "skeletal implementations" in java.util.

There is a *lot* of (previously unavoidable (painful memories)) javadoc
duplication in java.util, some of which could now be undone.


On Thu, Jan 2, 2014 at 8:03 AM, Chris Hegarty <chris.hegarty at oracle.com>wrote:

> Hi Doug,
>
> I agree with your changes to AbstractMap. I’ve taken your patch, removed
> the superfluous paragraph tags, and generated a webrev.
>    http://cr.openjdk.java.net/~chegar/AbstractMap_implSpec/webrev/
>
> I also ran specdiff to see what else may be impacted by this. It shows
> that the only spec changes are to AbstractMap itself, and to
> ConcurrentHashMap size and isEmpty. What we want.
>
> http://cr.openjdk.java.net/~chegar/AbstractMap_implSpec/specdiff/overview-summary.html
>
> I know it is late in the JDK 8 game, but I see this as a serious bug in
> the documentation, and it needs to be addressed.
>
> Conservatively, one could argue that minimally pasting the appropriate
> specs into CHM size and isEmpty would be sufficient to resolve the problem,
> but I think your first proposal is a more complete solution. Given the
> above specdiff, I would be confident to request this change for inclusion
> in JDK 8.
>
> I filed the following bug to track this issue:
>    https://bugs.openjdk.java.net/browse/JDK-8031133
>
> -Chris.
>
> On 28 Dec 2013, at 13:47, Doug Lea <dl at cs.oswego.edu> wrote:
>
> >
> > There might have been some mis-communication about whether the
> > new @implSpec tag would be used in existing java.util.AbstractX
> > classes. I had assumed that it would be, and pulled out a few
> > javadoc overrides in java.util.concurrent classes that would
> > not be needed if @implSpec were used. With @implSpec, you do
> > not need to paste in the interface-level spec in an overridden
> > method just to suppress inclusion of (incorrect) doc comments
> > about the default implementation.
> >
> > Is it too late to do this for JDK8? It is easy -- just add a line
> > with @implSpec for each defined method. AbstractMap diffs
> > below. (They could be tweaked to get rid of now-unnecessary
> > paragraph markup, but I don't think this would improve display.)
> >
> > If this isn't done, then minimally we'd need to paste in
> > interface-level specs in ConcurrentHashMap.{size, isEmpty}
> > since the JDK8 javadocs as they stand are wrong. There
> > may be other cases as well.
> >
> > ...
> >
> >
> > diff -r 8bc258c021a3 src/share/classes/java/util/AbstractMap.java
> > --- a/src/share/classes/java/util/AbstractMap.java    Thu Nov 21
> 09:23:03 2013 -0800
> > +++ b/src/share/classes/java/util/AbstractMap.java    Sat Dec 28
> 08:33:42 2013 -0500
> > @@ -78,6 +78,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation returns <tt>entrySet().size()</tt>.
> >      */
> >     public int size() {
> > @@ -87,6 +88,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation returns <tt>size() == 0</tt>.
> >      */
> >     public boolean isEmpty() {
> > @@ -96,6 +98,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over <tt>entrySet()</tt> searching
> >      * for an entry with the specified value.  If such an entry is found,
> >      * <tt>true</tt> is returned.  If the iteration terminates without
> > @@ -126,6 +129,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over <tt>entrySet()</tt> searching
> >      * for an entry with the specified key.  If such an entry is found,
> >      * <tt>true</tt> is returned.  If the iteration terminates without
> > @@ -157,6 +161,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over <tt>entrySet()</tt> searching
> >      * for an entry with the specified key.  If such an entry is found,
> >      * the entry's value is returned.  If the iteration terminates
> without
> > @@ -191,6 +196,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation always throws an
> >      * <tt>UnsupportedOperationException</tt>.
> >      *
> > @@ -206,6 +212,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over <tt>entrySet()</tt>
> searching for an
> >      * entry with the specified key.  If such an entry is found, its
> value is
> >      * obtained with its <tt>getValue</tt> operation, the entry is
> removed
> > @@ -255,6 +262,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over the specified map's
> >      * <tt>entrySet()</tt> collection, and calls this map's <tt>put</tt>
> >      * operation once for each entry returned by the iteration.
> > @@ -276,6 +284,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation calls <tt>entrySet().clear()</tt>.
> >      *
> >      * <p>Note that this implementation throws an
> > @@ -302,6 +311,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation returns a set that subclasses {@link
> AbstractSet}.
> >      * The subclass's iterator method returns a "wrapper object" over
> this
> >      * map's <tt>entrySet()</tt> iterator.  The <tt>size</tt> method
> > @@ -358,6 +368,7 @@
> >     /**
> >      * {@inheritDoc}
> >      *
> > +     * @implSpec
> >      * <p>This implementation returns a collection that subclasses {@link
> >      * AbstractCollection}.  The subclass's iterator method returns a
> >      * "wrapper object" over this map's <tt>entrySet()</tt> iterator.
> > @@ -425,6 +436,7 @@
> >      * <tt>equals</tt> method works properly across different
> implementations
> >      * of the <tt>Map</tt> interface.
> >      *
> > +     * @implSpec
> >      * <p>This implementation first checks if the specified object is
> this map;
> >      * if so it returns <tt>true</tt>.  Then, it checks if the specified
> >      * object is a map whose size is identical to the size of this map;
> if
> > @@ -478,6 +490,7 @@
> >      * <tt>m1</tt> and <tt>m2</tt>, as required by the general contract
> of
> >      * {@link Object#hashCode}.
> >      *
> > +     * @implSpec
> >      * <p>This implementation iterates over <tt>entrySet()</tt>, calling
> >      * {@link Map.Entry#hashCode hashCode()} on each element (entry) in
> the
> >      * set, and adding up the results.
>
>