Merge fc4b330776 into d0021d1d3b

HOCON.md

@@ -1027,6 +1027,19 @@ Then the `${x}` in "foo.conf", which has been fixed up to
 `${a.x}`, would evaluate to `42` rather than to `10`.
 Substitution happens _after_ parsing the whole configuration.
 
+Includes inside of lists use the list index as a path segment when
+fixing up substitutions.
+
+Example:
+
+    {
+        a : [ { include "foo.conf" } ]
+        a.0.x : 42
+    }
+
+Since the `${x}` in "foo.conf" would be fixed up to `${a.0.x}' we
+get our expected value of 42.
+
 However, there are plenty of cases where the included file might
 intend to refer to the application's root config. For example, to
 get a value from a system property or from the reference

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

@@ -34,11 +34,6 @@ final class ConfigParser {
         final private ConfigOrigin baseOrigin;
         final private LinkedList<Path> pathStack;
 
-        // the number of lists we are inside; this is used to detect the "cannot
-        // generate a reference to a list element" problem, and once we fix that
-        // problem we should be able to get rid of this variable.
-        int arrayCount;
-
         ParseContext(ConfigSyntax flavor, ConfigOrigin origin, ConfigNodeRoot document,
                 FullIncluder includer, ConfigIncludeContext includeContext) {
             lineNumber = 1;
@@ -48,7 +43,6 @@ final class ConfigParser {
             this.includer = includer;
             this.includeContext = includeContext;
             this.pathStack = new LinkedList<Path>();
-            this.arrayCount = 0;
         }
 
         // merge a bunch of adjacent values into one
@@ -95,8 +89,6 @@ final class ConfigParser {
         private AbstractConfigValue parseValue(AbstractConfigNodeValue n, List<String> comments) {
             AbstractConfigValue v;
 
-            int startingArrayCount = arrayCount;
-
             if (n instanceof ConfigNodeSimpleValue) {
                 v = ((ConfigNodeSimpleValue) n).value();
             } else if (n instanceof ConfigNodeObject) {
@@ -114,9 +106,6 @@ final class ConfigParser {
                 comments.clear();
             }
 
-            if (arrayCount != startingArrayCount)
-                throw new ConfigException.BugOrBroken("Bug in config parser: unbalanced array count");
-
             return v;
         }
 
@@ -190,14 +179,6 @@ final class ConfigParser {
                     throw new ConfigException.BugOrBroken("should not be reached");
             }
 
-            // we really should make this work, but for now throwing an
-            // exception is better than producing an incorrect result.
-            // See https://github.com/typesafehub/config/issues/160
-            if (arrayCount > 0 && obj.resolveStatus() != ResolveStatus.RESOLVED)
-                throw parseError("Due to current limitations of the config parser, when an include statement is nested inside a list value, "
-                        + "${} substitutions inside the included file cannot be resolved correctly. Either move the include outside of the list value or "
-                        + "remove the ${} statements from the included file.");
-
             if (!pathStack.isEmpty()) {
                 Path prefix = fullCurrentPath();
                 obj = obj.relativized(prefix);
@@ -243,21 +224,6 @@ final class ConfigParser {
 
                     // path must be on-stack while we parse the value
                     pathStack.push(path);
-                    if (((ConfigNodeField) node).separator() == Tokens.PLUS_EQUALS) {
-                        // we really should make this work, but for now throwing
-                        // an exception is better than producing an incorrect
-                        // result. See
-                        // https://github.com/typesafehub/config/issues/160
-                        if (arrayCount > 0)
-                            throw parseError("Due to current limitations of the config parser, += does not work nested inside a list. "
-                                    + "+= expands to a ${} substitution and the path in ${} cannot currently refer to list elements. "
-                                    + "You might be able to move the += outside of the list and then refer to it from inside the list with ${}.");
-
-                        // because we will put it in an array after the fact so
-                        // we want this to be incremented during the parseValue
-                        // below in order to throw the above exception.
-                        arrayCount += 1;
-                    }
 
                     AbstractConfigNodeValue valueNode;
                     AbstractConfigValue newValue;
@@ -268,7 +234,6 @@ final class ConfigParser {
                     newValue = parseValue(valueNode, comments);
 
                     if (((ConfigNodeField) node).separator() == Tokens.PLUS_EQUALS) {
-                        arrayCount -= 1;
 
                         List<AbstractConfigValue> concat = new ArrayList<AbstractConfigValue>(2);
                         AbstractConfigValue previousRef = new ConfigReference(newValue.origin(),
@@ -349,7 +314,6 @@ final class ConfigParser {
         }
 
         private SimpleConfigList parseArray(ConfigNodeArray n) {
-            arrayCount += 1;
 
             SimpleConfigOrigin arrayOrigin = lineOrigin();
             List<AbstractConfigValue> values = new ArrayList<AbstractConfigValue>();
@@ -359,6 +323,8 @@ final class ConfigParser {
 
             AbstractConfigValue v = null;
 
+            int index = 0;
+
             for (AbstractConfigNode node : n.children()) {
                 if (node instanceof ConfigNodeComment) {
                     comments.add(((ConfigNodeComment) node).commentText());
@@ -379,14 +345,16 @@ final class ConfigParser {
                         values.add(v.withOrigin(v.origin().appendComments(new ArrayList<String>(comments))));
                         comments.clear();
                     }
+                    pathStack.push(new Path(Integer.toString(index)));
+                    index ++;
                     v = parseValue((AbstractConfigNodeValue)node, comments);
+                    pathStack.pop();
                 }
             }
             // There shouldn't be any comments at this point, but add them just in case
             if (v != null) {
                 values.add(v.withOrigin(v.origin().appendComments(new ArrayList<String>(comments))));
             }
-            arrayCount -= 1;
             return new SimpleConfigList(arrayOrigin, values);
         }
 

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

@@ -55,7 +55,7 @@ final class ResolveSource {
         }
     }
 
-    static private ValueWithPath findInObject(AbstractConfigObject obj, Path path) {
+    static private ValueWithPath findInObject(AbstractConfigValue obj, Path path) {
         try {
             // we'll fail if anything along the path can't
             // be looked at without resolving.
@@ -65,19 +65,34 @@ final class ResolveSource {
         }
     }
 
-    static private ValueWithPath findInObject(AbstractConfigObject obj, Path path, Node<Container> parents) {
+    static private ValueWithPath findInObject(AbstractConfigValue obj, Path path, Node<Container> parents) {
         String key = path.first();
         Path next = path.remainder();
         if (ConfigImpl.traceSubstitutionsEnabled())
             ConfigImpl.trace("*** looking up '" + key + "' in " + obj);
-        AbstractConfigValue v = obj.attemptPeekWithPartialResolve(key);
+
-        Node<Container> newParents = parents == null ? new Node<Container>(obj) : parents.prepend(obj);
+        AbstractConfigValue v = null;
+        Node<Container> newParents = null;
+
+        if (obj instanceof AbstractConfigObject) {
+            v = ((AbstractConfigObject) obj).attemptPeekWithPartialResolve(key);
+            newParents = parents == null ? new Node<Container>((AbstractConfigObject) obj) : parents.prepend((AbstractConfigObject) obj);
+        } else if (obj instanceof SimpleConfigList) {
+            int ix;
+            try {
+                ix = Integer.parseInt(key);
+            } catch(Exception e) {
+                throw new ConfigException.BadPath(key, "path for list must be numeric index");
+            }
+            v = ((SimpleConfigList) obj).get(ix);
+            newParents = parents == null ? new Node<Container>((SimpleConfigList) obj) : parents.prepend((SimpleConfigList) obj);
+        }
 
         if (next == null) {
             return new ValueWithPath(v, newParents);
         } else {
-            if (v instanceof AbstractConfigObject) {
+            if (v instanceof AbstractConfigObject || v instanceof SimpleConfigList) {
-                return findInObject((AbstractConfigObject) v, next, newParents);
+                return findInObject(v, next, newParents);
             } else {
                 return new ValueWithPath(null, newParents);
             }

config/src/test/resources/include-from-list.conf

@@ -1,5 +1,10 @@
 // The {} inside the [] is needed because
 // just [ include ] means an array with the
 // string "include" in it.
-a = [ { include "test01.conf" } ]
+a = [
+    {
+       include "test12.conf"
+       replace-key: "replaced"
+     }
+ ]
 

config/src/test/resources/test12.conf

@@ -0,0 +1,3 @@
+// This file is included by include-from-list.conf to verify that includes with substitutions work in arrays
+
+replace-me: ${replace-key}

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

@@ -345,26 +345,19 @@ class ConcatenationTest extends TestUtils {
         assertEquals(Seq(1, 2, 3), conf.getObjectList("x.a").asScala.toList.map(_.toConfig.getInt("b")))
     }
 
-    // We would ideally make this case NOT throw an exception but we need to do some work
-    // to get there, see https://github.com/typesafehub/config/issues/160
     @Test
     def plusEqualsMultipleTimesNestedInArray() {
-        val e = intercept[ConfigException.Parse] {
+        val conf = parseConfig("""x = [ { a += 1, a += 2, a += 3 } ] """).resolve()
-            val conf = parseConfig("""x = [ { a += 1, a += 2, a += 3 } ] """).resolve()
+        assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList)
-            assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList)
-        }
-        assertTrue(e.getMessage.contains("limitation"))
     }
 
-    // We would ideally make this case NOT throw an exception but we need to do some work
-    // to get there, see https://github.com/typesafehub/config/issues/160
     @Test
     def plusEqualsMultipleTimesNestedInPlusEquals() {
-        val e = intercept[ConfigException.Parse] {
+        val e = intercept[ConfigException.BugOrBroken] {
             val conf = parseConfig("""x += { a += 1, a += 2, a += 3 } """).resolve()
             assertEquals(Seq(1, 2, 3), conf.getObjectList("x").asScala.toVector(0).toConfig.getIntList("a").asScala.toList)
         }
-        assertTrue(e.getMessage.contains("limitation"))
+        assertTrue(e.getMessage.contains("did not find"))
     }
 
     // from https://github.com/typesafehub/config/issues/177

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

@@ -850,6 +850,14 @@ class ConfParserTest extends TestUtils {
         assertTrue("including basename URL doesn't load anything", conf.isEmpty())
     }
 
+    @Test
+    def includeWithSubstitutionsFromList() {
+        val conf = ConfigFactory.parseString("include file(" + jsonQuotedResourceFile("include-from-list") + ")")
+
+        val resolved = conf.resolve()
+        assertEquals(resolved.getConfigList("a").get(0).getString("replace-me"), "replaced")
+    }
+
     @Test
     def acceptBOMStartingFile() {
         // BOM at start of file should be ignored

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

@@ -905,17 +905,6 @@ class PublicApiTest extends TestUtils {
         assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("include statements nested"))
     }
 
-    // We would ideally make this case NOT throw an exception but we need to do some work
-    // to get there, see https://github.com/typesafehub/config/issues/160
-    @Test
-    def detectIncludeFromList() {
-        val e = intercept[ConfigException.Parse] {
-            ConfigFactory.load("include-from-list.conf")
-        }
-
-        assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("limitation"))
-    }
-
     @Test
     def missingOverrideResourceFails() {
         assertEquals("config.file is not set", null, System.getProperty("config.file"))