Introduce IdentityConversionCheck

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/IdentityConversionCheck.java

@@ -0,0 +1,106 @@
+package tech.picnic.errorprone.bugpatterns;
+
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.staticMethod;
+import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.LinkType;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.BugPattern.StandardTags;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.bugpatterns.TypesWithUndefinedEquality;
+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.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.ASTHelpers.TargetType;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Types;
+import java.util.Arrays;
+import java.util.List;
+
+/** A {@link BugChecker} that flags redundant identity conversions. */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "IdentityConversion",
+    summary = "Avoid or clarify identity conversions",
+    linkType = LinkType.NONE,
+    severity = SeverityLevel.WARNING,
+    tags = StandardTags.SIMPLIFICATION)
+public final class IdentityConversionCheck extends BugChecker
+    implements MethodInvocationTreeMatcher {
+  private static final long serialVersionUID = 1L;
+  private static final Matcher<ExpressionTree> IS_CONVERSION_METHOD =
+      anyOf(
+          staticMethod()
+              .onClassAny(
+                  "com.google.common.collect.ImmutableBiMap",
+                  "com.google.common.collect.ImmutableList",
+                  "com.google.common.collect.ImmutableListMultimap",
+                  "com.google.common.collect.ImmutableMap",
+                  "com.google.common.collect.ImmutableMultimap",
+                  "com.google.common.collect.ImmutableMultiset",
+                  "com.google.common.collect.ImmutableRangeMap",
+                  "com.google.common.collect.ImmutableRangeSet",
+                  "com.google.common.collect.ImmutableSet",
+                  "com.google.common.collect.ImmutableSetMultimap",
+                  "com.google.common.collect.ImmutableTable")
+              .named("copyOf"),
+          staticMethod()
+              .onClassAny(
+                  Byte.class.getName(),
+                  Character.class.getName(),
+                  Double.class.getName(),
+                  Float.class.getName(),
+                  Integer.class.getName(),
+                  String.class.getName())
+              .named("valueOf"),
+          staticMethod().onClass("reactor.adapter.rxjava.RxJava2Adapter"),
+          staticMethod()
+              .onClass("reactor.core.publisher.Flux")
+              .namedAnyOf("concat", "firstWithSignal", "from", "merge"),
+          staticMethod().onClass("reactor.core.publisher.Mono").namedAnyOf("from", "fromDirect"));
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    List<? extends ExpressionTree> arguments = tree.getArguments();
+    if (arguments.size() != 1 || !IS_CONVERSION_METHOD.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    ExpressionTree sourceTree = arguments.get(0);
+    Type sourceType = ASTHelpers.getType(sourceTree);
+    TargetType targetType = ASTHelpers.targetType(state);
+    if (sourceType == null || targetType == null) {
+      return Description.NO_MATCH;
+    }
+
+    if (state.getTypes().isSameType(sourceType, ASTHelpers.getType(tree))
+        || isSubtypeWithDefinedEquality(sourceType, targetType.type(), state)) {
+      return buildDescription(tree)
+          .setMessage(
+              "This method invocation appears redundant; remove it or suppress this warning and "
+                  + "add an comment explaining its purpose")
+          .addFix(SuggestedFix.replace(tree, state.getSourceForNode(sourceTree)))
+          .addFix(SuggestedFixes.addSuppressWarnings(state, canonicalName()))
+          .build();
+    }
+    return Description.NO_MATCH;
+  }
+
+  private static boolean isSubtypeWithDefinedEquality(
+      Type sourceType, Type targetType, VisitorState state) {
+    Types types = state.getTypes();
+    return !types.isSameType(targetType, OBJECT_TYPE.get(state))
+        && types.isSubtype(sourceType, targetType)
+        && Arrays.stream(TypesWithUndefinedEquality.values())
+            .noneMatch(b -> b.matchesType(sourceType, state) || b.matchesType(targetType, state));
+  }
+}

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListMultimapTemplates.java

@@ -272,17 +272,4 @@ ImmutableListMultimap<K, V2> after(
       return ImmutableListMultimap.copyOf(Multimaps.transformValues(multimap, transformation));
     }
   }
-
-  /** Don't unnecessarily copy an {@link ImmutableListMultimap}. */
-  static final class ImmutableListMultimapCopyOfImmutableListMultimap<K, V> {
-    @BeforeTemplate
-    ImmutableListMultimap<K, V> before(ImmutableListMultimap<K, V> multimap) {
-      return ImmutableListMultimap.copyOf(multimap);
-    }
-
-    @AfterTemplate
-    ImmutableListMultimap<K, V> after(ImmutableListMultimap<K, V> multimap) {
-      return multimap;
-    }
-  }
 }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMapTemplates.java

@@ -242,19 +242,6 @@ ImmutableMap<K, V2> after(Map<K, V1> map) {
     }
   }
 
