Throw exceptions when we need a reference to an array element

This arose in issue #160

There are two known cases: when we want to relativize references
in included files, and when we use `+=` while inside a list.
Both of those are currently impossible to handle without a way
to generate a reference to a list element.

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

@@ -126,6 +126,10 @@ static private final class ParseContext {
         // used to modify the error message to reflect that
         // someone may think this is .properties format.
         int equalsCount;
+        // 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, Iterator<Token> tokens,
                 FullIncluder includer, ConfigIncludeContext includeContext) {
@@ -138,6 +142,7 @@ static private final class ParseContext {
             this.includeContext = includeContext;
             this.pathStack = new LinkedList<Path>();
             this.equalsCount = 0;
+            this.arrayCount = 0;
         }
 
         static private boolean attractsTrailingComments(Token token) {
@@ -499,6 +504,9 @@ private String addQuoteSuggestion(Path lastPath, boolean insideEquals, String ba
         private AbstractConfigValue parseValue(TokenWithComments t) {
             AbstractConfigValue v;
 
+            int startingArrayCount = arrayCount;
+            int startingEqualsCount = equalsCount;
+
             if (Tokens.isValue(t.token)) {
                 // if we consolidateValueTokens() multiple times then
                 // this value could be a concatenation, object, array,
@@ -519,6 +527,11 @@ private AbstractConfigValue parseValue(TokenWithComments t) {
 
             v = v.withOrigin(t.prependComments(v.origin()));
 
+            if (arrayCount != startingArrayCount)
+                throw new ConfigException.BugOrBroken("Bug in config parser: unbalanced array count");
+            if (equalsCount != startingEqualsCount)
+                throw new ConfigException.BugOrBroken("Bug in config parser: unbalanced equals count");
+
             return v;
         }
 
@@ -678,6 +691,14 @@ private void parseInclude(Map<String, AbstractConfigValue> values) {
                 throw parseError("include keyword is not followed by a quoted string, but by: " + t);
             }
 
+            // 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);
@@ -739,6 +760,21 @@ private AbstractConfigObject parseObject(boolean hadOpenCurly) {
 
                     // path must be on-stack while we parse the value
                     pathStack.push(path);
+                    if (afterKey.token == 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;
+                    }
 
                     TokenWithComments valueToken;
                     AbstractConfigValue newValue;
@@ -767,6 +803,8 @@ private AbstractConfigObject parseObject(boolean hadOpenCurly) {
                     newValue = parseValue(valueToken.prepend(keyToken.comments));
 
                     if (afterKey.token == Tokens.PLUS_EQUALS) {
+                        arrayCount -= 1;
+
                         List<AbstractConfigValue> concat = new ArrayList<AbstractConfigValue>(2);
                         AbstractConfigValue previousRef = new ConfigReference(newValue.origin(),
                                 new SubstitutionExpression(fullCurrentPath(), true /* optional */));
@@ -858,6 +896,8 @@ private AbstractConfigObject parseObject(boolean hadOpenCurly) {
 
         private SimpleConfigList parseArray() {
             // invoked just after the OPEN_SQUARE
+            arrayCount += 1;
+
             SimpleConfigOrigin arrayOrigin = lineOrigin();
             List<AbstractConfigValue> values = new ArrayList<AbstractConfigValue>();
 
@@ -867,6 +907,7 @@ private SimpleConfigList parseArray() {
 
             // special-case the first element
             if (t.token == Tokens.CLOSE_SQUARE) {
+                arrayCount -= 1;
                 return new SimpleConfigList(t.appendComments(arrayOrigin),
                         Collections.<AbstractConfigValue> emptyList());
             } else if (Tokens.isValue(t.token) || t.token == Tokens.OPEN_CURLY
@@ -890,6 +931,7 @@ private SimpleConfigList parseArray() {
                 } else {
                     t = nextTokenIgnoringNewline();
                     if (t.token == Tokens.CLOSE_SQUARE) {
+                        arrayCount -= 1;
                         return new SimpleConfigList(t.appendComments(arrayOrigin), values);
                     } else {
                         throw parseError(addKeyName("List should have ended with ] or had a comma, instead had token: "

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

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

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

@@ -320,4 +320,52 @@ class ConcatenationTest extends TestUtils {
         val conf = parseConfig(""" a = ${x}, a += ${y}, x = [1], y = 2 """).resolve()
         assertEquals(Seq(1, 2), conf.getIntList("a").asScala.toList)
     }
+
+    @Test
+    def plusEqualsMultipleTimes() {
+        val conf = parseConfig(""" a += 1, a += 2, a += 3 """).resolve()
+        assertEquals(Seq(1, 2, 3), conf.getIntList("a").asScala.toList)
+    }
+
+    @Test
+    def plusEqualsMultipleTimesNested() {
+        val conf = parseConfig(""" x { a += 1, a += 2, a += 3 } """).resolve()
+        assertEquals(Seq(1, 2, 3), conf.getIntList("x.a").asScala.toList)
+    }
+
+    @Test
+    def plusEqualsAnObjectMultipleTimes() {
+        val conf = parseConfig(""" a += { b: 1 }, a += { b: 2 }, a += { b: 3 } """).resolve()
+        assertEquals(Seq(1, 2, 3), conf.getObjectList("a").asScala.toList.map(_.toConfig.getInt("b")))
+    }
+
+    @Test
+    def plusEqualsAnObjectMultipleTimesNested() {
+        val conf = parseConfig(""" x { a += { b: 1 }, a += { b: 2 }, a += { b: 3 } } """).resolve()
+        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()
+            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() {
+        System.err.println("==============")
+        val e = intercept[ConfigException.Parse] {
+            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"))
+        System.err.println("==============")
+    }
 }

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

@@ -827,6 +827,17 @@ 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"))