mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 08:11:25 +00:00
Issue#475 Review suggestions
This commit is contained in:
Kamil Gabaydullin
committed by
Kamil-Gabaydullin
Kamil-Gabaydullin
parent
26a04b9b6d
commit
63d351c54a
@@ -3,6 +3,7 @@ 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 com.google.errorprone.matchers.Matchers.staticMethod;
|
||||
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;
|
||||
@@ -15,14 +16,16 @@ 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.matchers.Matcher;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ExpressionTree;
|
||||
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;
|
||||
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
|
||||
|
||||
/**
|
||||
* A {@link BugChecker} that flags multiple usages of {@link Mockito#verifyNoInteractions} in favor
|
||||
@@ -42,14 +45,16 @@ import org.mockito.Mockito;
|
||||
public final class MockitoVerifyNoInteractionsUsage extends BugChecker
|
||||
implements MethodTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final Matcher<ExpressionTree> VERIFY_NO_INTERACTIONS =
|
||||
staticMethod().onClass("org.mockito.Mockito").named("verifyNoInteractions");
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
boolean isTestMethod = TEST_METHOD.matches(tree, state);
|
||||
if (!isTestMethod) {
|
||||
if (!TEST_METHOD.matches(tree, state)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
var verifyNoInteractionsInvocations = getVerifyNoInteractionsInvocations(tree);
|
||||
ImmutableList<MethodInvocationTree> verifyNoInteractionsInvocations =
|
||||
getVerifyNoInteractionsInvocations(tree, state);
|
||||
if (verifyNoInteractionsInvocations.size() < 2) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
@@ -72,21 +77,26 @@ public final class MockitoVerifyNoInteractionsUsage extends BugChecker
|
||||
"");
|
||||
}
|
||||
});
|
||||
fixBuilder.replace(lastInvocation, "verifyNoInteractions(" + combinedArgument + ")");
|
||||
|
||||
String callAsString = SourceCode.treeToString(lastInvocation, state);
|
||||
fixBuilder.replace(
|
||||
lastInvocation,
|
||||
callAsString.startsWith("Mockito.")
|
||||
? "Mockito.verifyNoInteractions(" + combinedArgument + ")"
|
||||
: "verifyNoInteractions(" + combinedArgument + ")");
|
||||
|
||||
return describeMatch(tree, fixBuilder.build());
|
||||
}
|
||||
|
||||
private static ImmutableList<MethodInvocationTree> getVerifyNoInteractionsInvocations(
|
||||
MethodTree methodTree) {
|
||||
MethodTree methodTree, VisitorState state) {
|
||||
ImmutableList.Builder<MethodInvocationTree> 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")) {
|
||||
if (VERIFY_NO_INTERACTIONS.matches(node, state)) {
|
||||
invocationTreeBuilder.add(node);
|
||||
}
|
||||
return super.visitMethodInvocation(node, unused);
|
||||
|
||||
@@ -15,8 +15,10 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
CompilationTestHelper.newInstance(MockitoVerifyNoInteractionsUsage.class, getClass())
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.verifyNoInteractions;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"",
|
||||
@@ -25,43 +27,118 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
" void a() {}",
|
||||
" @Test",
|
||||
" void b() {",
|
||||
" Runnable runnable = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable);",
|
||||
" Object mock = mock(Object.class);",
|
||||
" verifyNoInteractions(mock);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void c() {",
|
||||
" Runnable runnable = mock(Runnable.class);",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable, runnable1);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1, mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" void d() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable1);",
|
||||
" verifyNoInteractions(runnable2);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" void e() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable1);",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable2);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @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);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock2, mock3);",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock4);",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.addSourceLines(
|
||||
"B.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"import org.mockito.Mockito;",
|
||||
"",
|
||||
"class B {",
|
||||
" private static void verifyNoInteractions(Object... objects) {}",
|
||||
"",
|
||||
" @Test",
|
||||
" void a() {",
|
||||
" Object mock = mock(Object.class);",
|
||||
" verifyNoInteractions(mock);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void b() {",
|
||||
" Object mock = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void c() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1, mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void d() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void e() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" void f() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" Mockito.verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" void g() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock1);",
|
||||
" Mockito.verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
" @Test",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" void h() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock2, mock3);",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock4);",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
@@ -80,10 +157,10 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable1);",
|
||||
" verifyNoInteractions(runnable2);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
@@ -96,10 +173,10 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
"",
|
||||
" verifyNoInteractions(runnable1, runnable2);",
|
||||
" verifyNoInteractions(mock1, mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
@@ -118,10 +195,10 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable1);",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable2);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
@@ -134,58 +211,164 @@ final class MockitoVerifyNoInteractionsUsageTest {
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
"",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" verifyNoInteractions(runnable1, runnable2);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock1, mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
|
||||
@Test
|
||||
void replaceManyCalls() {
|
||||
void replaceComplexCalls() {
|
||||
refactoringTestHelper
|
||||
.addInputLines(
|
||||
"A.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.verifyNoInteractions;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
"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);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock2, mock3);",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
" verifyNoInteractions(mock4);",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
"A.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.verifyNoInteractions;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"",
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
"",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
"",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock2, mock3, mock4, mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
|
||||
@Test
|
||||
void replaceStaticCalls() {
|
||||
refactoringTestHelper
|
||||
.addInputLines(
|
||||
"A.java",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"import org.mockito.Mockito;",
|
||||
"",
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock1);",
|
||||
" Mockito.verifyNoInteractions(mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
"A.java",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.verifyNoInteractions;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"import org.mockito.Mockito;",
|
||||
"",
|
||||
"class A {",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Runnable runnable1 = mock(Runnable.class);",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
"",
|
||||
" Runnable runnable2 = mock(Runnable.class);",
|
||||
" Runnable runnable3 = mock(Runnable.class);",
|
||||
" Mockito.verifyNoInteractions(mock1, mock2);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
|
||||
@Test
|
||||
void replaceComplexStaticCalls() {
|
||||
refactoringTestHelper
|
||||
.addInputLines(
|
||||
"A.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
" Runnable runnable4 = mock(Runnable.class);",
|
||||
" Runnable runnable5 = mock(Runnable.class);",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"import org.mockito.Mockito;",
|
||||
"",
|
||||
" verifyNoInteractions(runnable1, runnable2, runnable3, runnable4, runnable5);",
|
||||
"class A {",
|
||||
" private static void verifyNoInteractions(Object... objects) {}",
|
||||
"",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock2, mock3);",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock4);",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
"A.java",
|
||||
"import static org.assertj.core.api.Assertions.assertThat;",
|
||||
"import static org.mockito.Mockito.mock;",
|
||||
"import static org.mockito.Mockito.when;",
|
||||
"",
|
||||
"import org.junit.jupiter.api.Test;",
|
||||
"import org.mockito.Mockito;",
|
||||
"",
|
||||
"class A {",
|
||||
" private static void verifyNoInteractions(Object... objects) {}",
|
||||
"",
|
||||
" @Test",
|
||||
" void m() {",
|
||||
" Object mock1 = mock(Object.class);",
|
||||
" Object mock2 = mock(Object.class);",
|
||||
" when(mock1.toString()).thenReturn(\"test\");",
|
||||
" Object mock3 = mock(Object.class);",
|
||||
"",
|
||||
" Object mock4 = mock(Object.class);",
|
||||
" String str = mock1.toString();",
|
||||
" Object mock5 = mock(Object.class);",
|
||||
" Mockito.verifyNoInteractions(mock2, mock3, mock4);",
|
||||
" assertThat(str).isEqualTo(\"test\");",
|
||||
" verifyNoInteractions(mock5);",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
|
||||
Reference in New Issue
Block a user