Have RequestParamType ignore parameter types with custom deserialization support

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

@@ -42,6 +42,7 @@
 import java.util.stream.Stream;
 import org.jspecify.annotations.Nullable;
 import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
+import tech.picnic.errorprone.bugpatterns.util.Flags;
 import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
 /**
@@ -220,7 +221,7 @@ private static AnnotationAttributeMatcher createAnnotationAttributeMatcher(
 
   private static ImmutableList<String> excludedAnnotations(ErrorProneFlags flags) {
     Set<String> exclusions = new HashSet<>();
-    flags.getList(EXCLUDED_ANNOTATIONS_FLAG).ifPresent(exclusions::addAll);
+    exclusions.addAll(Flags.getList(flags, EXCLUDED_ANNOTATIONS_FLAG));
     exclusions.addAll(BLACKLISTED_ANNOTATIONS);
     return ImmutableList.copyOf(exclusions);
   }

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

@@ -49,6 +49,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Stream;
+import tech.picnic.errorprone.bugpatterns.util.Flags;
 import tech.picnic.errorprone.bugpatterns.util.MethodMatcherFactory;
 import tech.picnic.errorprone.bugpatterns.util.SourceCode;
 
@@ -63,9 +64,8 @@
 public final class RedundantStringConversion extends BugChecker
     implements BinaryTreeMatcher, CompoundAssignmentTreeMatcher, MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 1L;
-  private static final String FLAG_PREFIX = "RedundantStringConversion:";
   private static final String EXTRA_STRING_CONVERSION_METHODS_FLAG =
-      FLAG_PREFIX + "ExtraConversionMethods";
+      "RedundantStringConversion:ExtraConversionMethods";
 
   @SuppressWarnings("UnnecessaryLambda")
   private static final Matcher<ExpressionTree> ANY_EXPR = (t, s) -> true;
@@ -374,10 +374,9 @@ private static Matcher<MethodInvocationTree> createConversionMethodMatcher(
       ErrorProneFlags flags) {
     // XXX: ErrorProneFlags#getList splits by comma, but method signatures may also contain commas.
     // For this class methods accepting more than one argument are not valid, but still: not nice.
-    return flags
-        .getList(EXTRA_STRING_CONVERSION_METHODS_FLAG)
-        .map(new MethodMatcherFactory()::create)
-        .map(m -> anyOf(WELL_KNOWN_STRING_CONVERSION_METHODS, m))
-        .orElse(WELL_KNOWN_STRING_CONVERSION_METHODS);
+    return anyOf(
+        WELL_KNOWN_STRING_CONVERSION_METHODS,
+        new MethodMatcherFactory()
+            .create(Flags.getList(flags, EXTRA_STRING_CONVERSION_METHODS_FLAG)));
   }
 }

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

@@ -1,5 +1,6 @@
 package tech.picnic.errorprone.bugpatterns;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
 import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
@@ -9,41 +10,72 @@
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
 import static com.google.errorprone.matchers.Matchers.isType;
+import static com.google.errorprone.matchers.Matchers.not;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
 import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.errorprone.BugPattern;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.suppliers.Suppliers;
+import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
+import tech.picnic.errorprone.bugpatterns.util.Flags;
 
 /** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
 @AutoService(BugChecker.class)
 @BugPattern(
-    summary = "`@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
+    summary =
+        "By default, `@RequestParam` does not support `ImmutableCollection` and `ImmutableMap` subtypes",
     link = BUG_PATTERNS_BASE_URL + "RequestParamType",
     linkType = CUSTOM,
     severity = ERROR,
     tags = LIKELY_ERROR)
 public final class RequestParamType extends BugChecker implements VariableTreeMatcher {
   private static final long serialVersionUID = 1L;
-  private static final Matcher<VariableTree> HAS_UNSUPPORTED_REQUEST_PARAM =
-      allOf(
-          annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
-          anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)));
+  private static final String SUPPORTED_CUSTOM_TYPES_FLAG = "RequestParamType:SupportedCustomTypes";
 
-  /** Instantiates a new {@link RequestParamType} instance. */
-  public RequestParamType() {}
+  private final Matcher<VariableTree> hasUnsupportedRequestParamType;
+
+  /** Instantiates a default {@link RequestParamType} instance. */
+  public RequestParamType() {
+    this(ErrorProneFlags.empty());
+  }
+
+  /**
+   * Instantiates a customized {@link RequestParamType} instance.
+   *
+   * @param flags Any provided command line flags.
+   */
+  public RequestParamType(ErrorProneFlags flags) {
+    hasUnsupportedRequestParamType = hasUnsupportedRequestParamType(flags);
+  }
 
   @Override
   public Description matchVariable(VariableTree tree, VisitorState state) {
-    return HAS_UNSUPPORTED_REQUEST_PARAM.matches(tree, state)
+    return hasUnsupportedRequestParamType.matches(tree, state)
         ? describeMatch(tree)
         : Description.NO_MATCH;
   }
+
+  private static Matcher<VariableTree> hasUnsupportedRequestParamType(ErrorProneFlags flags) {
+    return allOf(
+        annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
+        anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)),
+        not(isSubtypeOfAny(Flags.getList(flags, SUPPORTED_CUSTOM_TYPES_FLAG))));
+  }
+
+  private static Matcher<Tree> isSubtypeOfAny(ImmutableList<String> inclusions) {
+    return anyOf(
+        inclusions.stream()
+            .map(inclusion -> isSubtypeOf(Suppliers.typeFromString(inclusion)))
+            .collect(toImmutableList()));
+  }
 }

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