-  /** Don't unnecessarily copy an {@link ImmutableMap}. */
-  static final class ImmutableMapCopyOfImmutableMap<K, V> {
-    @BeforeTemplate
-    ImmutableMap<K, V> before(ImmutableMap<K, V> map) {
-      return ImmutableMap.copyOf(map);
-    }
-
-    @AfterTemplate
-    ImmutableMap<K, V> after(ImmutableMap<K, V> map) {
-      return map;
-    }
-  }
-
   // XXX: Add a template for this:
   // Maps.transformValues(streamOfEntries.collect(groupBy(fun)), ImmutableMap::copyOf)
   // ->

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableMultisetTemplates.java

@@ -101,17 +101,4 @@ ImmutableMultiset<T> after(Stream<T> stream) {
       return stream.collect(toImmutableMultiset());
     }
   }
-
-  /** Don't unnecessarily copy an {@link ImmutableMultiset}. */
-  static final class ImmutableMultisetCopyOfImmutableMultiset<T> {
-    @BeforeTemplate
-    ImmutableMultiset<T> before(ImmutableMultiset<T> multiset) {
-      return ImmutableMultiset.copyOf(multiset);
-    }
-
-    @AfterTemplate
-    ImmutableMultiset<T> after(ImmutableMultiset<T> multiset) {
-      return multiset;
-    }
-  }
 }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetMultimapTemplates.java

@@ -215,17 +215,4 @@ ImmutableSetMultimap<K, V2> after(
       return ImmutableSetMultimap.copyOf(Multimaps.transformValues(multimap, transformation));
     }
   }
-
-  /** Don't unnecessarily copy an {@link ImmutableSetMultimap}. */
-  static final class ImmutableSetMultimapCopyOfImmutableSetMultimap<K, V> {
-    @BeforeTemplate
-    ImmutableSetMultimap<K, V> before(ImmutableSetMultimap<K, V> multimap) {
-      return ImmutableSetMultimap.copyOf(multimap);
-    }
-
-    @AfterTemplate
-    ImmutableSetMultimap<K, V> after(ImmutableSetMultimap<K, V> multimap) {
-      return multimap;
-    }
-  }
 }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSetTemplates.java

@@ -126,19 +126,6 @@ ImmutableSet<T> after(Stream<T> stream) {
     }
   }
 
-  /** Don't unnecessarily copy an {@link ImmutableSet}. */
-  static final class ImmutableSetCopyOfImmutableSet<T> {
-    @BeforeTemplate
-    ImmutableSet<T> before(ImmutableSet<T> set) {
-      return ImmutableSet.copyOf(set);
-    }
-
-    @AfterTemplate
-    ImmutableSet<T> after(ImmutableSet<T> set) {
-      return set;
-    }
-  }
-
   /** Prefer {@link SetView#immutableCopy()} over the more verbose alternative. */
   static final class ImmutableSetCopyOfSetView<T> {
     @BeforeTemplate

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/ReactorTemplates.java

@@ -203,19 +203,6 @@ Flux<T> after(Mono<T> mono) {
     }
   }
 
-  /** Don't unnecessarily invoke {@link Flux#concat(Publisher)}. */
-  static final class FluxIdentity<T> {
-    @BeforeTemplate
-    Flux<T> before(Flux<T> flux) {
-      return Flux.concat(flux);
-    }
-
-    @AfterTemplate
-    Flux<T> after(Flux<T> flux) {
-      return flux;
-    }
-  }
-
   /**
    * Prefer a collection using {@link MoreCollectors#toOptional()} over more contrived alternatives.
    */

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/RxJava2AdapterTemplates.java

@@ -41,11 +41,9 @@ static final class FlowableToFlux<T> {
     @BeforeTemplate
     Publisher<T> before(Flowable<T> flowable) {
       return Refaster.anyOf(
-          Flux.from(flowable),
           flowable.compose(Flux::from),
           flowable.to(Flux::from),
           flowable.as(Flux::from),
-          RxJava2Adapter.flowableToFlux(flowable),
           flowable.compose(RxJava2Adapter::flowableToFlux),
           flowable.to(RxJava2Adapter::flowableToFlux));
     }
@@ -67,7 +65,6 @@ Publisher<T> before(Flux<T> flux) {
           Flowable.fromPublisher(flux),
           flux.transform(Flowable::fromPublisher),
           flux.as(Flowable::fromPublisher),
-          RxJava2Adapter.fluxToFlowable(flux),
           flux.transform(RxJava2Adapter::fluxToFlowable));
     }
 
@@ -140,7 +137,6 @@ Publisher<T> before(Mono<T> mono) {
           Flowable.fromPublisher(mono),
           mono.transform(Flowable::fromPublisher),
           mono.as(Flowable::fromPublisher),
-          RxJava2Adapter.monoToFlowable(mono),
           mono.transform(RxJava2Adapter::monoToFlowable));
     }