RFR: 8312089: Simplify and modernize equals, hashCode, and compareTo in java.nio and implementation code [v2]

[Pavel Rappo]

On Fri, 14 Jul 2023 13:16:27 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Please review this PR to use modern APIs and language features to simplify equals, hashCode, and compareTo for in java.nio and implementation code.
>> 
>> Please note, test results are pending.
>> 
>> Additional notes:
>> 
>> * This PR saves a volatile read in java.nio.file.attribute.AclEntry.hashCode. Not that it's too important, but worth noting because of rearrangements.
>> 
>> * java.nio.charset.Charset#compareTo seems **inconsistent** with equals. If so, I cannot see where that inconsistency is specified.
>> 
>> * Is this a **bug** in sun.nio.ch.FileKey#hashCode? Tell me if not, I'll revert it.
>> 
>> * This PR simplifies the tail of java.nio.file.attribute.FileTime.compareTo. Unless I'm missing something, that comment in source above the affected lines **seems** not to prohibit such a simplification.
>> 
>> * sun.nio.fs.UnixFileStore#hashCode does not include entry.name(). While it's not wrong, I wonder if it was on purpose.
>> 
>> * Despite its title, this PR also and opportunistically refactors sun.nio.fs.UnixPath.endsWith.
>
> Pavel Rappo has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Address another case from feedback
>  - Address feedback

> * java.nio.charset.Charset#compareTo seems **inconsistent** with equals. If so, I cannot see where that inconsistency is specified.

@AlanBateman, what do you think of that? For convenience, let me paste here relevant source parts:

    /**
       ...
     * <p> Every charset has a <i>canonical name</i> and may also have one or more
     * <i>aliases</i>.  The canonical name is returned by the {@link #name() name} method
     * of this class.  Canonical names are, by convention, usually in upper case.
     * The aliases of a charset are returned by the {@link #aliases() aliases}
     * method.
       ...
     */
    public abstract class Charset implements Comparable<Charset>
    {
        ...
      
        /**
         * Compares this charset to another.
         *
         * <p> Charsets are ordered by their canonical names, without regard to
         * case.
           ...
         */
        @Override
        public final int compareTo(Charset that) {
            return (name().compareToIgnoreCase(that.name()));
        }
    
        ...
        
        /**
         * Tells whether or not this object is equal to another.
         *
         * <p> Two charsets are equal if, and only if, they have the same canonical
         * names.  A charset is never equal to any other type of object.  </p>
         *
         * @return  {@code true} if, and only if, this charset is equal to the
         *          given object
         */
        @Override
        public final boolean equals(Object ob) {
            if (this == ob)
                return true;
            return ob instanceof Charset other && name.equals(other.name());
        }
    
        ...
    }

Here's what Comparable has to say about this:

    public interface Comparable<T> {
        /**
           ...
         * @apiNote
         * It is strongly recommended, but <i>not</i> strictly required that
         * {@code (x.compareTo(y)==0) == (x.equals(y))}.  Generally speaking, any
         * class that implements the {@code Comparable} interface and violates
         * this condition should clearly indicate this fact.  The recommended
         * language is "Note: this class has a natural ordering that is
         * inconsistent with equals."
           ...
         */
        public int compareTo(T o);

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14886#issuecomment-1635967137