@@ -0,0 +1,25 @@
+package tech.picnic.errorprone.bugpatterns.util;
+
+import com.google.common.collect.ImmutableList;
+import com.google.errorprone.ErrorProneFlags;
+
+/** Helper methods for working with {@link ErrorProneFlags}. */
+public final class Flags {
+  private Flags() {}
+
+  /**
+   * Returns the list of (comma-separated) arguments passed using the given Error Prone flag.
+   *
+   * @param errorProneFlags The full set of flags provided.
+   * @param name The name of the flag of interest.
+   * @return A non-{@code null} list of provided arguments; this list is empty if the flag was not
+   *     provided, or if the flag's value is the empty string.
+   */
+  public static ImmutableList<String> getList(ErrorProneFlags errorProneFlags, String name) {
+    return errorProneFlags
+        .getList(name)
+        .map(ImmutableList::copyOf)
+        .filter(flags -> !flags.equals(ImmutableList.of("")))
+        .orElseGet(ImmutableList::of);
+  }
+}

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeTest.java

@@ -6,6 +6,10 @@
 final class RequestParamTypeTest {
   private final CompilationTestHelper compilationTestHelper =
       CompilationTestHelper.newInstance(RequestParamType.class, getClass());
+  private final CompilationTestHelper restrictedCompilationTestHelper =
+      CompilationTestHelper.newInstance(RequestParamType.class, getClass())
+          .setArgs(
+              "-XepOpt:RequestParamType:SupportedCustomTypes=com.google.common.collect.ImmutableSet,com.google.common.collect.ImmutableSortedMultiset");
 
   @Test
   void identification() {
@@ -63,4 +67,53 @@ void identification() {
             "}")
         .doTest();
   }
+
+  @Test
+  void identificationRestricted() {
+    restrictedCompilationTestHelper
+        .addSourceLines(
+            "A.java",
+            "import com.google.common.collect.ImmutableBiMap;",
+            "import com.google.common.collect.ImmutableCollection;",
+            "import com.google.common.collect.ImmutableList;",
+            "import com.google.common.collect.ImmutableMap;",
+            "import com.google.common.collect.ImmutableMultiset;",
+            "import com.google.common.collect.ImmutableSet;",
+            "import com.google.common.collect.ImmutableSortedMultiset;",
+            "import com.google.common.collect.ImmutableSortedSet;",
+            "import org.springframework.web.bind.annotation.GetMapping;",
+            "import org.springframework.web.bind.annotation.RequestParam;",
+            "",
+            "interface A {",
+            "  @GetMapping",
+            "  // BUG: Diagnostic contains:",
+            "  A immutableCollection(@RequestParam ImmutableCollection<String> param);",
+            "",
+            "  @GetMapping",
+            "  // BUG: Diagnostic contains:",
+            "  A immutableList(@RequestParam ImmutableList<String> param);",
+            "",
+            "  @GetMapping",
+            "  A immutableSet(@RequestParam ImmutableSet<String> param);",
+            "",
+            "  @GetMapping",
+            "  A immutableSortedSet(@RequestParam ImmutableSortedSet<String> param);",
+            "",
+            "  @GetMapping",
+            "  // BUG: Diagnostic contains:",
+            "  A immutableMultiset(@RequestParam ImmutableMultiset<String> param);",
+            "",
+            "  @GetMapping",
+            "  A immutableSortedMultiset(@RequestParam ImmutableSortedMultiset<String> param);",
+            "",
+            "  @GetMapping",
+            "  // BUG: Diagnostic contains:",
+            "  A immutableMap(@RequestParam ImmutableMap<String, String> param);",
+            "",
+            "  @GetMapping",
+            "  // BUG: Diagnostic contains:",
+            "  A immutableBiMap(@RequestParam ImmutableBiMap<String, String> param);",
+            "}")
+        .doTest();
+  }
 }

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/FlagsTest.java

@@ -0,0 +1,31 @@
+package tech.picnic.errorprone.bugpatterns.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+import com.google.common.collect.ImmutableList;
+import com.google.errorprone.ErrorProneOptions;
+import java.util.stream.Stream;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+final class FlagsTest {
+  private static Stream<Arguments> getListTestCases() {
+    /* { args, flag, expected } */
+    return Stream.of(
+        arguments(ImmutableList.of(), "Foo", ImmutableList.of()),
+        arguments(ImmutableList.of("-XepOpt:Foo=bar,baz"), "Qux", ImmutableList.of()),
+        arguments(ImmutableList.of("-XepOpt:Foo="), "Foo", ImmutableList.of()),
+        arguments(ImmutableList.of("-XepOpt:Foo=bar"), "Foo", ImmutableList.of("bar")),
+        arguments(ImmutableList.of("-XepOpt:Foo=bar,baz"), "Foo", ImmutableList.of("bar", "baz")),
+        arguments(ImmutableList.of("-XepOpt:Foo=,"), "Foo", ImmutableList.of("", "")));
+  }
+
+  @MethodSource("getListTestCases")
+  @ParameterizedTest
+  void getList(ImmutableList<String> args, String flag, ImmutableList<String> expected) {
+    assertThat(Flags.getList(ErrorProneOptions.processArgs(args).getFlags(), flag))
+        .containsExactlyElementsOf(expected);
+  }
+}