Scan try-with-resources blocks

PiperOrigin-RevId: 450190698
google · May 21, 2022 · 98dfcaf · 98dfcaf
1 parent be243a1
commit 98dfcaf
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 14 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java
@@ -56,10 +56,13 @@ public class GuardedByChecker extends BugChecker
   private final GuardedByFlags flags = GuardedByFlags.allOn();
 
   private final boolean reportMissingGuards;
+  private final boolean checkTryWithResources;
 
   public GuardedByChecker(ErrorProneFlags errorProneFlags) {
     reportMissingGuards =
         errorProneFlags.getBoolean("GuardedByChecker:reportMissingGuards").orElse(true);
+    checkTryWithResources =
+        errorProneFlags.getBoolean("GuardedByChecker:checkTryWithResources").orElse(true);
   }
 
   @Override
@@ -87,7 +90,8 @@ private void analyze(VisitorState state) {
             report(GuardedByChecker.this.checkGuardedAccess(tree, guard, live, state), state),
         tree1 -> isSuppressed(tree1, state),
         flags,
-        reportMissingGuards);
+        reportMissingGuards,
+        checkTryWithResources);
   }
 
   @Override

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/HeldLockAnalyzer.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/HeldLockAnalyzer.java
@@ -86,10 +86,12 @@ public static void analyze(
       LockEventListener listener,
       Predicate<Tree> isSuppressed,
       GuardedByFlags flags,
-      boolean reportMissingGuards) {
+      boolean reportMissingGuards,
+      boolean checkTryWithResources) {
     HeldLockSet locks = HeldLockSet.empty();
     locks = handleMonitorGuards(state, locks, flags);
-    new LockScanner(state, listener, isSuppressed, flags, reportMissingGuards)
+    new LockScanner(
+            state, listener, isSuppressed, flags, reportMissingGuards, checkTryWithResources)
         .scan(state.getPath(), locks);
   }
 
@@ -126,6 +128,7 @@ private static class LockScanner extends TreePathScanner<Void, HeldLockSet> {
     private final Predicate<Tree> isSuppressed;
     private final GuardedByFlags flags;
     private final boolean reportMissingGuards;
+    private final boolean checkTryWithResources;
 
     private static final GuardedByExpression.Factory F = new GuardedByExpression.Factory();
 
@@ -134,12 +137,14 @@ private LockScanner(
         LockEventListener listener,
         Predicate<Tree> isSuppressed,
         GuardedByFlags flags,
-        boolean reportMissingGuards) {
+        boolean reportMissingGuards,
+        boolean checkTryWithResources) {
       this.visitorState = visitorState;
       this.listener = listener;
       this.isSuppressed = isSuppressed;
       this.flags = flags;
       this.reportMissingGuards = reportMissingGuards;
+      this.checkTryWithResources = checkTryWithResources;
     }
 
     @Override
@@ -182,12 +187,11 @@ public Void visitTry(TryTree tree, HeldLockSet locks) {
       // are held for the entirety of the try and catch statements.
       Collection<GuardedByExpression> releasedLocks =
           ReleasedLockFinder.find(tree.getFinallyBlock(), visitorState, flags);
-      if (resources.isEmpty()) {
+      // We don't know what to do with the try-with-resources block.
+      // TODO(cushon) - recognize common try-with-resources patterns. Currently there is no
+      // standard implementation of an AutoCloseable lock resource to detect.
+      if (checkTryWithResources || resources.isEmpty()) {
         scan(tree.getBlock(), locks.plusAll(releasedLocks));
-      } else {
-        // We don't know what to do with the try-with-resources block.
-        // TODO(cushon) - recognize common try-with-resources patterns. Currently there is no
-        // standard implementation of an AutoCloseable lock resource to detect.
       }
       scan(tree.getCatches(), locks.plusAll(releasedLocks));
       scan(tree.getFinallyBlock(), locks);

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByCheckerTest.java
@@ -1043,11 +1043,8 @@ public void tryWithResources() {
         .doTest();
   }
 
-  // Test that the contents of try-with-resources block are ignored (for now), but the catch and
-  // finally blocks are checked.
-  // TODO(cushon): support try-with-resources block.
   @Test
-  public void tryWithResourcesAreNotFullyUnsupported() {
+  public void tryWithResources_resourceVariables() {
     compilationHelper
         .addSourceLines(
             "threadsafety/Test.java",
@@ -1060,7 +1057,8 @@ public void tryWithResourcesAreNotFullyUnsupported() {
             "  int x;",
             "  void m(AutoCloseable c) throws Exception {",
             "    try (AutoCloseable unused = c) {",
-            "      x++;  // should be an error!",
+            "      // BUG: Diagnostic contains:",
+            "      x++;",
             "    } catch (Exception e) {",
             "      // BUG: Diagnostic contains:",
             "      // should be guarded by 'this.lock'",