diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleType.java b/core/src/main/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleType.java new file mode 100644 index 00000000000..b5d11620085 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleType.java @@ -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 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 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() { + @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); + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index a1eb6c17095..11139118774 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -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; @@ -938,6 +939,7 @@ public static ScannerSupplier warningChecks() { JUnit3FloatingPointComparisonWithoutDelta.class, JUnit4ClassUsedInJUnit3.class, JUnitAmbiguousTestClass.class, + JUnitIncompatibleType.class, JavaDurationGetSecondsGetNano.class, JavaDurationWithNanos.class, JavaDurationWithSeconds.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleTypeTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleTypeTest.java new file mode 100644 index 00000000000..58c912bad28 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/collectionincompatibletype/JUnitIncompatibleTypeTest.java @@ -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(); + } +}