diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index 172e1ce548..9f455e7838 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -143,6 +143,11 @@ assertj-core provided + + org.immutables + value-annotations + test + org.junit.jupiter junit-jupiter-api diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparator.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparator.java new file mode 100644 index 0000000000..5e8a3c2c71 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparator.java @@ -0,0 +1,81 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.NONE; +import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; +import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR; +import static com.google.errorprone.matchers.Matchers.allOf; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.enclosingClass; +import static com.google.errorprone.matchers.Matchers.hasAnnotation; +import static com.google.errorprone.matchers.Matchers.hasModifier; +import static com.google.errorprone.matchers.Matchers.isSubtypeOf; +import static com.google.errorprone.matchers.Matchers.methodReturns; +import static com.google.errorprone.matchers.Matchers.not; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; +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.MethodTree; +import java.util.SortedSet; +import javax.lang.model.element.Modifier; + +/** + * A {@link BugChecker} which flags {@link SortedSet} property declarations inside + * {@code @Value.Immutable}- and {@code @Value.Modifiable}-annotated types that lack a + * {@code @Value.NaturalOrder} or {@code @Value.ReverseOrder} annotation. + * + *

Without such an annotation: + * + *

+ */ +@AutoService(BugChecker.class) +@BugPattern( + summary = + "`SortedSet` properties of a `@Value.Immutable` or `@Value.Modifiable` type must be " + + "annotated with `@Value.NaturalOrder` or `@Value.ReverseOrder`", + linkType = NONE, + severity = ERROR, + tags = LIKELY_ERROR) +public final class ImmutablesSortedSetComparator extends BugChecker implements MethodTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Matcher METHOD_LACKS_ANNOTATION = + allOf( + methodReturns(isSubtypeOf(SortedSet.class)), + anyOf( + allOf( + hasModifier(Modifier.ABSTRACT), + enclosingClass( + anyOf( + hasAnnotation("org.immutables.value.Value.Immutable"), + hasAnnotation("org.immutables.value.Value.Modifiable")))), + hasAnnotation("org.immutables.value.Value.Default")), + not( + anyOf( + hasAnnotation("org.immutables.value.Value.NaturalOrder"), + hasAnnotation("org.immutables.value.Value.ReverseOrder")))); + + @Override + public Description matchMethod(MethodTree tree, VisitorState state) { + if (!METHOD_LACKS_ANNOTATION.matches(tree, state)) { + return Description.NO_MATCH; + } + + SuggestedFix.Builder builder = SuggestedFix.builder(); + String valueTypeIdentifier = + SuggestedFixes.qualifyType(state, builder, "org.immutables.value.Value"); + return describeMatch( + tree, + builder.prefixWith(tree, String.format("@%s.NaturalOrder ", valueTypeIdentifier)).build()); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java new file mode 100644 index 0000000000..3942c6de1e --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ImmutablesSortedSetComparatorTest.java @@ -0,0 +1,182 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class ImmutablesSortedSetComparatorTest { + private final CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass()); + private final BugCheckerRefactoringTestHelper refactoringTestHelper = + BugCheckerRefactoringTestHelper.newInstance(ImmutablesSortedSetComparator.class, getClass()); + + @Test + void identification() { + compilationTestHelper + .addSourceLines( + "A.java", + "import com.google.common.collect.ContiguousSet;", + "import com.google.common.collect.ImmutableSet;", + "import com.google.common.collect.ImmutableSortedSet;", + "import java.util.NavigableSet;", + "import java.util.Set;", + "import java.util.SortedSet;", + "import java.util.TreeSet;", + "import org.immutables.value.Value;", + "", + "interface A {", + " @Value.Immutable", + " interface ImmutableInterface {", + " Set set();", + "", + " // BUG: Diagnostic contains:", + " SortedSet sortedSet();", + "", + " @Value.NaturalOrder", + " SortedSet sortedSet2();", + " }", + "", + " @Value.Modifiable", + " interface ModifiableInterfaceWithDefaults {", + " @Value.Default", + " default Set set() {", + " return new TreeSet<>();", + " }", + "", + " @Value.Default", + " // BUG: Diagnostic contains:", + " default NavigableSet navigableSet() {", + " return new TreeSet<>();", + " }", + "", + " @Value.Default", + " @Value.ReverseOrder", + " default NavigableSet navigableSet2() {", + " return new TreeSet<>();", + " }", + "", + " default NavigableSet nonPropertyNavigableSet() {", + " return new TreeSet<>();", + " }", + " }", + "", + " interface NonImmutablesInterface {", + " SortedSet sortedSet();", + " }", + "", + " @Value.Immutable", + " abstract class AbstractImmutableWithDefaults {", + " @Value.Default", + " ImmutableSet immutableSet() {", + " return ImmutableSet.of();", + " }", + "", + " @Value.Default", + " // BUG: Diagnostic contains:", + " ImmutableSortedSet immutableSortedSet() {", + " return ImmutableSortedSet.of();", + " }", + "", + " @Value.Default", + " @Value.NaturalOrder", + " ImmutableSortedSet immutableSortedSet2() {", + " return ImmutableSortedSet.of();", + " }", + "", + " ImmutableSortedSet nonPropertyImmutableSortedSet() {", + " return ImmutableSortedSet.of();", + " }", + " }", + "", + " @Value.Modifiable", + " abstract class AbstractModifiable {", + " abstract ImmutableSet immutableSet();", + "", + " // BUG: Diagnostic contains:", + " abstract ContiguousSet contiguousSet();", + "", + " @Value.ReverseOrder", + " abstract ContiguousSet contiguousSet2();", + " }", + "", + " abstract class AbstractNonImmutables {", + " abstract SortedSet sortedSet();", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + refactoringTestHelper + .addInputLines( + "A.java", + "import com.google.common.collect.ImmutableSortedSet;", + "import java.util.SortedSet;", + "import org.immutables.value.Value;", + "", + "@Value.Immutable", + "abstract class A {", + " abstract ImmutableSortedSet sortedSet();", + "", + " @Value.Modifiable", + " interface B {", + " SortedSet sortedSet();", + " }", + "}") + .addOutputLines( + "A.java", + "import com.google.common.collect.ImmutableSortedSet;", + "import java.util.SortedSet;", + "import org.immutables.value.Value;", + "", + "@Value.Immutable", + "abstract class A {", + " @Value.NaturalOrder", + " abstract ImmutableSortedSet sortedSet();", + "", + " @Value.Modifiable", + " interface B {", + " @Value.NaturalOrder", + " SortedSet sortedSet();", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } + + @Test + void replacementWithImportClash() { + refactoringTestHelper + .addInputLines( + "MySpringService.java", + "import com.google.common.collect.ImmutableSortedSet;", + "import org.springframework.beans.factory.annotation.Value;", + "", + "class MySpringService {", + " MySpringService(@Value(\"${someProperty}\") String prop) {}", + " ;", + "", + " @org.immutables.value.Value.Immutable", + " interface A {", + " ImmutableSortedSet sortedSet();", + " }", + "}") + .addOutputLines( + "MySpringService.java", + "import com.google.common.collect.ImmutableSortedSet;", + "import org.springframework.beans.factory.annotation.Value;", + "", + "class MySpringService {", + " MySpringService(@Value(\"${someProperty}\") String prop) {}", + " ;", + "", + " @org.immutables.value.Value.Immutable", + " interface A {", + " @org.immutables.value.Value.NaturalOrder", + " ImmutableSortedSet sortedSet();", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/pom.xml b/pom.xml index f8d4dd71ec..b2a01c63b5 100644 --- a/pom.xml +++ b/pom.xml @@ -386,6 +386,11 @@ hamcrest-core 2.2
+ + org.immutables + value-annotations + 2.9.0 + org.junit junit-bom