Skip to content

Commit

Permalink
MockIllegalThrows: flag cases where Mockito is configured to throw …
Browse files Browse the repository at this point in the history
…checked exception types which it certainly can't.

PiperOrigin-RevId: 564692050
  • Loading branch information
graememorgan authored and Error Prone Team committed Jan 31, 2025
1 parent ec153f5 commit 3e22618
Show file tree
Hide file tree
Showing 3 changed files with 229 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright 2025 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.isCheckedExceptionType;
import static com.google.errorprone.util.ASTHelpers.isSameType;
import static java.lang.String.format;
import static java.util.stream.Collectors.joining;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.NewClassTree;

/** A BugPattern; see the summary */
@BugPattern(summary = "This exception can't be thrown by the mocked method.", severity = WARNING)
public final class MockIllegalThrows extends BugChecker implements MethodInvocationTreeMatcher {

private static final Matcher<ExpressionTree> WHEN =
staticMethod().onClass("org.mockito.Mockito").named("when");

// TODO(ghm): Consider covering doThrow as well, even if we weakly discourage it.
private static final Matcher<ExpressionTree> THEN_THROW =
instanceMethod().onDescendantOf("org.mockito.stubbing.OngoingStubbing").named("thenThrow");

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!THEN_THROW.matches(tree, state)) {
return NO_MATCH;
}
ExpressionTree exceptionTree = tree.getArguments().get(0);
var thrownType = getType(exceptionTree);
if (!isCheckedExceptionType(thrownType, state)) {
return NO_MATCH;
}
// Heuristic: if the type being thrown is Exception/Throwable, but doesn't come directly from a
// constructor, it might be a parameter, and we can't know that it's not always given sensible
// types.
if (!(exceptionTree instanceof NewClassTree)
&& (isSameType(thrownType, state.getSymtab().exceptionType, state)
|| isSameType(thrownType, state.getSymtab().throwableType, state))) {
return NO_MATCH;
}
for (var receiver = getReceiver(tree);
receiver instanceof MethodInvocationTree whenMit;
receiver = getReceiver(receiver)) {
if (WHEN.matches(receiver, state)
&& whenMit.getArguments().get(0) instanceof MethodInvocationTree mit
&& getType(mit.getMethodSelect()).getThrownTypes().stream()
.noneMatch(
throwableType -> state.getTypes().isAssignable(thrownType, throwableType))) {
var thrownTypes = getType(mit.getMethodSelect()).getThrownTypes();
return buildDescription(whenMit.getArguments().get(0))
.setMessage(
thrownTypes.isEmpty()
? format(
"%s is not throwable by this method; only unchecked exceptions can be"
+ " thrown.",
thrownType.tsym.getSimpleName())
: format(
"%s is not throwable by this method; possible exception types are %s, or"
+ " any unchecked exception.",
thrownType.tsym.getSimpleName(),
thrownTypes.stream()
.map(t -> t.tsym.getSimpleName().toString())
.collect(joining(", "))))
.build();
}
}
return NO_MATCH;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@
import com.google.errorprone.bugpatterns.MixedArrayDimensions;
import com.google.errorprone.bugpatterns.MixedDescriptors;
import com.google.errorprone.bugpatterns.MixedMutabilityReturnType;
import com.google.errorprone.bugpatterns.MockIllegalThrows;
import com.google.errorprone.bugpatterns.MockNotUsedInProduction;
import com.google.errorprone.bugpatterns.MockitoDoSetup;
import com.google.errorprone.bugpatterns.MockitoUsage;
Expand Down Expand Up @@ -1021,6 +1022,7 @@ public static ScannerSupplier warningChecks() {
MissingRefasterAnnotation.class,
MissingSummary.class,
MixedMutabilityReturnType.class,
MockIllegalThrows.class,
MockNotUsedInProduction.class,
ModifiedButNotUsed.class,
ModifyCollectionInEnhancedForLoop.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* Copyright 2025 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns;

import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public final class MockIllegalThrowsTest {
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(MockIllegalThrows.class, getClass());

@Test
public void positive() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import static org.mockito.Mockito.when;
abstract class Test {
abstract Object foo();
void test(Test t) {
// BUG: Diagnostic contains: only unchecked
when(t.foo()).thenThrow(new Exception());
}
}
""")
.doTest();
}

@Test
public void positiveWithSpecificType() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import static org.mockito.Mockito.when;
abstract class Test {
static class SpecificException extends Exception {}
abstract Object foo() throws SpecificException;
void test(Test t) throws Exception {
// BUG: Diagnostic contains: are SpecificException, or any unchecked
when(t.foo()).thenThrow(new Exception());
}
}
""")
.doTest();
}

@Test
public void negative_exceptionTypeViaParameter() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import static org.mockito.Mockito.when;
abstract class Test {
static class SpecificException extends Exception {}
abstract Object foo() throws SpecificException;
void test(Test t, Exception e) throws Exception {
when(t.foo()).thenThrow(e);
}
}
""")
.doTest();
}

@Test
public void negative() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import static org.mockito.Mockito.when;
abstract class Test {
abstract Object foo() throws Exception;
void test(Test t) throws Exception {
when(t.foo()).thenThrow(new Exception());
}
}
""")
.doTest();
}

@Test
public void genericException() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import static org.mockito.Mockito.when;
abstract class Test {
interface GenericException<E extends Exception> {
Object execute() throws E;
}
void test(GenericException<Exception> ge) throws Exception {
when(ge.execute()).thenThrow(new Exception());
}
}
""")
.doTest();
}
}

0 comments on commit 3e22618

Please sign in to comment.