Skip to content

Commit 98dfcaf

Browse files
cushonError Prone Team
authored and
Error Prone Team
committed
Scan try-with-resources blocks
PiperOrigin-RevId: 450190698
1 parent be243a1 commit 98dfcaf

File tree

3 files changed

+20
-14
lines changed

3 files changed

+20
-14
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,13 @@ public class GuardedByChecker extends BugChecker
5656
private final GuardedByFlags flags = GuardedByFlags.allOn();
5757

5858
private final boolean reportMissingGuards;
59+
private final boolean checkTryWithResources;
5960

6061
public GuardedByChecker(ErrorProneFlags errorProneFlags) {
6162
reportMissingGuards =
6263
errorProneFlags.getBoolean("GuardedByChecker:reportMissingGuards").orElse(true);
64+
checkTryWithResources =
65+
errorProneFlags.getBoolean("GuardedByChecker:checkTryWithResources").orElse(true);
6366
}
6467

6568
@Override
@@ -87,7 +90,8 @@ private void analyze(VisitorState state) {
8790
report(GuardedByChecker.this.checkGuardedAccess(tree, guard, live, state), state),
8891
tree1 -> isSuppressed(tree1, state),
8992
flags,
90-
reportMissingGuards);
93+
reportMissingGuards,
94+
checkTryWithResources);
9195
}
9296

9397
@Override

core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/HeldLockAnalyzer.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,12 @@ public static void analyze(
8686
LockEventListener listener,
8787
Predicate<Tree> isSuppressed,
8888
GuardedByFlags flags,
89-
boolean reportMissingGuards) {
89+
boolean reportMissingGuards,
90+
boolean checkTryWithResources) {
9091
HeldLockSet locks = HeldLockSet.empty();
9192
locks = handleMonitorGuards(state, locks, flags);
92-
new LockScanner(state, listener, isSuppressed, flags, reportMissingGuards)
93+
new LockScanner(
94+
state, listener, isSuppressed, flags, reportMissingGuards, checkTryWithResources)
9395
.scan(state.getPath(), locks);
9496
}
9597

@@ -126,6 +128,7 @@ private static class LockScanner extends TreePathScanner<Void, HeldLockSet> {
126128
private final Predicate<Tree> isSuppressed;
127129
private final GuardedByFlags flags;
128130
private final boolean reportMissingGuards;
131+
private final boolean checkTryWithResources;
129132

130133
private static final GuardedByExpression.Factory F = new GuardedByExpression.Factory();
131134

@@ -134,12 +137,14 @@ private LockScanner(
134137
LockEventListener listener,
135138
Predicate<Tree> isSuppressed,
136139
GuardedByFlags flags,
137-
boolean reportMissingGuards) {
140+
boolean reportMissingGuards,
141+
boolean checkTryWithResources) {
138142
this.visitorState = visitorState;
139143
this.listener = listener;
140144
this.isSuppressed = isSuppressed;
141145
this.flags = flags;
142146
this.reportMissingGuards = reportMissingGuards;
147+
this.checkTryWithResources = checkTryWithResources;
143148
}
144149

145150
@Override
@@ -182,12 +187,11 @@ public Void visitTry(TryTree tree, HeldLockSet locks) {
182187
// are held for the entirety of the try and catch statements.
183188
Collection<GuardedByExpression> releasedLocks =
184189
ReleasedLockFinder.find(tree.getFinallyBlock(), visitorState, flags);
185-
if (resources.isEmpty()) {
190+
// We don't know what to do with the try-with-resources block.
191+
// TODO(cushon) - recognize common try-with-resources patterns. Currently there is no
192+
// standard implementation of an AutoCloseable lock resource to detect.
193+
if (checkTryWithResources || resources.isEmpty()) {
186194
scan(tree.getBlock(), locks.plusAll(releasedLocks));
187-
} else {
188-
// We don't know what to do with the try-with-resources block.
189-
// TODO(cushon) - recognize common try-with-resources patterns. Currently there is no
190-
// standard implementation of an AutoCloseable lock resource to detect.
191195
}
192196
scan(tree.getCatches(), locks.plusAll(releasedLocks));
193197
scan(tree.getFinallyBlock(), locks);

core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByCheckerTest.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,11 +1043,8 @@ public void tryWithResources() {
10431043
.doTest();
10441044
}
10451045

1046-
// Test that the contents of try-with-resources block are ignored (for now), but the catch and
1047-
// finally blocks are checked.
1048-
// TODO(cushon): support try-with-resources block.
10491046
@Test
1050-
public void tryWithResourcesAreNotFullyUnsupported() {
1047+
public void tryWithResources_resourceVariables() {
10511048
compilationHelper
10521049
.addSourceLines(
10531050
"threadsafety/Test.java",
@@ -1060,7 +1057,8 @@ public void tryWithResourcesAreNotFullyUnsupported() {
10601057
" int x;",
10611058
" void m(AutoCloseable c) throws Exception {",
10621059
" try (AutoCloseable unused = c) {",
1063-
" x++; // should be an error!",
1060+
" // BUG: Diagnostic contains:",
1061+
" x++;",
10641062
" } catch (Exception e) {",
10651063
" // BUG: Diagnostic contains:",
10661064
" // should be guarded by 'this.lock'",

0 commit comments

Comments
 (0)