Bug in generics architecture: FileHandle<FileLocation> is not DataHandle<Location>

[Daniel-Alievsky]

This issue is connected with #468

Below is a very simple test, using SCIFIO TiffParser:

public class FileHandleGenericsBug {
    public static void main(String[] args) {
        if (args.length < 1) {
            System.out.println("Usage:");
            System.out.println("    " + FileHandleGenericsBug.class.getName() + " some_tiff_file");
            return;
        }
        final File file = new File(args[0]);

        Context context = new SCIFIO().getContext();
        TiffParser parser = new TiffParser(context, new FileLocation(file));
        DataHandle<Location> stream = parser.getStream();
        System.out.println("Successfully opened: " + stream.getLocation());

        BytesLocation bytesLocation = new BytesLocation(1000);
        stream.set(bytesLocation); // - crash!! java.lang.ClassCastException

        System.out.println("This operator will not be performed");
        context.close();
    }
}

TiffParser input stream, as well as some analogous streams in other classes, is declared as DataHandle<Location>. But really it is not a sub-type of DataHandle<Location>! In terms of Java generics, it is DataHandle<? extends Location>

Unfortunately, DataHandle is implemented as a container, that can store an object of the generic type, i.e. Location, alike in classes List, Set etc. In other words, it has a method
void set(Location loc);
It means that we can pass to this method any Location subclass, for example, BytesLocation. As a result, a simple call "stream.set(bytesLocation)" in the code above throws ClassCastException.

Of course, the compiler tries to warn about the problem, but you suppress the warning inside the method PluginInfo.loadClass:

	public Class<? extends PT> loadClass() throws InstantiableException {
		if (pluginClass == null) {
			try {
				final Class<?> c = Types.load(className, classLoader, false);
				@SuppressWarnings("unchecked")
				final Class<? extends PT> typedClass = (Class<? extends PT>) c;
				pluginClass = typedClass;
			}
			catch (final IllegalArgumentException exc) {
				throw new InstantiableException("Class not found: " + className, exc);
			}
		}

		return pluginClass;
	}

FileHandle class "supports" FileLocation, so, it is created without exceptions, though actually FileHandle is not DataHandle<Location>

Of course, the same problem appears when I try to create DataHandle<Location> with help of my own function:

    /**
     * Warning: you should never call {@link DataHandle#set(Object)} method of the returned result!
     */
    @SuppressWarnings("rawtypes, unchecked")
    public static DataHandle<Location> getFileHandle(FileLocation fileLocation) {
        Objects.requireNonNull(fileLocation, "Null fileLocation");
        FileHandle fileHandle = new FileHandle();
        fileHandle.set(fileLocation);
        return (DataHandle) fileHandle;
    }

The problem is essentially serious. Unfortunately, TiffParser, like many other classes based on DataHandle technique, declare the streams as DataHandle<Location>, and it is a public declaration (for example, result of TiffParser.getStream() method is used in TIFFFormat class). I'm afraid you cannot just to replace DataHandle<Location> with correct and safe DataHandle<? extends Location> - a lot of client will stop be compiled.

But, maybe, you can rework the class FileHandle (and BytesHandle, and other known implementations of DataHandle) to reduce the problem? I've tried to do this with FileHandle - see the attached file. Now it extends AbstractDataHandle<Location>, not AbstractDataHandle<FileLocation>. I've added "main" test method to illustrate an idea: now "set" method works with Location instead of FileLocation, but it checks the type of the argument itself. However, I don't know how to correctly change the method getType() - it seems it will become a problem...

In any case, please think over the problem. Maybe you will find better solution.

FileHandle.zip

[Daniel-Alievsky]

Also note: according common Java agreements, DataHandle<Location> public type of the method result seems to be better than DataHandle<? extends Location> If possible, we should prefer more simple generic types in API, and it is an argument in favor of reworking FileHandle to a subclass of AbstractDataHandle<Location>