diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsage.java new file mode 100644 index 0000000000..60cae372eb --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsage.java @@ -0,0 +1,98 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static java.util.stream.Collectors.joining; +import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; +import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD; + +import com.google.auto.service.AutoService; +import com.google.common.collect.ImmutableList; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.util.TreeScanner; +import com.sun.tools.javac.code.Symbol; +import java.util.List; +import org.jspecify.annotations.Nullable; +import org.mockito.Mockito; + +/** + * A {@link BugChecker} that flags multiple usages of {@link Mockito#verifyNoInteractions} in favor + * of one call with varargs. + * + *

Multiple calls of {@link Mockito#verifyNoInteractions} can make the code more verbose than + * necessary. Instead of multiple calls, because {@link Mockito#verifyNoInteractions} accepts + * varargs, one call should be preferred. + */ +@AutoService(BugChecker.class) +@BugPattern( + summary = "Prefer one call to `verifyNoInteractions(varargs...)` over multiple calls", + link = BUG_PATTERNS_BASE_URL + "MockitoVerifyNoInteractionsUsage", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class MockitoVerifyNoInteractionsUsage extends BugChecker + implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + boolean isTestMethod = TEST_METHOD.matches(tree, state); + if (!isTestMethod) { + return Description.NO_MATCH; + } + var verifyNoInteractionsInvocations = getVerifyNoInteractionsInvocations(tree); + if (verifyNoInteractionsInvocations.size() < 2) { + return Description.NO_MATCH; + } + String combinedArgument = + verifyNoInteractionsInvocations.stream() + .map(MethodInvocationTree::getArguments) + .flatMap(List::stream) + .map(Object::toString) + .collect(joining(", ")); + + SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); + MethodInvocationTree lastInvocation = + verifyNoInteractionsInvocations.get(verifyNoInteractionsInvocations.size() - 1); + verifyNoInteractionsInvocations.forEach( + invocationTree -> { + if (!invocationTree.equals(lastInvocation)) { + fixBuilder.replace( + ASTHelpers.getStartPosition(invocationTree), + state.getEndPosition(invocationTree) + 1, + ""); + } + }); + fixBuilder.replace(lastInvocation, "verifyNoInteractions(" + combinedArgument + ")"); + + return describeMatch(tree, fixBuilder.build()); + } + + private static ImmutableList getVerifyNoInteractionsInvocations( + MethodTree methodTree) { + ImmutableList.Builder invocationTreeBuilder = ImmutableList.builder(); + + new TreeScanner<@Nullable Void, @Nullable Void>() { + @Override + public @Nullable Void visitMethodInvocation( + MethodInvocationTree node, @Nullable Void unused) { + Symbol symbol = ASTHelpers.getSymbol(node); + if (symbol.getSimpleName().toString().equals("verifyNoInteractions")) { + invocationTreeBuilder.add(node); + } + return super.visitMethodInvocation(node, unused); + } + }.scan(methodTree, null); + + return invocationTreeBuilder.build(); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsageTest.java new file mode 100644 index 0000000000..7aaacd20b9 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoVerifyNoInteractionsUsageTest.java @@ -0,0 +1,193 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class MockitoVerifyNoInteractionsUsageTest { + private final BugCheckerRefactoringTestHelper refactoringTestHelper = + BugCheckerRefactoringTestHelper.newInstance( + MockitoVerifyNoInteractionsUsage.class, getClass()); + + @Test + void identification() { + CompilationTestHelper.newInstance(MockitoVerifyNoInteractionsUsage.class, getClass()) + .addSourceLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void a() {}", + " @Test", + " void b() {", + " Runnable runnable = mock(Runnable.class);", + " verifyNoInteractions(runnable);", + " }", + " @Test", + " void c() {", + " Runnable runnable = mock(Runnable.class);", + " Runnable runnable1 = mock(Runnable.class);", + " verifyNoInteractions(runnable, runnable1);", + " }", + " @Test", + " // BUG: Diagnostic contains:", + " void d() {", + " Runnable runnable1 = mock(Runnable.class);", + " Runnable runnable2 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " verifyNoInteractions(runnable2);", + " }", + " @Test", + " // BUG: Diagnostic contains:", + " void e() {", + " Runnable runnable1 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " Runnable runnable2 = mock(Runnable.class);", + " verifyNoInteractions(runnable2);", + " }", + " @Test", + " // BUG: Diagnostic contains:", + " void f() {", + " Runnable runnable1 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " Runnable runnable2 = mock(Runnable.class);", + " Runnable runnable3 = mock(Runnable.class);", + " verifyNoInteractions(runnable2, runnable3);", + " Runnable runnable4 = mock(Runnable.class);", + " Runnable runnable5 = mock(Runnable.class);", + " verifyNoInteractions(runnable4);", + " verifyNoInteractions(runnable5);", + " }", + "}") + .doTest(); + } + + @Test + void replaceSequentialCalls() { + refactoringTestHelper + .addInputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + " Runnable runnable2 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " verifyNoInteractions(runnable2);", + " }", + "}") + .addOutputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + " Runnable runnable2 = mock(Runnable.class);", + "", + " verifyNoInteractions(runnable1, runnable2);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } + + @Test + void replaceNonSequentialCalls() { + refactoringTestHelper + .addInputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " Runnable runnable2 = mock(Runnable.class);", + " verifyNoInteractions(runnable2);", + " }", + "}") + .addOutputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + "", + " Runnable runnable2 = mock(Runnable.class);", + " verifyNoInteractions(runnable1, runnable2);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } + + @Test + void replaceManyCalls() { + refactoringTestHelper + .addInputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + " verifyNoInteractions(runnable1);", + " Runnable runnable2 = mock(Runnable.class);", + " Runnable runnable3 = mock(Runnable.class);", + " verifyNoInteractions(runnable2, runnable3);", + " Runnable runnable4 = mock(Runnable.class);", + " Runnable runnable5 = mock(Runnable.class);", + " verifyNoInteractions(runnable4);", + " verifyNoInteractions(runnable5);", + " }", + "}") + .addOutputLines( + "A.java", + "import static org.mockito.Mockito.mock;", + "import static org.mockito.Mockito.verifyNoInteractions;", + "", + "import org.junit.jupiter.api.Test;", + "", + "class A {", + " @Test", + " void m() {", + " Runnable runnable1 = mock(Runnable.class);", + "", + " Runnable runnable2 = mock(Runnable.class);", + " Runnable runnable3 = mock(Runnable.class);", + "", + " Runnable runnable4 = mock(Runnable.class);", + " Runnable runnable5 = mock(Runnable.class);", + "", + " verifyNoInteractions(runnable1, runnable2, runnable3, runnable4, runnable5);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +}