From 1ae734ce1b656c32d6cd80f27cc595f690396e35 Mon Sep 17 00:00:00 2001 From: bhagwani Date: Sun, 19 Jul 2020 15:25:23 -0700 Subject: [PATCH] Style guide allows empty catch blocks with clear parameter names setting up expectation. Exempting expected, ignored and ok parameter names. https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions Inverse flume results (matches that won't be flagged now) : unknown commit Fixes #1654 PiperOrigin-RevId: 322052035 --- .../errorprone/bugpatterns/EmptyCatch.java | 10 ++++++++ .../testdata/EmptyCatchNegativeCases.java | 23 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/EmptyCatch.java b/core/src/main/java/com/google/errorprone/bugpatterns/EmptyCatch.java index 8b6020a70d5..fd2e36751a7 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/EmptyCatch.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/EmptyCatch.java @@ -20,6 +20,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.CatchTreeMatcher; @@ -27,6 +28,7 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.BlockTree; import com.sun.source.tree.CatchTree; +import com.sun.source.tree.VariableTree; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern( @@ -39,6 +41,9 @@ ) public class EmptyCatch extends BugChecker implements CatchTreeMatcher { + private static final ImmutableSet EXEMPTED_PARAMETER_NAMES = + ImmutableSet.of("expected", "ok", "ignored"); + @Override public Description matchCatch(CatchTree tree, VisitorState state) { BlockTree block = tree.getBlock(); @@ -51,6 +56,11 @@ public Description matchCatch(CatchTree tree, VisitorState state) { if (ASTHelpers.isJUnitTestCode(state)) { return NO_MATCH; } + VariableTree param = tree.getParameter(); + if (EXEMPTED_PARAMETER_NAMES.stream() + .anyMatch(paramName -> param.getName().contentEquals(paramName))) { + return NO_MATCH; + } return describeMatch(tree); } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchNegativeCases.java index 57d7f72a656..4fa5ac8e1ae 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchNegativeCases.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/EmptyCatchNegativeCases.java @@ -126,11 +126,32 @@ public void catchIsLoggedOnly() { } @Test - public void expectedException() { + public void expectedExceptionInTest() { try { System.err.println(); fail(); } catch (Exception expected) { } } + + public void expectedException() { + try { + System.err.println(); + } catch (Exception expected) { + } + } + + public void ignoredException() { + try { + System.err.println(); + } catch (Exception ignored) { + } + } + + public void okException() { + try { + System.err.println(); + } catch (Exception ok) { + } + } }