New error-prone rule: AutoCloseableMustBeClosed

[schlosna]

Before this PR

Methods and constructors of AutoCloseable instances are often not annotated with @MustBeClosed, so MustBeClosedChecker does not always do complete analysis to identify potential resource leaks.

After this PR

==COMMIT_MSG==
A new suggested error-prone rule AutoCloseableMustBeClosed annotates methods and constructors that return an AutoCloseable type as @MustBeClosed to allow for MustBeClosedChecker to perform analysis that resources are appropriately closed.

See https://errorprone.info/bugpattern/MustBeClosedChecker
==COMMIT_MSG==

Possible downsides?

For now this is just a SUGGESTION level to test out on several projects.

This may be noisy both in terms of additional annotations as well as requiring manual work to resolve findings flagged by MustBeClosedChecker (both valid and false positives where an AutoCloseable is passed through layers of abstractions).

One example that we may wish to exclude is the case of java.util.stream.Stream which implements AutoCloseable; however, in most cases it is undesirable to clutter stream usages and methods returning Stream with try-with-resources, and a limitation of Stream terminal . This is limitation is partially covered by the existing StreamResourceLeak checks.

[changelog-app]

Generate changelog in changelog/@unreleased

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

A new suggested error-prone rule AutoCloseableMustBeClosed annotates methods and constructors that return an AutoCloseable type as @MustBeClosed to allow for MustBeClosedChecker to perform analysis that resources are appropriately closed.

See https://errorprone.info/bugpattern/MustBeClosedChecker

Check the box to generate changelog(s)

• Generate changelog entry

[pkoenig10]

I generally dislike SUGGESTION because it clutters the compiler output without forcing me to fix it. Can we make this WARNING?

Or do we expect there are repos with -Werror that have a large number of occurrences that would require a lot of effort to resolve? In that case I'd prefer they just disable the check until they are ready to address it.

[carterkozak]

I think we want to roll this out to a few repos before hitting everything, we can either:

1. Cut an RC to try it out (next baseline release will revert the checck)
2. Disable the check by setting errorProneOptions.check("AutoCloseableMustBeClosed", CheckSeverity.OFF) in BaselineErrorProne.java
3. Just roll it out everywhere

I'm curious how it works with one large internal repo in particular, that normally catches the edge cases.

[schlosna]

Hey @pkoenig10 as @carterkozak proposed I was mainly thinking of starting it out as a SUGGESTION to collect signal/noise ratio on a few projects first, potentially refine exclusions similar to Stream, then when its high signal bump it up to WARNING so that projects that opt-in to -Werror can address findings.

[carterkozak]

Lgtm

[svc-autorelease]

Released 3.72.0