Skip to content

Commit

Permalink
Introduce JUnitIncompatibleType, a JUnit analogue of TruthIncompatibl…
Browse files Browse the repository at this point in the history
…eType

PiperOrigin-RevId: 596279255
  • Loading branch information
graememorgan authored and Error Prone Team committed Jan 8, 2024
1 parent 654b1d9 commit 375ffde
Show file tree
Hide file tree
Showing 3 changed files with 240 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* Copyright 2024 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.collectionincompatibletype;

import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.getType;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.bugpatterns.TypeCompatibility;
import com.google.errorprone.bugpatterns.TypeCompatibility.TypeCompatibilityReport;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.Signatures;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.ParenthesizedTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TypeCastTree;
import com.sun.source.util.SimpleTreeVisitor;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.ArrayType;
import javax.inject.Inject;

/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
@BugPattern(summary = "The types passed to this assertion are incompatible.", severity = WARNING)
public final class JUnitIncompatibleType extends BugChecker implements MethodInvocationTreeMatcher {
private static final Matcher<ExpressionTree> ASSERT_EQUALS =
allOf(
staticMethod()
.onClassAny("org.junit.Assert", "junit.framework.Assert", "junit.framework.TestCase")
.namedAnyOf("assertEquals", "assertNotEquals"),
anyOf(
staticMethod()
.anyClass()
.withAnyName()
.withParameters("java.lang.Object", "java.lang.Object"),
staticMethod()
.anyClass()
.withAnyName()
.withParameters("java.lang.String", "java.lang.Object", "java.lang.Object")));

private static final Matcher<ExpressionTree> ASSERT_ARRAY_EQUALS =
staticMethod()
.onClassAny("org.junit.Assert", "junit.framework.Assert", "junit.framework.TestCase")
.namedAnyOf("assertArrayEquals");

private final TypeCompatibility typeCompatibility;

@Inject
JUnitIncompatibleType(TypeCompatibility typeCompatibility) {
this.typeCompatibility = typeCompatibility;
}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
var arguments = tree.getArguments();
if (ASSERT_EQUALS.matches(tree, state)) {
var typeA = getType(ignoringCasts(arguments.get(arguments.size() - 2)));
var typeB = getType(ignoringCasts(arguments.get(arguments.size() - 1)));
return checkCompatibility(tree, typeA, typeB, state);
} else if (ASSERT_ARRAY_EQUALS.matches(tree, state)) {
var typeA =
((ArrayType) getType(ignoringCasts(arguments.get(arguments.size() - 2)))).elemtype;
var typeB =
((ArrayType) getType(ignoringCasts(arguments.get(arguments.size() - 1)))).elemtype;
return checkCompatibility(tree, typeA, typeB, state);
}
return NO_MATCH;
}

private Description checkCompatibility(
ExpressionTree tree, Type targetType, Type sourceType, VisitorState state) {
TypeCompatibilityReport compatibilityReport =
typeCompatibility.compatibilityOfTypes(targetType, sourceType, state);
if (compatibilityReport.isCompatible()) {
return NO_MATCH;
}
String sourceTypeName = Signatures.prettyType(sourceType);
String targetTypeName = Signatures.prettyType(targetType);
// If the pretty names are the same, fall back to full names.
if (sourceTypeName.equals(targetTypeName)) {
sourceTypeName = sourceType.toString();
targetTypeName = targetType.toString();
}

return buildDescription(tree)
.setMessage(
String.format(
"The types of this assertion are mismatched: type `%s` is not compatible with `%s`"
+ compatibilityReport.extraReason(),
sourceTypeName,
targetTypeName))
.build();
}

private Tree ignoringCasts(Tree tree) {
return tree.accept(
new SimpleTreeVisitor<Tree, Void>() {
@Override
protected Tree defaultAction(Tree node, Void unused) {
return node;
}

@Override
public Tree visitTypeCast(TypeCastTree node, Void unused) {
return getType(node).isPrimitive() ? node : node.getExpression().accept(this, null);
}

@Override
public Tree visitParenthesized(ParenthesizedTree node, Void unused) {
return node.getExpression().accept(this, null);
}
},
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@
import com.google.errorprone.bugpatterns.collectionincompatibletype.CollectionUndefinedEquality;
import com.google.errorprone.bugpatterns.collectionincompatibletype.CompatibleWithMisuse;
import com.google.errorprone.bugpatterns.collectionincompatibletype.IncompatibleArgumentType;
import com.google.errorprone.bugpatterns.collectionincompatibletype.JUnitIncompatibleType;
import com.google.errorprone.bugpatterns.collectionincompatibletype.TruthIncompatibleType;
import com.google.errorprone.bugpatterns.flogger.FloggerArgumentToString;
import com.google.errorprone.bugpatterns.flogger.FloggerFormatString;
Expand Down Expand Up @@ -938,6 +939,7 @@ public static ScannerSupplier warningChecks() {
JUnit3FloatingPointComparisonWithoutDelta.class,
JUnit4ClassUsedInJUnit3.class,
JUnitAmbiguousTestClass.class,
JUnitIncompatibleType.class,
JavaDurationGetSecondsGetNano.class,
JavaDurationWithNanos.class,
JavaDurationWithSeconds.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright 2024 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.collectionincompatibletype;

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 JUnitIncompatibleTypeTest {

private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(JUnitIncompatibleType.class, getClass());

@Test
public void assertEquals_mismatched() {
compilationHelper
.addSourceLines(
"Test.java",
"import static org.junit.Assert.assertEquals;",
"import static org.junit.Assert.assertNotEquals;",
"class Test {",
" public void test() {",
" // BUG: Diagnostic contains:",
" assertEquals(1, \"\");",
" // BUG: Diagnostic contains:",
" assertEquals(\"foo\", 1, \"\");",
" // BUG: Diagnostic contains:",
" assertNotEquals(1, \"\");",
" // BUG: Diagnostic contains:",
" assertNotEquals(\"foo\", 1, \"\");",
" }",
"}")
.doTest();
}

@Test
public void assertEquals_matched() {
compilationHelper
.addSourceLines(
"Test.java",
"import static org.junit.Assert.assertEquals;",
"import static org.junit.Assert.assertNotEquals;",
"class Test {",
" public void test() {",
" assertEquals(1, 2);",
" assertEquals(1, 2L);",
" assertEquals(\"foo\", 1, 2);",
" assertNotEquals(1, 2);",
" assertNotEquals(\"foo\", 1, 2);",
" }",
"}")
.doTest();
}

@Test
public void assertArrayEquals_mismatched() {
compilationHelper
.addSourceLines(
"Test.java",
"import static org.junit.Assert.assertArrayEquals;",
"final class Test {",
" public void test() {",
" // BUG: Diagnostic contains:",
" assertArrayEquals(new Test[]{}, new String[]{\"\"});",
" // BUG: Diagnostic contains:",
" assertArrayEquals(\"foo\", new Test[]{}, new String[]{\"\"});",
" }",
"}")
.doTest();
}

@Test
public void assertArrayEquals_primitiveOverloadsFine() {
compilationHelper
.addSourceLines(
"Test.java",
"import static org.junit.Assert.assertArrayEquals;",
"final class Test {",
" public void test() {",
" assertArrayEquals(new long[]{1L}, new long[]{2L});",
" }",
"}")
.doTest();
}
}

0 comments on commit 375ffde

Please sign in to comment.