Merge pull request #9 from prdoyle/fixes

Fixes for VariantCaseMap and MapValue.copy
boskworks · Jul 26, 2024 · c99383d · c99383d
2 parents 66c7baa + 5613ea0
commit c99383d
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 19 deletions.
diff --git a/bosk-core/src/main/java/works/bosk/MapValue.java b/bosk-core/src/main/java/works/bosk/MapValue.java
@@ -63,7 +63,7 @@ public static <VV> MapValue<VV> fromOrderedMap(Map<String, VV> entries) {
 	/**
 	 * Preserves the order, if any, of the entries in <code>map</code>.
 	 */
-	public static <VV> MapValue<VV> copyOf(Map<String, VV> contents) {
+	public static <VV> MapValue<VV> copyOf(Map<String, ? extends VV> contents) {
 		OrderedPMap<String, VV> map = OrderedPMap.from(contents);
 		map.forEach((k,v) -> {
 			requireNonNull(k);

diff --git a/bosk-core/src/main/java/works/bosk/SerializationPlugin.java b/bosk-core/src/main/java/works/bosk/SerializationPlugin.java
@@ -403,25 +403,29 @@ private static void scanForVariantCaseMap(Class<?> nodeClass, AtomicReference<Va
 
 		for (Class<?> c = nodeClass; c != Object.class && c != null; c = c.getSuperclass()) {
 			for (Field f: c.getDeclaredFields()) {
-				if (isStatic(f.getModifiers()) && !isPrivate(f.getModifiers())) {
-					f.setAccessible(true);
-					var annotations = f.getAnnotationsByType(VariantCaseMap.class);
-					if (annotations.length >= 2) {
-						throw new IllegalStateException("Multiple variant case maps for the same class: " + f);
-					}
-					MapValue value;
-					try {
-						value = (MapValue) f.get(null);
-					} catch (IllegalAccessException e) {
-						throw new AssertionError("Field should not be inaccessible: " + f, e);
-					}
-					if (value == null) {
-						throw new NullPointerException("VariantCaseMap cannot be null: " + f);
-					}
-					var old = variantCaseMap.get();
-					boolean success = variantCaseMap.compareAndSet(old, old.plus(nodeClass, value, c));
-					assert success: "Hey who's messing with our AtomicReference?";
+				var annotations = f.getAnnotationsByType(VariantCaseMap.class);
+				if (annotations.length == 0) {
+					// This is not the droid you're looking for
+					continue;
+				} else if (annotations.length >= 2) {
+					throw new IllegalStateException("Multiple variant case maps for the same class: " + f);
+				}
+				if (!isStatic(f.getModifiers()) || isPrivate(f.getModifiers())) {
+					throw new IllegalStateException("The variant case map must be static and final: " + f);
+				}
+				f.setAccessible(true);
+				MapValue value;
+				try {
+					value = (MapValue) f.get(null);
+				} catch (IllegalAccessException e) {
+					throw new AssertionError("Field should not be inaccessible: " + f, e);
+				}
+				if (value == null) {
+					throw new NullPointerException("VariantCaseMap cannot be null: " + f);
 				}
+				var old = variantCaseMap.get();
+				boolean success = variantCaseMap.compareAndSet(old, old.plus(nodeClass, value, c));
+				assert success: "Hey who's messing with our AtomicReference?";
 			}
 		}
 

diff --git a/bosk-core/src/test/java/works/bosk/MapValueTest.java b/bosk-core/src/test/java/works/bosk/MapValueTest.java
@@ -317,6 +317,14 @@ <V> void without_returnsOriginalIfNoEffect(Map<String,V> map, MapValue<V> mapVal
 		assertSame(mapValue, mapValue.without(NONEXISTENT_KEY));
 	}
 
+	@Test
+	void mapOfSubtype_works() {
+		Map<String, String> map = Map.of("key", "value");
+		MapValue<CharSequence> actual = MapValue.copyOf(map);
+		MapValue<CharSequence> expected = MapValue.singleton("key", "value");
+		assertEquals(expected, actual);
+	}
+
 	@Test
 	void testBad_duplicateKeysFromFunction() {
 		assertThrows(IllegalArgumentException.class, () -> MapValue.fromFunction(asList("dup", "dup"), Identifier::unique));

diff --git a/bosk-core/src/test/java/works/bosk/TypeValidationTest.java b/bosk-core/src/test/java/works/bosk/TypeValidationTest.java
@@ -2,6 +2,7 @@
 
 import java.lang.reflect.Type;
 import java.util.ArrayList;
+import java.util.Map;
 import java.util.Optional;
 import lombok.AccessLevel;
 import lombok.EqualsAndHashCode;
@@ -38,6 +39,7 @@ class TypeValidationTest {
 			AllowedFieldNames.class,
 			ImplicitReferences_onConstructorParameters.class,
 			ImplicitReferences_onFields.class,
+			ExtraStaticField.class
 			})
 	void testValidRootClasses(Class<?> rootClass) throws InvalidTypeException {
 		TypeValidation.validateType(rootClass);
@@ -261,6 +263,20 @@ public static final class ImplicitReferences_onFields implements Entity {
 		@Enclosing Reference<ImplicitReferences_onFields> enclosingRef;
 	}
 
+	public interface ExtraStaticField extends VariantNode {
+		record Subtype() implements ExtraStaticField {}
+		@Override default String tag() { return ""; }
+
+		@VariantCaseMap MapValue<Class<? extends ExtraStaticField>> CASE_MAP = MapValue.copyOf(Map.of(
+			"subtype", Subtype.class
+		));
+
+		/**
+		 * This is not annotated with @VariantCaseMap so we expect it to be ignored by the scan.
+		 */
+		String EXTRA_FIELD = "ignore me";
+	}
+
 	@Getter @FieldDefaults(level=AccessLevel.PRIVATE, makeFinal=true) @RequiredArgsConstructor
 	public static final class MissingConstructorArgument implements Entity {
 		Identifier id;