disallow adding a map to itself

src/main/java/com/hubspot/jinjava/util/ScopeMap.java

@@ -11,6 +11,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
 
 public class ScopeMap<K, V> implements Map<K, V> {
   private final Map<K, V> scope;
@@ -21,7 +22,7 @@ public ScopeMap() {
   }
 
   public ScopeMap(ScopeMap<K, V> parent) {
-    this.scope = new HashMap<K, V>();
+    this.scope = new HashMap<>();
     this.parent = parent;
 
     Set<ScopeMap<K, V>> parents = new HashSet<>();
@@ -111,6 +112,11 @@ public V get(Object key) {
 
   @Override
   public V put(K key, V value) {
+    if (value == this) {
+      throw new IllegalArgumentException(
+        String.format("attempt to put on map with key '%s' and value of itself", key)
+      );
+    }
     return scope.put(key, value);
   }
 
@@ -120,7 +126,18 @@ public V remove(Object key) {
   }
 
   @Override
-  public void putAll(Map<? extends K, ? extends V> m) {
+  public void putAll(@Nonnull Map<? extends K, ? extends V> m) {
+    for (Entry<? extends K, ? extends V> entry : m.entrySet()) {
+      if (entry.getValue() == this) {
+        throw new IllegalArgumentException(
+          String.format(
+            "attempt to putAll on map with key '%s' and value of itself",
+            entry.getKey()
+          )
+        );
+      }
+    }
+
     scope.putAll(m);
   }
 
@@ -130,6 +147,7 @@ public void clear() {
   }
 
   @Override
+  @Nonnull
   public Set<K> keySet() {
     Set<K> keys = new HashSet<>();
 
@@ -143,6 +161,7 @@ public Set<K> keySet() {
   }
 
   @Override
+  @Nonnull
   public Collection<V> values() {
     Set<java.util.Map.Entry<K, V>> entrySet = entrySet();
     Collection<V> values = new ArrayList<>(entrySet.size());
@@ -159,11 +178,12 @@ public Collection<V> values() {
     justification = "using overridden get() to do scoped retrieve with parent fallback",
     value = "WMI_WRONG_MAP_ITERATOR"
   )
+  @Nonnull
   public Set<java.util.Map.Entry<K, V>> entrySet() {
     Set<java.util.Map.Entry<K, V>> entries = new HashSet<>();
 
     for (K key : keySet()) {
-      entries.add(new ScopeMapEntry<K, V>(key, get(key), this));
+      entries.add(new ScopeMapEntry<>(key, get(key), this));
     }
 
     return entries;
@@ -192,10 +212,9 @@ public V getValue() {
 
     @Override
     public V setValue(V value) {
-      V old = value;
       this.value = value;
       map.put(key, value);
-      return old;
+      return value;
     }
   }
 }

src/test/java/com/hubspot/jinjava/util/ScopeMapTest.java

@@ -1,6 +1,7 @@
 package com.hubspot.jinjava.util;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.entry;
 
 import com.google.common.collect.ImmutableMap;
@@ -55,4 +56,21 @@ public void withParent() {
         entry("a4", "vc4")
       );
   }
+
+  @SuppressWarnings("CollectionAddedToSelf")
+  @Test
+  public void itDisallowsPuttingItself() {
+    ScopeMap<Object, Object> map = new ScopeMap<>();
+    assertThatThrownBy(() -> map.put("foo", map))
+      .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void itDisallowsPuttingAllWithItselfAsValue() {
+    ScopeMap<Object, Object> map1 = new ScopeMap<>();
+    ScopeMap<Object, Object> map2 = new ScopeMap<>();
+    map2.put("map1", map1);
+    assertThatThrownBy(() -> map1.putAll(map2))
+      .isInstanceOf(IllegalArgumentException.class);
+  }
 }