robtimus opened a new pull request #173:
URL: https://github.com/apache/commons-io/pull/173
Using `CloseShieldInputStream`, `CloseShieldOutputStream`,
`CloseShieldReader` or `CloseShieldWriter` currently makes it easier to let
resource leaks go undetected. That's because most IDEs are smart when it comes
to wrapping an `InputStream` into another `InputStream`, and likewise for
`OutputStream`, `Reader` and `Writer` - it will assume that the `close` method
delegates and therefore assumes that closing the wrapper is safe. For instance,
the following will not produce an IDE warning that the `FileInputStream` is
never closed:
try (InputStream inputStream = new CloseShieldInputStream(new
FileInputStream(file))) {
// use inputStream
}
This PR should make it easier for IDEs to detect such resource leaks by two
simple changes:
* Add a static method to do the wrapping. IDEs are not smart enough to
detect that these static methods simply wrap the given resource. (In fact, if
I'd make a copy of `Objects.requireNonNull`, my IDE would complain about that
too.)
* Deprecate the constructors so people are encouraged to use the static
methods. In version 3.x these can be made private.
Using a static import for the wrapper method, the above code snippet becomes
the following, and there is an IDE warning that the `FileInputStream` is never
closed:
try (InputStream inputStream = dontClose(new FileInputStream(file))) {
// use inputStream
}
I've tried to import and use all of the separate `dontClose` methods in one
class, and the compiler has no problems with that, so there will be no
ambiguity errors if you statically import more than one `dontClose` method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
