From ad103c26890d53f5858b1601dcdd029430b20e94 Mon Sep 17 00:00:00 2001
From: janakr <janakr@google.com>
Date: Tue, 22 Mar 2022 11:00:03 -0700
Subject: [PATCH] Allow ImmutableList (and any type that extends List) to be
 the type of an option with allowMultiple = true.

PiperOrigin-RevId: 436517607
---
 .../options/processor/OptionProcessor.java    |  7 ++---
 .../processor/OptionProcessorTest.java        | 16 +++++++---
 ...mutableListTypeForAllowMultipleOption.java | 31 +++++++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)
 create mode 100644 src/test/java/com/google/devtools/common/options/processor/optiontestsources/ImmutableListTypeForAllowMultipleOption.java

diff --git a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java
index cf21e6a9fc6470..90918a2f4bdcb7 100644
--- a/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java
+++ b/src/main/java/com/google/devtools/common/options/processor/OptionProcessor.java
@@ -190,12 +190,11 @@ private ImmutableList<TypeMirror> getAcceptedConverterReturnTypes(VariableElemen
             optionType);
       }
       DeclaredType optionDeclaredType = (DeclaredType) optionType;
-      // optionDeclaredType.asElement().asType() gets us from List<actualType> to List<E>, so this
-      // is unfortunately necessary.
-      if (!typeUtils.isAssignable(optionDeclaredType.asElement().asType(), listType)) {
+      if (!typeUtils.isAssignable(typeUtils.erasure(optionDeclaredType), listType)) {
         throw new OptionProcessorException(
             optionField,
-            "Option that allows multiple occurrences must be of type %s, but is of type %s",
+            "Option that allows multiple occurrences must be assignable to type %s, but is of type"
+                + " %s",
             listType,
             optionType);
       }
diff --git a/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java b/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java
index 6d975537e7fb18..5c98374726caee 100644
--- a/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java
+++ b/src/test/java/com/google/devtools/common/options/processor/OptionProcessorTest.java
@@ -236,8 +236,8 @@ public void allowMultipleOptionWithCollectionTypeIsRejected() {
         .processedWith(new OptionProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "Option that allows multiple occurrences must be of type java.util.List<E>, "
-                + "but is of type java.util.Collection<java.lang.String>");
+            "Option that allows multiple occurrences must be assignable to type java.util.List<E>,"
+                + " but is of type java.util.Collection<java.lang.String>");
   }
 
   @Test
@@ -247,8 +247,16 @@ public void allowMultipleOptionWithNonListTypeIsRejected() {
         .processedWith(new OptionProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "Option that allows multiple occurrences must be of type java.util.List<E>, "
-                + "but is of type java.lang.String");
+            "Option that allows multiple occurrences must be assignable to type java.util.List<E>,"
+                + " but is of type java.lang.String");
+  }
+
+  @Test
+  public void allowMultipleOptionWithImmutableListTypeIsAllowed() {
+    assertAbout(javaSource())
+        .that(getFile("ImmutableListTypeForAllowMultipleOption.java"))
+        .processedWith(new OptionProcessor())
+        .compilesWithoutError();
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/common/options/processor/optiontestsources/ImmutableListTypeForAllowMultipleOption.java b/src/test/java/com/google/devtools/common/options/processor/optiontestsources/ImmutableListTypeForAllowMultipleOption.java
new file mode 100644
index 00000000000000..dec4600d00147b
--- /dev/null
+++ b/src/test/java/com/google/devtools/common/options/processor/optiontestsources/ImmutableListTypeForAllowMultipleOption.java
@@ -0,0 +1,31 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.devtools.common.options.processor.optiontestsources;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionDocumentationCategory;
+import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.OptionsBase;
+
+/** This example options class should compile. */
+public class ImmutableListTypeForAllowMultipleOption extends OptionsBase {
+  @Option(
+      name = "option",
+      defaultValue = "null",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      allowMultiple = true)
+  public ImmutableList<String> badOption;
+}