Make merging work properly with substitutions involved.

If we saw a ConfigSubstitution value, we would then
ignore any objects after it. But in fact, the substitution
might expand to an object, and then we would need to merge
it with the objects after it.

If we had an object and merged a substitution with it, we
were previously ignoring the substitution. But in fact,
the substitution might expand to an object, and we would
need to merge that object in.

So in both cases now we create a ConfigDelayedMerge
or ConfigDelayedMergeObject object instead.

As part of this, the merge() code was refactored to
use a withFallback() method, which is now handy
and public.

src/main/java/com/typesafe/config/ConfigException.java

@@ -198,4 +198,23 @@ public class ConfigException extends RuntimeException {
             this(origin, message, null);
         }
     }
+
+    /**
+     * Exception indicating that you tried to use a function that requires
+     * substitutions to be resolved, but substitutions have not been resolved.
+     * This is always a bug in either application code or the library; it's
+     * wrong to write a handler for this exception because you should be able to
+     * fix the code to avoid it.
+     */
+    public static class NotResolved extends BugOrBroken {
+        private static final long serialVersionUID = 1L;
+
+        public NotResolved(String message, Throwable cause) {
+            super(message, cause);
+        }
+
+        public NotResolved(String message) {
+            this(message, null);
+        }
+    }
 }

src/main/java/com/typesafe/config/ConfigObject.java

@@ -32,6 +32,9 @@ public interface ConfigObject extends ConfigValue, Map<String, ConfigValue> {
     @Override
     Map<String, Object> unwrapped();
 
+    @Override
+    ConfigObject withFallback(ConfigValue other);
+
     boolean getBoolean(String path);
 
     Number getNumber(String path);

src/main/java/com/typesafe/config/ConfigValue.java

@@ -1,5 +1,6 @@
 package com.typesafe.config;
 
+
 /**
  * Interface implemented by any configuration value. From the perspective of
  * users of this interface, the object is immutable. It is therefore safe to use
@@ -26,4 +27,19 @@ public interface ConfigValue {
      * a ConfigObject or ConfigList, it is recursively unwrapped.
      */
     Object unwrapped();
+
+    /**
+     * Returns a new value computed by merging this value with another, with
+     * keys in this value "winning" over the other one. Only ConfigObject has
+     * anything to do in this method (it merges the fallback keys into itself).
+     * All other values just return the original value, since they automatically
+     * override any fallback.
+     *
+     * @param other
+     *            an object whose keys should be used if the keys are not
+     *            present in this one
+     * @return a new object (or the original one, if the fallback doesn't get
+     *         used)
+     */
+    ConfigValue withFallback(ConfigValue other);
 }

src/main/java/com/typesafe/config/impl/AbstractConfigObject.java

@@ -3,8 +3,10 @@ package com.typesafe.config.impl;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import com.typesafe.config.Config;
@@ -100,16 +102,12 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
         return ConfigValueType.OBJECT;
     }
 
-    static AbstractConfigObject transformed(AbstractConfigObject obj,
-            ConfigTransformer transformer) {
-        if (obj.transformer != transformer)
-            return new TransformedConfigObject(transformer, obj);
+    @Override
+    AbstractConfigObject transformed(ConfigTransformer newTransformer) {
+        if (this.transformer != newTransformer)
+            return new TransformedConfigObject(newTransformer, this);
         else
-            return obj;
-    }
-
-    private AbstractConfigObject transformed(AbstractConfigObject obj) {
-        return transformed(obj, transformer);
+            return this;
     }
 
     static private AbstractConfigValue resolve(AbstractConfigObject self,
@@ -162,6 +160,41 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
                 originalPath);
     }
 
