-
Notifications
You must be signed in to change notification settings - Fork 781
Description
Description of the problem / feature request:
Usage of Path.toFile() should be introduced as WARNING due to frequent misunderstandings of its behaviour.
Feature requests: what underlying problem are you trying to solve with this feature?
In code review I often find code where a Path has been passed in, and they want to pass it to an API which only accepts File, and the developer has thought Path.toFile() to be a convenient conversion.
The reality is that Path.toFile() breaks the abstraction of the Path API, defeating most of the purpose of using it. If we start using an in-memory filesystem to provide Path objects in order to speed up tests, calls to toFile() will throw UnsupportedOperationException, and code failing to handle this in a sensible fashion will just break.
So it would be nice if calls to Path.toFile() were treated as WARNING level.
Alternatively, perhaps it could be ERROR if an escape hatch were to be used?
For example, this is bad:
public ArchiveFile openArchive(Path file) {
return new ArchiveFile(file.toFile());
}This is okay because createTempFile is documented to return a file on the default filesystem:
public ArchiveFile openArchive(Path file) {
Path copy = Files.createTempFile();
Files.copy(file, copy);
return new ArchiveFile(copy.toFile());
}This is what you'd probably do if you wanted the best of both worlds:
public ArchiveFile openArchive(Path file) {
try {
return new ArchiveFile(file.toPath());
} catch (UnsupportedOperationException e) {
// Expected situation of the file not being on the default filesystem
Path copy = Files.createTempFile();
Files.copy(file, copy);
return new ArchiveFile(copy.toFile());
}
}(Though both of these carry an interesting question of whose job it is to clean up that temp file.)
Or perhaps this is OK in some situation, though I'm not that fond of it:
public ArchiveFile openArchive(Path file) {
try {
return new ArchiveFile(file.toFile());
} catch (UnsupportedOperationException e) {
throw new IllegalArgumentException("You were supposed to copy the file to temp first", e);
}
}