Skip to content

Commit

Permalink
Generalize CannotMockFinalMethod to cover static methods too.
Browse files Browse the repository at this point in the history
Flume of new findings: unknown commit

PiperOrigin-RevId: 499833012
  • Loading branch information
graememorgan authored and Error Prone Team committed Jan 5, 2023
1 parent a604633 commit 8d518bf
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static java.lang.String.format;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
Expand All @@ -32,10 +33,10 @@

/** A BugPattern; see the summary */
@BugPattern(
summary = "Mockito cannot mock final methods, and can't detect this at runtime",
altNames = {"MockitoBadFinalMethod"},
summary = "Mockito cannot mock final or static methods, and can't detect this at runtime",
altNames = {"MockitoBadFinalMethod", "CannotMockFinalMethod"},
severity = WARNING)
public final class CannotMockFinalMethod extends BugChecker implements MethodInvocationTreeMatcher {
public final class CannotMockMethod extends BugChecker implements MethodInvocationTreeMatcher {

private static final Matcher<ExpressionTree> WHEN =
staticMethod().onClass("org.mockito.Mockito").named("when");
Expand All @@ -60,6 +61,19 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
}

private Description describe(MethodInvocationTree tree, MethodSymbol methodSymbol) {
return (methodSymbol.flags() & Flags.FINAL) == 0 ? NO_MATCH : describeMatch(tree);
if (methodSymbol.isStatic()) {
return buildDescription(tree, "static");
}
if ((methodSymbol.flags() & Flags.FINAL) == Flags.FINAL) {
return buildDescription(tree, "final");
}
return NO_MATCH;
}

private Description buildDescription(MethodInvocationTree tree, String issue) {
return buildDescription(tree)
.setMessage(
format("Mockito cannot mock %s methods, and can't detect this at runtime", issue))
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import com.google.errorprone.bugpatterns.ByteBufferBackingArray;
import com.google.errorprone.bugpatterns.CacheLoaderNull;
import com.google.errorprone.bugpatterns.CannotMockFinalClass;
import com.google.errorprone.bugpatterns.CannotMockFinalMethod;
import com.google.errorprone.bugpatterns.CannotMockMethod;
import com.google.errorprone.bugpatterns.CanonicalDuration;
import com.google.errorprone.bugpatterns.CatchAndPrintStackTrace;
import com.google.errorprone.bugpatterns.CatchFail;
Expand Down Expand Up @@ -1050,7 +1050,7 @@ public static ScannerSupplier errorChecks() {
BuilderReturnThis.class,
CanIgnoreReturnValueSuggester.class,
CannotMockFinalClass.class,
CannotMockFinalMethod.class,
CannotMockMethod.class,
CatchingUnchecked.class,
CheckedExceptionNotThrown.class,
ClassName.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public final class CannotMockFinalMethodTest {
public final class CannotMockMethodTest {
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(CannotMockFinalMethod.class, getClass());
CompilationTestHelper.newInstance(CannotMockMethod.class, getClass());

@Test
public void whenCall_flagged() {
Expand All @@ -44,6 +44,24 @@ public void whenCall_flagged() {
.doTest();
}

@Test
public void whenCall_forStaticMethod_flagged() {
compilationHelper
.addSourceLines(
"Test.java",
"import static org.mockito.Mockito.when;",
"class Test {",
" static Integer foo() {",
" return 1;",
" }",
" void test() {",
" // BUG: Diagnostic contains:",
" when(foo());",
" }",
"}")
.doTest();
}

@Test
public void verifyCall_flagged() {
compilationHelper
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Mockito cannot mock `final` methods, and cannot tell at runtime that this is
attempted and fail with an error (as mocking `final` classes does).
Mockito cannot mock `final` or `static` methods, and cannot tell at runtime that
this is attempted and fail with an error (as mocking `final` classes does).

`when(mock.finalMethod())` will invoke the real implementation of `finalMethod`.
In some cases, this may wind up accidentally doing what's intended:
Expand Down

0 comments on commit 8d518bf

Please sign in to comment.