Sonar should flag calls to Path.toFile()

Path.toFile() breaks the entire abstraction of the Path API and can throw UnsupportedOperationException. Code calling toFile() without catching and appropriately handling UnsupportedOperationException is buggy, which seems like a fairly simple check to add.

Sadly some of Sonar’s checks are recommending calling this method as well, so as part of adding this, alternatives should be found for those.

hello @trejkaz,

I agree with you that when using Path API, it is preferred to use methods designed for this API.

You are not mentioning which checks recommend such code, I am only aware of this one https://rules.sonarsource.com/java/RSPEC-3725, there is a very particular reason for this rule, it tries to avoid two JDK bugs https://bugs.openjdk.java.net/browse/JDK-8153414 and https://bugs.openjdk.java.net/browse/JDK-8154077. We designed this rule because we encountered these bugs in production.
This rule will only raise an issue when your Java version is set to 8 (which is not necessarily the runtime version the code is running on, but it’s the best approximation of that we found).

However, I think that now the rule should be removed from the default quality profile, as Java 8 is quite old and many people use Java 11 at runtime. This will be changed in the next release.

1 Like

Don’t get me wrong, the check itself is almost correct.

Files.exists is an evil method. Not because of the performance issue mentioned on Java 8 (since fixed), but because it silently returns false if it gets an exception. Files.isRegularFile, Files.isDirectory, and anything along the same lines are similarly guilty. The correct course of action is actually to call Files.readAttributes, which properly throws the exception.

What I had hoped to draw attention to is that in avoiding calling one bad thing, the recommended course of action was in fact to call two bad things:

  1. Path#toFile() breaks abstraction and should be banned unless the code is shown to properly handle the UnsupportedOperationException.

  2. File#exists() is evil on the same basis, that it does not throw IOException, and thus should be avoided for the same reason as Files#exists.

Ah, yes. And of course, I almost forgot. A footnote that file existence checks in themselves are inherently race conditions, in that the information is out of date as soon as you get it.