From ced45af2fefea6df130009b5656bb5bca6b4b0fd Mon Sep 17 00:00:00 2001
From: Tobi <tobi_adewuyi@yahoo.com>
Date: Tue, 4 May 2021 00:50:06 -0700
Subject: [PATCH] Throw exception if 'None' value given to List-type command
 line options in Bazel transitions

* Proposed fix for #12559
* Command line options specified as Java List types should default to empty when given 'None' Starlark value for Bazel transitions

Closes #13286.

PiperOrigin-RevId: 371860821
---
 .../starlark/FunctionTransitionUtil.java      |  5 +++
 .../StarlarkRuleTransitionProviderTest.java   | 31 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
index cd571a603f3b1e..a8f9951e45f4bf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
@@ -332,6 +332,11 @@ private static BuildOptions applyTransition(
                       .convert(
                           optionValueAsList.stream().map(Object::toString).collect(joining(",")));
             }
+          } else if (def.getType() == List.class && optionValue == null) {
+            throw ValidationException.format(
+                "'None' value not allowed for List-type option '%s'. Please use '[]' instead if"
+                    + " trying to set option to empty value.",
+                optionName);
           } else if (optionValue == null || def.getType().isInstance(optionValue)) {
             convertedValue = optionValue;
           } else if (def.getType().equals(boolean.class) && optionValue instanceof Boolean) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
index f6061cc2c76e94..d1d472b69a84f2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -1328,6 +1328,37 @@ public void successfulTypeConversionOfNativeListOptionEmptyList() throws Excepti
     assertThat(getConfiguration(ct).getOptions().get(CppOptions.class).fissionModes).isEmpty();
   }
 
+  @Test
+  public void failedTypeConversionOfNativeListOptionNone() throws Exception {
+    writeAllowlistFile();
+    scratch.file(
+        "test/transitions.bzl",
+        "def _impl(settings, attr):",
+        "  return {'//command_line_option:copt': None}",
+        "my_transition = transition(implementation = _impl, inputs = [],",
+        "  outputs = ['//command_line_option:copt'])");
+    scratch.file(
+        "test/rules.bzl",
+        "load('//test:transitions.bzl', 'my_transition')",
+        "def _impl(ctx):",
+        "  return []",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  cfg = my_transition,",
+        "  attrs = {",
+        "    '_allowlist_function_transition': attr.label(",
+        "        default = '//tools/allowlists/function_transition_allowlist',",
+        "    ),",
+        "  })");
+    scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'test')");
+
+    reporter.removeHandler(failFastHandler);
+    getConfiguredTarget("//test");
+    assertContainsEvent(
+        "'None' value not allowed for List-type option 'copt'. Please use '[]' instead if trying"
+            + " to set option to empty value.");
+  }
+
   @Test
   public void starlarkPatchTransitionRequiredFragments() throws Exception {
     // All Starlark rule transitions are patch transitions, while all Starlark attribute transitions