Formatter is now AutoCloseable, so that facilitates FormatterFunc.Closeable
works.
#284
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a long time, we've had
FormatterFunc.Closeable
in the codebase, but with no usages.spotless/lib/src/main/java/com/diffplug/spotless/FormatterFunc.java
Lines 22 to 43 in f622379
The idea was that a
FormatterFunc
with native integration might be holding native handles that would need to be released. Now that we're about to integrate with V8 thanks to #283, we need to make this dead code come to life. The good news is that the abstraction we laid down without a concrete usecase 2 years ago fits the need in #283 perfectly. Aside: terrible idea that we did this without a use-case, the git blame shows it's my fault :'(The bare minimum that we'll need to do is make
Formatter
into anAutoCloseable
, which then will cleanup all of itsFormatterStep
/FormatterFunc
.Formatter
is only used in a few places in our entire codebase (8 including tests), so it's an easy change.The next choice is whether
FormatterStep
should also beAutoCloseable
. This is problematic, because we use them willy-nilly all over the place. It's a very nice property to be able to use them willy-nilly.It's important to note that the only time a
FormatterStep
ever needs to be closed is if it has actually been applied, because until then itsFormatterFunc
has not been created. The intention of the library is that no one will callFormatterStep.format
exceptFormatter
, so lifting the resource-management burden toFormatter
is relatively safe.It might be tempting here to think "aha! We don't need SpotlessCache anymore!", which is the mechanism we use for caching ClassLoaders. However, it's important to note that we do this to share ClassLoaders across spotless runs so that the JIT can warmup (we get huge speed improvements compared to creating a destroying classloader for each run of Spotless). So this doesn't affect
ClassLoader
resources, just other kinds of resources.