diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java b/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java new file mode 100644 index 000000000000..78e5c92aebb5 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsage.java @@ -0,0 +1,110 @@ +/* + * Copyright 2023 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.common.collect.Iterables.getLast; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.instanceMethod; +import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.util.ASTHelpers.getStartPosition; + +import com.google.common.collect.ImmutableList; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.StandardTags; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.fixes.SuggestedFixes; +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.NewArrayTree; +import java.util.Optional; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern( + tags = {StandardTags.REFACTORING}, + summary = + "containsExactly is preferred over containsExactlyElementsIn when creating new iterables.", + severity = WARNING) +public final class TruthContainsExactlyElementsInUsage extends BugChecker + implements MethodInvocationTreeMatcher { + + private static final Matcher CONTAINS_EXACTLY_ELEMENTS_IN_METHOD_MATCHER = + instanceMethod() + .onDescendantOf("com.google.common.truth.IterableSubject") + .named("containsExactlyElementsIn"); + + // Not including Sets for the rare occasion of duplicate element declarations inside the set. If + // refactored to containsExactly, the behavior is different. + private static final Matcher NEW_ITERABLE_MATCHERS = + anyOf( + staticMethod().onClass("com.google.common.collect.Lists").named("newArrayList"), + staticMethod().onClass("com.google.common.collect.ImmutableList").named("of"), + staticMethod().onClass("java.util.Arrays").named("asList"), + staticMethod().onClass("java.util.Collections").named("singletonList"), + staticMethod().onClass("java.util.List").named("of")); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!CONTAINS_EXACTLY_ELEMENTS_IN_METHOD_MATCHER.matches(tree, state)) { + return Description.NO_MATCH; + } + + // Avoids refactoring variables and method invocations that are not creating new iterables. + // The first param from containsExactlyElementsIn should always be an Iterable. + return getArgumentsFromNewIterableExpression(tree.getArguments().get(0), state) + .map(arguments -> describeMatch(tree, refactor(arguments, tree, state))) + .orElse(Description.NO_MATCH); + } + + // Returns the arguments from the expression. If it is not a valid expression, returns empty. + private static Optional> getArgumentsFromNewIterableExpression( + ExpressionTree expression, VisitorState state) { + if (expression instanceof MethodInvocationTree + && NEW_ITERABLE_MATCHERS.matches(expression, state)) { + MethodInvocationTree paramMethodInvocationTree = (MethodInvocationTree) expression; + return Optional.of(ImmutableList.copyOf(paramMethodInvocationTree.getArguments())); + } else if (expression instanceof NewArrayTree) { + NewArrayTree newArrayTree = (NewArrayTree) expression; + return Optional.of(ImmutableList.copyOf(newArrayTree.getInitializers())); + } + return Optional.empty(); + } + + private static SuggestedFix refactor( + ImmutableList arguments, MethodInvocationTree tree, VisitorState state) { + // First we replace the containsExactlyElementsIn method with containsExactly. + SuggestedFix.Builder fix = + SuggestedFix.builder() + .merge(SuggestedFixes.renameMethodInvocation(tree, "containsExactly", state)); + // Finally, we use the arguments from the new iterable to build the containsExactly arguments. + ExpressionTree expressionToReplace = tree.getArguments().get(0); + if (!arguments.isEmpty()) { + fix.replace(getStartPosition(expressionToReplace), getStartPosition(arguments.get(0)), "") + .replace( + state.getEndPosition(getLast(arguments)), + state.getEndPosition(expressionToReplace), + ""); + } else { + fix.delete(expressionToReplace); + } + return fix.build(); + } +} 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 2b6c69e1f450..6b8dfd937162 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -368,6 +368,7 @@ import com.google.errorprone.bugpatterns.TreeToString; import com.google.errorprone.bugpatterns.TruthAssertExpected; import com.google.errorprone.bugpatterns.TruthConstantAsserts; +import com.google.errorprone.bugpatterns.TruthContainsExactlyElementsInUsage; import com.google.errorprone.bugpatterns.TruthGetOrDefault; import com.google.errorprone.bugpatterns.TruthSelfEquals; import com.google.errorprone.bugpatterns.TryFailRefactoring; @@ -1180,6 +1181,7 @@ public static ScannerSupplier warningChecks() { TimeUnitMismatch.class, TooManyParameters.class, TransientMisuse.class, + TruthContainsExactlyElementsInUsage.class, TryFailRefactoring.class, TryWithResourcesVariable.class, TypeParameterNaming.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsageTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsageTest.java new file mode 100644 index 000000000000..ec0fbe438959 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/TruthContainsExactlyElementsInUsageTest.java @@ -0,0 +1,371 @@ +/* + * Copyright 2023 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.BugCheckerRefactoringTestHelper; +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 TruthContainsExactlyElementsInUsageTest { + + private final BugCheckerRefactoringTestHelper refactoringHelper = + BugCheckerRefactoringTestHelper.newInstance( + TruthContainsExactlyElementsInUsage.class, getClass()); + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(TruthContainsExactlyElementsInUsage.class, getClass()); + + @Test + public void negativeDirectContainsExactlyUsage() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1,2,3);", + " }", + "}") + .doTest(); + } + + @Test + public void negativeVariableContainsExactlyUsage() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.List;", + "public class ExampleClassTest {", + " void test() {", + " List list = ImmutableList.of(1, 2, 3);", + " assertThat(list).containsExactly(1,2,3);", + " }", + "}") + .doTest(); + } + + @Test + public void negativeVariableTruthContainsExactlyElementsInUsage() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.List;", + "public class ExampleClassTest {", + " void test() {", + " List list = ImmutableList.of(1, 2, 3);", + " assertThat(list).containsExactlyElementsIn(list);", + " }", + "}") + .doTest(); + } + + @Test + public void negativeVariableTruthContainsExactlyElementsInUsageWithCopy() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.List;", + "public class ExampleClassTest {", + " void test() {", + " List list = ImmutableList.of(1, 2, 3);", + " assertThat(list).containsExactlyElementsIn(ImmutableList.copyOf(list));", + " }", + "}") + .doTest(); + } + + @Test + public void negativeTruthContainsExactlyElementsInUsageWithHashSet() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.Sets;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(Sets.newHashSet(1, 2, 3));", + " }", + "}") + .doTest(); + } + + @Test + public void negativeTruthContainsExactlyElementsInUsageWithImmutableSet() { + compilationHelper + .addSourceLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.ImmutableSet;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(ImmutableSet.of(1, 2, 3));", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithArrayList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(Arrays.asList(1, 2, 3));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithListOf() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.List;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(List.of(1, 2, 3));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.List;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithInOrderList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(Arrays.asList(1, 2, 3)).inOrder();", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3).inOrder();", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithStaticallyImportedAsList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import static java.util.Arrays.asList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactlyElementsIn(asList(1, 2, 3));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import static java.util.Arrays.asList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithNewArrayList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.Lists;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(Lists.newArrayList(1, 2, 3));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.Lists;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithSingletonList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Collections;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1))", + " .containsExactlyElementsIn(Collections.singletonList(1));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Collections;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1)).containsExactly(1);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithEmptyList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of()).containsExactlyElementsIn(Arrays.asList());", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "import java.util.Arrays;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of()).containsExactly();", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithImmutableList() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(ImmutableList.of(1, 2, 3));", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1, 2, 3);", + " }", + "}") + .doTest(); + } + + @Test + public void refactoringTruthContainsExactlyElementsInUsageWithArray() { + refactoringHelper + .addInputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3))", + " .containsExactlyElementsIn(new Integer[] {1, 2, 3});", + " }", + "}") + .addOutputLines( + "ExampleClassTest.java", + "import static com.google.common.truth.Truth.assertThat;", + "import com.google.common.collect.ImmutableList;", + "public class ExampleClassTest {", + " void test() {", + " assertThat(ImmutableList.of(1, 2, 3)).containsExactly(1,2,3);", + " }", + "}") + .doTest(); + } +}