Convert incorrectly YAML-parsed booleans back to their enum values when possible

4 files changed, 135 insertions, 1 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f24fb4a..069c093 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
   https://google.github.io/TestParameterInjector/docs/latest/com/google/testing/junit/testparameterinjector/TestParameterValuesProvider.html).
 - Added support for repeated annotations to [`TestParameterValuesProvider.Context`](
   https://google.github.io/TestParameterInjector/docs/latest/com/google/testing/junit/testparameterinjector/TestParameterValuesProvider.Context.html)
+- Converting incorrectly YAML-parsed booleans back to their enum values when possible
 
 ## 1.15
 
diff --git a/junit4/src/main/java/com/google/testing/junit/testparameterinjector/ParameterValueParsing.java b/junit4/src/main/java/com/google/testing/junit/testparameterinjector/ParameterValueParsing.java
index e09c1d9..a013ebf 100644
--- a/junit4/src/main/java/com/google/testing/junit/testparameterinjector/ParameterValueParsing.java
+++ b/junit4/src/main/java/com/google/testing/junit/testparameterinjector/ParameterValueParsing.java
@@ -17,10 +17,12 @@ package com.google.testing.junit.testparameterinjector;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Iterables.getOnlyElement;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.primitives.Primitives;
 import com.google.common.primitives.UnsignedLong;
@@ -31,10 +33,12 @@ import java.lang.reflect.ParameterizedType;
 import java.math.BigInteger;
 import java.nio.charset.Charset;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import javax.annotation.Nullable;
 import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.Yaml;
@@ -131,6 +135,11 @@ final class ParameterValueParsing {
     yamlValueTransformer
         .ifJavaType(Enum.class)
         .supportParsedType(
+            Boolean.class,
+            bool ->
+                ParameterValueParsing.parseEnumIfUnambiguousYamlBoolean(
+                    bool, javaType.getRawType()))
+        .supportParsedType(
             String.class, str -> ParameterValueParsing.parseEnum(str, javaType.getRawType()));
 
     yamlValueTransformer
@@ -166,6 +175,42 @@ final class ParameterValueParsing {
     return yamlValueTransformer.transformedJavaValue();
   }
 
+  private static Enum<?> parseEnumIfUnambiguousYamlBoolean(boolean yamlValue, Class<?> enumType) {
+    Set<String> negativeYamlStrings =
+        ImmutableSet.of("false", "False", "FALSE", "n", "N", "no", "No", "NO", "off", "Off", "OFF");
+    Set<String> positiveYamlStrings =
+        ImmutableSet.of("on", "On", "ON", "true", "True", "TRUE", "y", "Y", "yes", "Yes", "YES");
+
+    // This is the list of YAML strings that a user could have used to define this boolean. Since
+    // the user probably didn't intend a boolean but an enum (since we're expecting an enum), one of
+    // these strings may (unambiguously) match one of the enum values.
+    Set<String> yamlStringCandidates = yamlValue ? positiveYamlStrings : negativeYamlStrings;
+
+    Set<Enum<?>> matches = new HashSet<>();
+    for (Object enumValueObject : enumType.getEnumConstants()) {
+      Enum<?> enumValue = (Enum<?>) enumValueObject;
+      if (yamlStringCandidates.contains(enumValue.name())) {
+        matches.add(enumValue);
+      }
+    }
+
+    checkArgument(
+        !matches.isEmpty(),
+        "Cannot cast a boolean (%s) to an enum of type %s.",
+        yamlValue,
+        enumType.getSimpleName());
+    checkArgument(
+        matches.size() == 1,
+        "Cannot cast a boolean (%s) to an enum of type %s. It is likely that the YAML parser is"
+            + " 'wrongly' parsing one of these values as boolean: %s. You can solve this by putting"
+            + " quotes around the YAML value, forcing the YAML parser to parse a String, which can"
+            + " then be converted to the enum.",
+        yamlValue,
+        enumType.getSimpleName(),
+        matches);
+    return getOnlyElement(matches);
+  }
+
   private static Map<?, ?> parseYamlMapToJavaMap(Map<?, ?> map, TypeToken<?> javaType) {
     Map<Object, Object> returnedMap = new LinkedHashMap<>();
     for (Entry<?, ?> entry : map.entrySet()) {
diff --git a/junit4/src/test/java/com/google/testing/junit/testparameterinjector/ParameterValueParsingTest.java b/junit4/src/test/java/com/google/testing/junit/testparameterinjector/ParameterValueParsingTest.java
index a9336b7..6364ec0 100644
--- a/junit4/src/test/java/com/google/testing/junit/testparameterinjector/ParameterValueParsingTest.java
+++ b/junit4/src/test/java/com/google/testing/junit/testparameterinjector/ParameterValueParsingTest.java
@@ -15,6 +15,7 @@
 package com.google.testing.junit.testparameterinjector;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Optional;
@@ -127,6 +128,19 @@ public class ParameterValueParsingTest {
         /* yamlString= */ "AAA",
         /* javaClass= */ TestEnum.class,
         /* expectedResult= */ TestEnum.AAA),
+    BOOLEAN_TO_ENUM_FALSE(
+        /* yamlString= */ "NO", /* javaClass= */ TestEnum.class, /* expectedResult= */ TestEnum.NO),
+    BOOLEAN_TO_ENUM_TRUE(
+        /* yamlString= */ "TRUE",
+        /* javaClass= */ TestEnum.class,
+        /* expectedResult= */ TestEnum.TRUE),
+    // This works because the YAML parser in between makes it impossible to differentiate. This test
+    // case is not testing desired behavior, but rather double-checking that the YAML parsing step
+    // actually happens and we are testing this edge case.
+    BOOLEAN_TO_ENUM_TRUE_DIFFERENT_ALIAS(
+        /* yamlString= */ "ON",
+        /* javaClass= */ TestEnum.class,
+        /* expectedResult= */ TestEnum.TRUE),
 
     STRING_TO_BYTES(
         /* yamlString= */ "data",
@@ -169,6 +183,24 @@ public class ParameterValueParsingTest {
     assertThat(result).isEqualTo(parseYamlValueToJavaTypeCases.expectedResult);
   }
 
+  @Test
+  public void parseYamlStringToJavaType_booleanToEnum_ambiguousValues_fails(
+      @TestParameter({"OFF", "YES", "false", "True"}) String yamlString) throws Exception {
+    IllegalArgumentException exception =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                ParameterValueParsing.parseYamlStringToJavaType(
+                    yamlString, TestEnumWithAmbiguousValues.class));
+
+    assertThat(exception)
+        .hasCauseThat()
+        .hasMessageThat()
+        .contains(
+            "It is likely that the YAML parser is 'wrongly' parsing one of these values as"
+                + " boolean");
+  }
+
   enum FormatTestNameStringTestCases {
     NULL_REFERENCE(/* value= */ null, /* expectedResult= */ "param=null"),
     BOOLEAN(/* value= */ false, /* expectedResult= */ "param=false"),
@@ -201,6 +233,17 @@ public class ParameterValueParsingTest {
 
   private enum TestEnum {
     AAA,
-    BBB;
+    BBB,
+    NO,
+    TRUE;
+  }
+
+  private enum TestEnumWithAmbiguousValues {
+    AAA,
+    BBB,
+    NO,
+    OFF,
+    YES,
+    TRUE;
   }
 }
diff --git a/junit5/src/main/java/com/google/testing/junit/testparameterinjector/junit5/ParameterValueParsing.java b/junit5/src/main/java/com/google/testing/junit/testparameterinjector/junit5/ParameterValueParsing.java
index 130c186..fc26088 100644
--- a/junit5/src/main/java/com/google/testing/junit/testparameterinjector/junit5/ParameterValueParsing.java
+++ b/junit5/src/main/java/com/google/testing/junit/testparameterinjector/junit5/ParameterValueParsing.java
@@ -17,10 +17,12 @@ package com.google.testing.junit.testparameterinjector.junit5;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Iterables.getOnlyElement;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.primitives.Primitives;
 import com.google.common.primitives.UnsignedLong;
@@ -31,10 +33,12 @@ import java.lang.reflect.ParameterizedType;
 import java.math.BigInteger;
 import java.nio.charset.Charset;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import javax.annotation.Nullable;
 import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.Yaml;
@@ -131,6 +135,11 @@ final class ParameterValueParsing {
     yamlValueTransformer
         .ifJavaType(Enum.class)
         .supportParsedType(
+            Boolean.class,
+            bool ->
+                ParameterValueParsing.parseEnumIfUnambiguousYamlBoolean(
+                    bool, javaType.getRawType()))
+        .supportParsedType(
             String.class, str -> ParameterValueParsing.parseEnum(str, javaType.getRawType()));
 
     yamlValueTransformer
@@ -166,6 +175,42 @@ final class ParameterValueParsing {
     return yamlValueTransformer.transformedJavaValue();
   }
 
+  private static Enum<?> parseEnumIfUnambiguousYamlBoolean(boolean yamlValue, Class<?> enumType) {
+    Set<String> negativeYamlStrings =
+        ImmutableSet.of("false", "False", "FALSE", "n", "N", "no", "No", "NO", "off", "Off", "OFF");
+    Set<String> positiveYamlStrings =
+        ImmutableSet.of("on", "On", "ON", "true", "True", "TRUE", "y", "Y", "yes", "Yes", "YES");
+
+    // This is the list of YAML strings that a user could have used to define this boolean. Since
+    // the user probably didn't intend a boolean but an enum (since we're expecting an enum), one of
+    // these strings may (unambiguously) match one of the enum values.
+    Set<String> yamlStringCandidates = yamlValue ? positiveYamlStrings : negativeYamlStrings;
+
+    Set<Enum<?>> matches = new HashSet<>();
+    for (Object enumValueObject : enumType.getEnumConstants()) {
+      Enum<?> enumValue = (Enum<?>) enumValueObject;
+      if (yamlStringCandidates.contains(enumValue.name())) {
+        matches.add(enumValue);
+      }
+    }
+
+    checkArgument(
+        !matches.isEmpty(),
+        "Cannot cast a boolean (%s) to an enum of type %s.",
+        yamlValue,
+        enumType.getSimpleName());
+    checkArgument(
+        matches.size() == 1,
+        "Cannot cast a boolean (%s) to an enum of type %s. It is likely that the YAML parser is"
+            + " 'wrongly' parsing one of these values as boolean: %s. You can solve this by putting"
+            + " quotes around the YAML value, forcing the YAML parser to parse a String, which can"
+            + " then be converted to the enum.",
+        yamlValue,
+        enumType.getSimpleName(),
+        matches);
+    return getOnlyElement(matches);
+  }
+
   private static Map<?, ?> parseYamlMapToJavaMap(Map<?, ?> map, TypeToken<?> javaType) {
     Map<Object, Object> returnedMap = new LinkedHashMap<>();
     for (Entry<?, ?> entry : map.entrySet()) {