Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Clean up out-of-scope references to auto-managed resource #564

Merged
merged 11 commits into from
Jun 11, 2021

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented Jun 9, 2021

Fix #561

This PR makes the UnclosedResourcesProcessor clean up references to the variable that was inlined into the resource block. Previously, such left-over references were not cleaned up, causing compile errors.

Here's an example of the new-and-improved repair, note how it removes the if in the catcher:

 public class ReferenceInCatcherAndFinalizerToFieldWithSameNameAsResource {
     private int bw = 2;

     public void saveTo(File file) throws IOException {
-        BufferedWriter bw = null;
-        try {
-            bw = new BufferedWriter(new FileWriter(file)); // Noncompliant
+        try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
             bw.write("Write some stuff to file");
         } catch (IOException e) {
             e.printStackTrace();
             System.out.println(this.bw); // reference to field; should not be cleaned!
-            if (bw != null) { // reference to local variable; should be cleaned!
-                bw.close(); // reference to local variable; should be cleaned!
-            }
         } finally {
             System.out.println(this.bw); // reference to field; should not be cleaned!
             System.out.println("done");

         }
     }
 }

It works by looking for references to the inlined variable inside the catchers and finalizer, and removing any statement containing a reference. It's possible that this could "over clean" for example by removing an if statement where the condition has a reference to the resource, but there's also unrelated stuff inside of the then or else blocks. But I think we'll deal with that only if we actually find it happening, as I've said before, this processor will never be perfect.

@slarse slarse added the WiP label Jun 9, 2021
@slarse slarse requested a review from fermadeiral June 11, 2021 06:32
@fermadeiral
Copy link
Collaborator

LGTM.

@fermadeiral fermadeiral merged commit 71d53df into master Jun 11, 2021
@fermadeiral fermadeiral deleted the issue/561-fix-double-resource-close branch June 11, 2021 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

UnclosedResourcesProcessor (rule 2095): Retaining finally block can cause "double close" of resources
2 participants