+    @Override
+    public AbstractConfigObject withFallback(ConfigValue other) {
+        if (other instanceof Unresolved) {
+            List<AbstractConfigValue> stack = new ArrayList<AbstractConfigValue>();
+            stack.add(this);
+            stack.addAll(((Unresolved) other).unmergedValues());
+            return new ConfigDelayedMergeObject(origin(), transformer, stack);
+        } else if (other instanceof AbstractConfigObject) {
+            AbstractConfigObject fallback = (AbstractConfigObject) other;
+            if (fallback.isEmpty()) {
+                return this; // nothing to do
+            } else {
+                Map<String, AbstractConfigValue> merged = new HashMap<String, AbstractConfigValue>();
+                Set<String> allKeys = new HashSet<String>();
+                allKeys.addAll(this.keySet());
+                allKeys.addAll(fallback.keySet());
+                for (String key : allKeys) {
+                    AbstractConfigValue first = this.peek(key);
+                    AbstractConfigValue second = fallback.peek(key);
+                    if (first == null)
+                        merged.put(key, second);
+                    else if (second == null)
+                        merged.put(key, first);
+                    else
+                        merged.put(key, first.withFallback(second));
+                }
+                return new SimpleConfigObject(origin(), transformer, merged);
+            }
+        } else {
+            // falling back to a non-object has no effect, we just override
+            // primitive values.
+            return this;
+        }
+    }
+
     /**
      * Stack should be from overrides to fallbacks (earlier items win). Objects
      * have their keys combined into a new object, while other kinds of value
@@ -174,45 +207,13 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
             return new SimpleConfigObject(origin, transformer,
                     Collections.<String, AbstractConfigValue> emptyMap());
         } else if (stack.size() == 1) {
-            return transformed(stack.get(0), transformer);
+            return stack.get(0).transformed(transformer);
         } else {
-            // for non-objects, we just take the first value; but for objects we
-            // have to do work to combine them.
-            Map<String, AbstractConfigValue> merged = new HashMap<String, AbstractConfigValue>();
-            Map<String, List<AbstractConfigObject>> objects = new HashMap<String, List<AbstractConfigObject>>();
-            for (AbstractConfigObject obj : stack) {
-                for (String key : obj.keySet()) {
-                    AbstractConfigValue v = obj.peek(key);
-                    if (!merged.containsKey(key)) {
-                        if (v.valueType() == ConfigValueType.OBJECT) {
-                            // requires recursive merge and transformer fixup
-                            List<AbstractConfigObject> stackForKey = null;
-                            if (objects.containsKey(key)) {
-                                stackForKey = objects.get(key);
-                            } else {
-                                stackForKey = new ArrayList<AbstractConfigObject>();
-                                objects.put(key, stackForKey);
-                            }
-                            stackForKey.add(transformed(
-                                    (AbstractConfigObject) v,
-                                    transformer));
-                        } else {
-                            if (!objects.containsKey(key)) {
-                                merged.put(key, v);
-                            }
-                        }
-                    }
-                }
+            AbstractConfigObject merged = stack.get(0);
+            for (int i = 1; i < stack.size(); ++i) {
+                merged = merged.withFallback(stack.get(i));
             }
-
-            for (Map.Entry<String, List<AbstractConfigObject>> entry : objects
-                    .entrySet()) {
-                List<AbstractConfigObject> stackForKey = entry.getValue();
-                AbstractConfigObject obj = merge(origin, stackForKey, transformer);
-                merged.put(entry.getKey(), obj);
-            }
-
-            return new SimpleConfigObject(origin, transformer, merged);
+            return merged.transformed(transformer);
         }
     }
 
@@ -300,7 +301,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
     public AbstractConfigObject getObject(String path) {
         AbstractConfigObject obj = (AbstractConfigObject) find(path,
                 ConfigValueType.OBJECT, path);
-        return transformed(obj);
+        return obj.transformed(this.transformer);
     }
 
     @Override
@@ -414,7 +415,7 @@ abstract class AbstractConfigObject extends AbstractConfigValue implements
             if (v.valueType() != ConfigValueType.OBJECT)
                 throw new ConfigException.WrongType(v.origin(), path,
                         ConfigValueType.OBJECT.name(), v.valueType().name());
-            l.add(transformed((AbstractConfigObject) v));
+            l.add(((AbstractConfigObject) v).transformed(this.transformer));
         }
         return l;
     }

src/main/java/com/typesafe/config/impl/AbstractConfigValue.java

@@ -34,6 +34,15 @@ abstract class AbstractConfigValue implements ConfigValue {
         return this;
     }
 
+    @Override
+    public AbstractConfigValue withFallback(ConfigValue other) {
+        return this;
+    }
+
+    AbstractConfigValue transformed(ConfigTransformer transformer) {
+        return this;
+    }
+
     protected boolean canEqual(Object other) {
         return other instanceof ConfigValue;
     }

src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java

@@ -0,0 +1,149 @@
+package com.typesafe.config.impl;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import com.typesafe.config.ConfigException;
+import com.typesafe.config.ConfigOrigin;
+import com.typesafe.config.ConfigValue;
+import com.typesafe.config.ConfigValueType;
+
+/**
+ * The issue here is that we want to first merge our stack of config files, and
+ * then we want to evaluate substitutions. But if two substitutions both expand
+ * to an object, we might need to merge those two objects. Thus, we can't ever
+ * "override" a substitution when we do a merge; instead we have to save the
+ * stack of values that should be merged, and resolve the merge when we evaluate
+ * substitutions.
+ */
+final class ConfigDelayedMerge extends AbstractConfigValue implements
+        Unresolved {
+
+    // earlier items in the stack win
+    final private List<AbstractConfigValue> stack;
+
+    ConfigDelayedMerge(ConfigOrigin origin, List<AbstractConfigValue> stack) {
+        super(origin);
+        this.stack = stack;
+        if (stack.isEmpty())
+            throw new ConfigException.BugOrBroken(
+                    "creating empty delayed merge value");
+
+    }
+
+    @Override
+    public ConfigValueType valueType() {
+        throw new ConfigException.NotResolved(
+                "called valueType() on value with unresolved substitutions, need to resolve first");
+    }
+
+    @Override
+    public Object unwrapped() {
+        throw new ConfigException.NotResolved(
+                "called unwrapped() on value with unresolved substitutions, need to resolve first");
+    }
+
+    @Override
+    AbstractConfigValue resolveSubstitutions(SubstitutionResolver resolver,
+            int depth, boolean withFallbacks) {
+        return resolveSubstitutions(stack, resolver, depth, withFallbacks);
+    }
+
+    // static method also used by ConfigDelayedMergeObject
+    static AbstractConfigValue resolveSubstitutions(
+            List<AbstractConfigValue> stack, SubstitutionResolver resolver,
+            int depth, boolean withFallbacks) {
+        // to resolve substitutions, we need to recursively resolve
+        // the stack of stuff to merge, and then merge the stack.
+        List<AbstractConfigObject> toMerge = new ArrayList<AbstractConfigObject>();
+
+        for (AbstractConfigValue v : stack) {
+            AbstractConfigValue resolved = resolver.resolve(v, depth,
+                    withFallbacks);
+
+            if (resolved instanceof AbstractConfigObject) {
+                toMerge.add((AbstractConfigObject) resolved);
+            } else {
+                if (toMerge.isEmpty()) {
+                    // done, we'll ignore any objects anyway since we
+                    // now have a non-object value. There is a semantic
+                    // effect to this optimization though: we won't detect
+                    // any cycles or other errors in the objects we are
+                    // not resolving. In other cases (without substitutions
+                    // involved) we might detect those errors.
+                    return resolved;
+                } else {
+                    // look for more objects to merge, since once we have
+                    // an object we merge all objects even if there are
+                    // intervening non-objects.
+                    continue;
+                }
+            }
+        }
+
+        return AbstractConfigObject.merge(toMerge.get(0).origin(), toMerge,
+                toMerge.get(0).transformer);
+    }
+
+    @Override
+    public AbstractConfigValue withFallback(ConfigValue other) {
+        if (other instanceof AbstractConfigObject
+                || other instanceof Unresolved) {
+            // if we turn out to be an object, and the fallback also does,
+            // then a merge may be required; delay until we resolve.
+            List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>();
+            newStack.addAll(stack);
+            if (other instanceof Unresolved)
+                newStack.addAll(((Unresolved) other).unmergedValues());
+            else
+                newStack.add((AbstractConfigValue) other);
+            return new ConfigDelayedMerge(origin(), newStack);
+        } else {
+            // if the other is not an object, there won't be anything
+            // to merge with, so we are it even if we are an object.
+            return this;
+        }
+    }
+
+    @Override
+    public Collection<AbstractConfigValue> unmergedValues() {
+        return stack;
+    }
+
+    @Override
+    protected boolean canEqual(Object other) {
+        return other instanceof ConfigDelayedMerge;
+    }
+
+    @Override
+    public boolean equals(Object other) {
+        // note that "origin" is deliberately NOT part of equality
+        if (other instanceof ConfigDelayedMerge) {
+            return canEqual(other)
+                    && this.stack.equals(((ConfigDelayedMerge) other).stack);
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public int hashCode() {
+        // note that "origin" is deliberately NOT part of equality
+        return stack.hashCode();
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("DELAYED_MERGE");
+        sb.append("(");
+        for (Object s : stack) {
+            sb.append(s.toString());
+            sb.append(",");
+        }
+        sb.setLength(sb.length() - 1); // chop comma
+        sb.append(")");
+        return sb.toString();
+    }
+}

src/main/java/com/typesafe/config/impl/ConfigDelayedMergeObject.java

@@ -0,0 +1,157 @@
+package com.typesafe.config.impl;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import com.typesafe.config.ConfigException;
+import com.typesafe.config.ConfigOrigin;
+import com.typesafe.config.ConfigValue;
+
+// This is just like ConfigDelayedMerge except we know statically
+// that it will turn out to be an object.
+final class ConfigDelayedMergeObject extends AbstractConfigObject implements
+        Unresolved {
+
+    final private List<AbstractConfigValue> stack;
+
+    ConfigDelayedMergeObject(ConfigOrigin origin,
+            ConfigTransformer transformer, List<AbstractConfigValue> stack) {
+        super(origin, transformer);
+        this.stack = stack;
+        if (stack.isEmpty())
+            throw new ConfigException.BugOrBroken(
+                    "creating empty delayed merge object");
+        if (!(stack.get(0) instanceof AbstractConfigObject))
+            throw new ConfigException.BugOrBroken(
+                    "created a delayed merge object not guaranteed to be an object");
+    }
+
+    @Override
+    AbstractConfigObject resolveSubstitutions(SubstitutionResolver resolver,
+            int depth, boolean withFallbacks) {
+        AbstractConfigValue merged = ConfigDelayedMerge.resolveSubstitutions(
+                stack, resolver, depth,
+                withFallbacks);
+        if (merged instanceof AbstractConfigObject) {
+            return (AbstractConfigObject) merged;
+        } else {
+            throw new ConfigException.BugOrBroken(
+                    "somehow brokenly merged an object and didn't get an object");
+        }
+    }
+
+    @Override
+    public AbstractConfigObject withFallback(ConfigValue other) {
+        if (other instanceof AbstractConfigObject
+                || other instanceof Unresolved) {
+            // since we are an object, and the fallback could be,
+            // then a merge may be required; delay until we resolve.
+            List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>();
+            newStack.addAll(stack);
+            if (other instanceof Unresolved)
+                newStack.addAll(((Unresolved) other).unmergedValues());
+            else
+                newStack.add((AbstractConfigValue) other);
+            return new ConfigDelayedMergeObject(origin(), transformer, newStack);
+        } else {
+            // if the other is not an object, there won't be anything
+            // to merge with.
+            return this;
+        }
+    }
+
+    @Override
+    public Collection<AbstractConfigValue> unmergedValues() {
+        return stack;
+    }
+
+    @Override
+    protected boolean canEqual(Object other) {
+        return other instanceof ConfigDelayedMergeObject;
+    }
+
+    @Override
+    public boolean equals(Object other) {
+        // note that "origin" is deliberately NOT part of equality
+        if (other instanceof ConfigDelayedMergeObject) {
+            return canEqual(other)
+                    && this.stack
+                            .equals(((ConfigDelayedMergeObject) other).stack);
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public int hashCode() {
+        // note that "origin" is deliberately NOT part of equality
+        return stack.hashCode();
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("DELAYED_MERGE_OBJECT");
+        sb.append("(");
+        for (Object s : stack) {
+            sb.append(s.toString());
+            sb.append(",");
+        }
+        sb.setLength(sb.length() - 1); // chop comma
+        sb.append(")");
+        return sb.toString();
+    }
+
+    private static ConfigException notResolved() {
+        return new ConfigException.NotResolved(
+                "bug: this object has not had substitutions resolved, so can't be used");
+    }
+
+    @Override
+    public Map<String, Object> unwrapped() {
+        throw notResolved();
+    }
+
+    @Override
+    public boolean containsKey(Object key) {
+        throw notResolved();
+    }
+
+    @Override
+    public boolean containsValue(Object value) {
+        throw notResolved();
+    }
+
+    @Override
+    public Set<java.util.Map.Entry<String, ConfigValue>> entrySet() {
+        throw notResolved();
+    }
+
+    @Override
+    public boolean isEmpty() {
+        throw notResolved();
+    }
+
+    @Override
+    public Set<String> keySet() {
+        throw notResolved();
+    }
+
+    @Override
+    public int size() {
+        throw notResolved();
+    }
+
+    @Override
+    public Collection<ConfigValue> values() {
+        throw notResolved();
+    }
+
+    @Override
+    protected AbstractConfigValue peek(String key) {
+        throw notResolved();
+    }
+}

src/main/java/com/typesafe/config/impl/ConfigImpl.java

@@ -46,15 +46,13 @@ public class ConfigImpl {
     static ConfigObject getEnvironmentAsConfig() {
         // This should not need to create a new config object
         // as long as the transformer is just the default transformer.
-        return AbstractConfigObject.transformed(envVariablesConfig(),
-                withExtraTransformer(null));
+        return envVariablesConfig().transformed(withExtraTransformer(null));
     }
 
     static ConfigObject getSystemPropertiesAsConfig() {
         // This should not need to create a new config object
         // as long as the transformer is just the default transformer.
-        return AbstractConfigObject.transformed(systemPropertiesConfig(),
-                withExtraTransformer(null));
+        return systemPropertiesConfig().transformed(withExtraTransformer(null));
     }
 
     private static ConfigTransformer withExtraTransformer(

src/main/java/com/typesafe/config/impl/ConfigSubstitution.java

@@ -1,5 +1,8 @@
 package com.typesafe.config.impl;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import com.typesafe.config.ConfigException;
@@ -12,7 +15,8 @@ import com.typesafe.config.ConfigValueType;
  * it can resolve to a value of any type, though if the substitution has more
  * than one piece it always resolves to a string via value concatenation.
  */
-final class ConfigSubstitution extends AbstractConfigValue {
+final class ConfigSubstitution extends AbstractConfigValue implements
+        Unresolved {
 
     // this is a list of String and Path where the Path
     // have to be resolved to values, then if there's more
@@ -26,14 +30,40 @@ final class ConfigSubstitution extends AbstractConfigValue {
 
     @Override
     public ConfigValueType valueType() {
-        throw new ConfigException.BugOrBroken(
-                "tried to get value type on a ConfigSubstitution; need to resolve substitution first");
+        throw new ConfigException.NotResolved(
+                "tried to get value type on an unresolved substitution: "
+                        + this);
     }
 
     @Override
     public Object unwrapped() {
-        throw new ConfigException.BugOrBroken(
-                "tried to unwrap a ConfigSubstitution; need to resolve substitution first");
+        throw new ConfigException.NotResolved(
+                "tried to unwrap an unresolved substitution: " + this);
+    }
+
+    @Override
+    public AbstractConfigValue withFallback(ConfigValue other) {
+        if (other instanceof AbstractConfigObject
+                || other instanceof Unresolved) {
+            // if we turn out to be an object, and the fallback also does,
+            // then a merge may be required; delay until we resolve.
+            List<AbstractConfigValue> newStack = new ArrayList<AbstractConfigValue>();
+            newStack.add(this);
+            if (other instanceof Unresolved)
+                newStack.addAll(((Unresolved) other).unmergedValues());
+            else
+                newStack.add((AbstractConfigValue) other);
+            return new ConfigDelayedMerge(origin(), newStack);
+        } else {
+            // if the other is not an object, there won't be anything
+            // to merge with, so we are it even if we are an object.
+            return this;
+        }
+    }
+
+    @Override
+    public Collection<ConfigSubstitution> unmergedValues() {
+        return Collections.singleton(this);
     }
 
     List<Object> pieces() {

src/main/java/com/typesafe/config/impl/TransformedConfigObject.java

@@ -4,6 +4,7 @@ import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 
+import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigValue;
 
 final class TransformedConfigObject extends AbstractConfigObject {
@@ -14,6 +15,9 @@ final class TransformedConfigObject extends AbstractConfigObject {
             AbstractConfigObject underlying) {
         super(underlying.origin(), transformer);
         this.underlying = underlying;
+        if (transformer == underlying.transformer)
+            throw new ConfigException.BugOrBroken(
+                    "Created unnecessary TransformedConfigObject");
     }
 
     @Override

src/main/java/com/typesafe/config/impl/Unresolved.java

@@ -0,0 +1,13 @@
+package com.typesafe.config.impl;
+
+import java.util.Collection;
+
+/**
+ * Interface that tags a ConfigValue that is inherently not resolved; i.e. a
+ * subclass of ConfigValue that will not appear in a resolved tree of values.
+ * Types such as ConfigObject may not be resolved _yet_, but they are not
+ * inherently unresolved.
+ */
+interface Unresolved {
+    Collection<? extends AbstractConfigValue> unmergedValues();
+}

src/test/scala/com/typesafe/config/impl/ConfigTest.scala

@@ -12,6 +12,59 @@ import com.typesafe.config.ConfigConfig
 
 class ConfigTest extends TestUtils {
 
+    def merge(toMerge: AbstractConfigObject*) = {
+        AbstractConfigObject.merge(fakeOrigin(), toMerge.toList.asJava, null)
+    }
+
+    // In many cases, we expect merging to be associative. It is
+    // not, however, when an object value is the first value,
+    // a non-object value follows, and then an object after that;
+    // in that case, if an association starts with the non-object
+    // value, then the value after the non-object value gets lost.
+    private def associativeMerge(allObjects: Seq[AbstractConfigObject])(assertions: AbstractConfigObject => Unit) {
+        def m(toMerge: AbstractConfigObject*) = merge(toMerge: _*)
+
+        def makeTrees(objects: Seq[AbstractConfigObject]): Iterator[AbstractConfigObject] = {
+            objects.length match {
+                case 0 => Iterator.empty
+                case 1 => {
+                    Iterator(objects(0))
+                }
+                case 2 => {
+                    Iterator(m(objects(0), objects(1)))
+                }
+                case 3 => {
+                    Seq(m(m(objects(0), objects(1)), objects(2)),
+                        m(objects(0), m(objects(1), objects(2))),
+                        m(objects(0), objects(1), objects(2))).iterator
+                }
+                case n => {
+                    // obviously if n gets very high we will be sad ;-)
+                    val trees = for {
+                        i <- (1 until n)
+                        val pair = objects.splitAt(i)
+                        first <- makeTrees(pair._1)
+                        second <- makeTrees(pair._2)
+                    } yield m(first, second)
+                    Iterator(m(objects: _*)) ++ trees.iterator
+                }
+            }
+        }
+
+        // the extra m(allObjects: _*) here is redundant but want to
+        // be sure we do it first, and guard against makeTrees
+        // being insane.
+        val trees = Seq(m(allObjects: _*)) ++ makeTrees(allObjects)
+        for (tree <- trees) {
+            // if this fails, we were not associative.
+            assertEquals(trees(0), tree)
+        }
+
+        for (tree <- trees) {
+            assertions(tree)
+        }
+    }
+
     @Test
     def mergeTrivial() {
         val obj1 = parseObject("""{ "a" : 1 }""")
@@ -99,6 +152,21 @@ class ConfigTest extends TestUtils {
         assertEquals(3, merged.getObject("root").keySet().size)
     }
 
+    @Test
+    def mergeWithEmpty() {
+        val obj1 = parseObject("""{ "a" : 1 }""")
+        val obj2 = parseObject("""{ }""")
+        val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null)
+
+        assertEquals(1, merged.getInt("a"))
+        assertEquals(1, merged.keySet().size)
+
+        val merged2 = AbstractConfigObject.merge(fakeOrigin(), List(obj2, obj1).asJava, null)
+
+        assertEquals(1, merged2.getInt("a"))
+        assertEquals(1, merged2.keySet().size)
+    }
+
     @Test
     def mergeOverrideObjectAndPrimitive() {
         val obj1 = parseObject("""{ "a" : 1 }""")
@@ -118,27 +186,192 @@ class ConfigTest extends TestUtils {
 
     @Test
     def mergeObjectThenPrimitiveThenObject() {
+        // the semantic here is that the primitive gets ignored, because
+        // it can't be merged with the object. But potentially it should
+        // throw an exception even, or warn.
         val obj1 = parseObject("""{ "a" : { "b" : 42 } }""")
         val obj2 = parseObject("""{ "a" : 2 }""")
-        val obj3 = parseObject("""{ "a" : { "b" : 43 } }""")
-
-        val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2, obj3).asJava, null)
+        val obj3 = parseObject("""{ "a" : { "b" : 43, "c" : 44 } }""")
 
+        val merged = merge(obj1, obj2, obj3)
         assertEquals(42, merged.getInt("a.b"))
-        assertEquals(1, merged.keySet().size)
-        assertEquals(1, merged.getObject("a").keySet().size())
+        assertEquals(1, merged.size)
+        assertEquals(44, merged.getInt("a.c"))
+        assertEquals(2, merged.getObject("a").size())
     }
 
     @Test
     def mergePrimitiveThenObjectThenPrimitive() {
+        // the primitive should override the object
         val obj1 = parseObject("""{ "a" : 1 }""")
         val obj2 = parseObject("""{ "a" : { "b" : 42 } }""")
         val obj3 = parseObject("""{ "a" : 3 }""")
 
-        val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2, obj3).asJava, null)
+        associativeMerge(Seq(obj1, obj2, obj3)) { merged =>
+            assertEquals(1, merged.getInt("a"))
+            assertEquals(1, merged.keySet().size)
+        }
+    }
 
-        assertEquals(1, merged.getInt("a"))
-        assertEquals(1, merged.keySet().size)
+    @Test
+    def mergeSubstitutedValues() {
+        val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : ${a} }""")
+        val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""")
+
+        val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null)
+        val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+            case x: ConfigObject => x
+        }
+
+        assertEquals(3, resolved.getObject("c").size())
+        assertEquals(1, resolved.getInt("c.x"))
+        assertEquals(2, resolved.getInt("c.y"))
+        assertEquals(4, resolved.getInt("c.z"))
+    }
+
+    @Test
+    def mergeObjectWithSubstituted() {
+        val obj1 = parseObject("""{ "a" : { "x" : 1, "z" : 4 }, "c" : { "z" : 42 } }""")
+        val obj2 = parseObject("""{ "b" : { "y" : 2, "z" : 5 }, "c" : ${b} }""")
+
+        val merged = AbstractConfigObject.merge(fakeOrigin(), List(obj1, obj2).asJava, null)
+        val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+            case x: ConfigObject => x
+        }
+
+        assertEquals(2, resolved.getObject("c").size())
+        assertEquals(2, resolved.getInt("c.y"))
+        assertEquals(42, resolved.getInt("c.z"))
+
+        val merged2 = AbstractConfigObject.merge(fakeOrigin(), List(obj2, obj1).asJava, null)
+        val resolved2 = SubstitutionResolver.resolveWithoutFallbacks(merged2, merged2) match {
+            case x: ConfigObject => x
+        }
+
+        assertEquals(2, resolved2.getObject("c").size())
+        assertEquals(2, resolved2.getInt("c.y"))
+        assertEquals(5, resolved2.getInt("c.z"))
+    }
+
+    private val cycleObject = {
+        parseObject("""
+{
+    "foo" : ${bar},
+    "bar" : ${a.b.c},
+    "a" : { "b" : { "c" : ${foo} } }
+}
+""")
+    }
+
+    @Test
+    def mergeHidesCycles() {
+        // the point here is that we should not try to evaluate a substitution
+        // that's been overridden, and thus not end up with a cycle as long
+        // as we override the problematic link in the cycle.
+        val e = intercept[ConfigException.BadValue] {
+            val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), cycleObject)
+        }
+        assertTrue(e.getMessage().contains("cycle"))
+
+        val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : 57 } } } """)
+        val merged = AbstractConfigObject.merge(fakeOrigin(), List(fixUpCycle, cycleObject).asJava, null)
+        val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), merged)
+        assertEquals(intValue(57), v);
+    }
+
+    @Test
+    def mergeWithObjectInFrontKeepsCycles() {
+        // the point here is that if our eventual value will be an object, then
+        // we have to evaluate the substitution to see if it's an object to merge,
+        // so we don't avoid the cycle.
+        val e = intercept[ConfigException.BadValue] {
+            val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), cycleObject)
+        }
+        assertTrue(e.getMessage().contains("cycle"))
+
+        val fixUpCycle = parseObject(""" { "a" : { "b" : { "c" : { "q" : "u" } } } } """)
+        val merged = AbstractConfigObject.merge(fakeOrigin(), List(fixUpCycle, cycleObject).asJava, null)
+        val e2 = intercept[ConfigException.BadValue] {
+            val v = SubstitutionResolver.resolveWithoutFallbacks(subst("foo"), merged)
+        }
+        assertTrue(e2.getMessage().contains("cycle"))
+    }
+
+    @Test
+    def mergeSeriesOfSubstitutions() {
+        val obj1 = parseObject("""{ "a" : { "x" : 1, "q" : 4 }, "j" : ${a} }""")
+        val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""")
+        val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""")
+
+        associativeMerge(Seq(obj1, obj2, obj3)) { merged =>
+            val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+                case x: ConfigObject => x
+            }
+
+            assertEquals(4, resolved.getObject("j").size())
+            assertEquals(1, resolved.getInt("j.x"))
+            assertEquals(2, resolved.getInt("j.y"))
+            assertEquals(3, resolved.getInt("j.z"))
+            assertEquals(4, resolved.getInt("j.q"))
+        }
+    }
+
+    @Test
+    def mergePrimitiveAndTwoSubstitutions() {
+        val obj1 = parseObject("""{ "j" : 42 }""")
+        val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""")
+        val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""")
+
+        associativeMerge(Seq(obj1, obj2, obj3)) { merged =>
+            val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+                case x: ConfigObject => x
+            }
+
+            assertEquals(3, resolved.size())
+            assertEquals(42, resolved.getInt("j"));
+            assertEquals(2, resolved.getInt("b.y"))
+            assertEquals(3, resolved.getInt("c.z"))
+        }
+    }
+
+    @Test
+    def mergeObjectAndTwoSubstitutions() {
+        val obj1 = parseObject("""{ "j" : { "x" : 1, "q" : 4 } }""")
+        val obj2 = parseObject("""{ "b" : { "y" : 2, "q" : 5 }, "j" : ${b} }""")
+        val obj3 = parseObject("""{ "c" : { "z" : 3, "q" : 6 }, "j" : ${c} }""")
+
+        associativeMerge(Seq(obj1, obj2, obj3)) { merged =>
+            val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+                case x: ConfigObject => x
+            }
+
+            assertEquals(4, resolved.getObject("j").size())
+            assertEquals(1, resolved.getInt("j.x"))
+            assertEquals(2, resolved.getInt("j.y"))
+            assertEquals(3, resolved.getInt("j.z"))
+            assertEquals(4, resolved.getInt("j.q"))
+        }
+    }
+
+    @Test
+    def mergeObjectSubstitutionObjectSubstitution() {
+        val obj1 = parseObject("""{ "j" : { "w" : 1, "q" : 5 } }""")
+        val obj2 = parseObject("""{ "b" : { "x" : 2, "q" : 6 }, "j" : ${b} }""")
+        val obj3 = parseObject("""{ "j" : { "y" : 3, "q" : 7 } }""")
+        val obj4 = parseObject("""{ "c" : { "z" : 4, "q" : 8 }, "j" : ${c} }""")
+
+        associativeMerge(Seq(obj1, obj2, obj3, obj4)) { merged =>
+            val resolved = SubstitutionResolver.resolveWithoutFallbacks(merged, merged) match {
+                case x: ConfigObject => x
+            }
+
+            assertEquals(5, resolved.getObject("j").size())
+            assertEquals(1, resolved.getInt("j.w"))
+            assertEquals(2, resolved.getInt("j.x"))
+            assertEquals(3, resolved.getInt("j.y"))
+            assertEquals(4, resolved.getInt("j.z"))
+            assertEquals(5, resolved.getInt("j.q"))
+        }
     }
 
     @Test

src/test/scala/com/typesafe/config/impl/ConfigValueTest.scala

@@ -7,6 +7,8 @@ import java.util.Collections
 import scala.collection.JavaConverters._
 import com.typesafe.config.ConfigObject
 import com.typesafe.config.ConfigList
+import com.typesafe.config.ConfigException
+import com.typesafe.config.ConfigValueType
 
 class ConfigValueTest extends TestUtils {
 
@@ -91,6 +93,33 @@ class ConfigValueTest extends TestUtils {
         checkNotEqualObjects(a, b)
     }
 
+    @Test
+    def configDelayedMergeEquality() {
+        val s1 = subst("foo")
+        val s2 = subst("bar")
+        val a = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s1, s2).asJava)
+        val sameAsA = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s1, s2).asJava)
+        val b = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](s2, s1).asJava)
+
+        checkEqualObjects(a, a)
+        checkEqualObjects(a, sameAsA)
+        checkNotEqualObjects(a, b)
+    }
+
+    @Test
+    def configDelayedMergeObjectEquality() {
+        val empty = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]())
+        val s1 = subst("foo")
+        val s2 = subst("bar")
+        val a = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s1, s2).asJava)
+        val sameAsA = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s1, s2).asJava)
+        val b = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](empty, s2, s1).asJava)
+
+        checkEqualObjects(a, a)
+        checkEqualObjects(a, sameAsA)
+        checkNotEqualObjects(a, b)
+    }
+
     @Test
     def valuesToString() {
         // just check that these don't throw, the exact output
@@ -101,10 +130,15 @@ class ConfigValueTest extends TestUtils {
         stringValue("hi").toString()
         nullValue().toString()
         boolValue(true).toString()
-        (new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]())).toString()
+        val emptyObj = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]())
+        emptyObj.toString()
         (new SimpleConfigList(fakeOrigin(), Collections.emptyList[AbstractConfigValue]())).toString()
         subst("a").toString()
         substInString("b").toString()
+        val dm = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](subst("a"), subst("b")).asJava)
+        dm.toString()
+        val dmo = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava)
+        dmo.toString()
     }
 
     private def unsupported(body: => Unit) {
@@ -222,4 +256,36 @@ class ConfigValueTest extends TestUtils {
         unsupported { l.retainAll(List[ConfigValue](intValue(1)).asJava) }
         unsupported { l.set(0, intValue(42)) }
     }
+
+    private def unresolved(body: => Unit) {
+        intercept[ConfigException.NotResolved] {
+            body
+        }
+    }
+
+    @Test
+    def notResolvedThrown() {
+        // ConfigSubstitution
+        unresolved { subst("foo").valueType() }
+        unresolved { subst("foo").unwrapped() }
+
+        // ConfigDelayedMerge
+        val dm = new ConfigDelayedMerge(fakeOrigin(), List[AbstractConfigValue](subst("a"), subst("b")).asJava)
+        unresolved { dm.valueType() }
+        unresolved { dm.unwrapped() }
+
+        // ConfigDelayedMergeObject
+        val emptyObj = new SimpleConfigObject(fakeOrigin(), null, Collections.emptyMap[String, AbstractConfigValue]())
+        val dmo = new ConfigDelayedMergeObject(fakeOrigin(), null, List[AbstractConfigValue](emptyObj, subst("a"), subst("b")).asJava)
+        assertEquals(ConfigValueType.OBJECT, dmo.valueType())
+        unresolved { dmo.unwrapped() }
+        unresolved { dmo.containsKey(null) }
+        unresolved { dmo.containsValue(null) }
+        unresolved { dmo.entrySet() }
+        unresolved { dmo.isEmpty() }
+        unresolved { dmo.keySet() }
+        unresolved { dmo.size() }
+        unresolved { dmo.values() }
+        unresolved { dmo.getInt("foo") }
+    }
 }