JSR199 FileObjects don't obey general contract of equals.

[Jonathan Gibbons]

We have a problem.  Actually, we have two, one for RegularFileObject and 
one for ZipFileObject.

In JavacFileManager, RegularFileObject.equals does not object the 
general contract of equals. Here is the relevant code:

        @Override
        public boolean equals(Object other) {
            if (!(other instanceof RegularFileObject))
                return false;
            RegularFileObject o = (RegularFileObject) other;
            try {
                return f.equals(o.f)
                    || f.getCanonicalFile().equals(o.f.getCanonicalFile());
            } catch (IOException e) {
                return false;
            }
        }

        @Override
        public int hashCode() {
            return f.hashCode();
        }

The contract for Object.equals says that:
"Note that it is generally necessary to override the hashCode method 
whenever this method is overridden, so as to maintain the general 
contract for the hashCode method, which states that equal objects must 
have equal hash codes."

The problem is that getCanonicalFile will equate a relative file ("foo") 
with its absolute equivalent ("/home/jjg/foo"), even though they have 
different hashcodes.

There are two possible solutions. For maximum compatibility, hashCode() 
should use getCanonicalFile for the hashCode (and hope that you don't 
get an IOException.)
Arguably a better solution for both equals and hashCode would be to use 
getAbsoluteFile instead of getCanonical file, but that would potentially 
and subtly change the semantics so that symbolic links would not be 
considered equal.

 From a historical perspective, the getCanonicalFile code in 
RegularFileObject can be traced back to the need to do case-equivalent 
fillename matching on Windows. (See 1.5.0_10 ClassReader.java, line 1819 
and following.)  However, that code was already outdated, since 1.5.0_10 
Win32FileSystem.java shows that filename matching ignores case.



The other issue is with ZipFileObject, which is related but different.   
This has a novel and probably incorrect definition of equality, which 
also violates the general contract of equals. Here's the code from 
ZipFileObject.

        @Override
        public boolean equals(Object other) {
            if (!(other instanceof ZipFileObject))
                return false;
            ZipFileObject o = (ZipFileObject) other;
            return zdir.equals(o.zdir) || name.equals(o.name);
        }

        @Override
        public int hashCode() {
            return zdir.hashCode() + name.hashCode();
        }

The novelty is the '||', which means that two ZipFileObjects are the 
same if they share the identical same zip file container, or (yes, or) 
they have the same path within a zip file.   Even if you change the || 
to &&, we still have an additional but relatively minor issue -- that 
the condition for equality of zipfiles is inconsistent with the 
condition for equality of regular files. (ie. zip file equality doesn't 
compare paths and they don't get the same canonical treatment.)


So the proposal is the following
- fix ZipFileObject equality to use && not ||
- fix ZipFileObject to compare zip files the same way we compare regular 
files (whatever we decide that should be)
- consider using absolute paths, and not canonical paths for 
FileObject.equals, at a risk of a minor behavioral change
- there is already a method StandardFileManager.IsSameFile; this should 
use canonical paths to determine same-ness


-- Jon