Merge pull request #268 from typesafehub/whitespace-in-concat

Allow unquoted whitespace between substitutions that expand to objects/l...

HOCON.md

@@ -383,6 +383,15 @@ A common use of array concatenation is to add to paths:
     path = [ /bin ]
     path = ${path} [ /usr/bin ]
 
+#### Note: Concatenation with whitespace and substitutions
+
+When concatenating substitutions such as `${foo} ${bar}`, the
+substitutions may turn out to be strings (which makes the
+whitespace between them significant) or may turn out to be objects
+or lists (which makes it irrelevant). Unquoted whitespace must be
+ignored in between substitutions which resolve to objects or
+lists. Quoted whitespace should be an error.
+
 #### Note: Arrays without commas or newlines
 
 Arrays allow you to use newlines instead of commas, but not

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

@@ -81,6 +81,10 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
         return Collections.singleton(this);
     }
 
+    private static boolean isIgnoredWhitespace(AbstractConfigValue value) {
+        return (value instanceof ConfigString) && !((ConfigString)value).wasQuoted();
+    }
+
     /**
      * Add left and right, or their merger, to builder.
      */
@@ -104,6 +108,10 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
             joined = right.withFallback(left);
         } else if (left instanceof SimpleConfigList && right instanceof SimpleConfigList) {
             joined = ((SimpleConfigList)left).concatenate((SimpleConfigList)right);
+        } else if ((left instanceof SimpleConfigList || left instanceof ConfigObject) &&
+                   isIgnoredWhitespace(right)) {
+            joined = left;
+            // it should be impossible that left is whitespace and right is a list or object
         } else if (left instanceof ConfigConcatenation || right instanceof ConfigConcatenation) {
             throw new ConfigException.BugOrBroken("unflattened ConfigConcatenation");
         } else if (left instanceof Unmergeable || right instanceof Unmergeable) {
@@ -119,7 +127,7 @@ final class ConfigConcatenation extends AbstractConfigValue implements Unmergeab
             } else {
                 ConfigOrigin joinedOrigin = SimpleConfigOrigin.mergeOrigins(left.origin(),
                         right.origin());
-                joined = new ConfigString(joinedOrigin, s1 + s2);
+                joined = new ConfigString.Quoted(joinedOrigin, s1 + s2);
             }
         }
 

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

@@ -227,7 +227,7 @@ public class ConfigImpl {
                 return defaultFalseValue;
             }
         } else if (object instanceof String) {
-            return new ConfigString(origin, (String) object);
+            return new ConfigString.Quoted(origin, (String) object);
         } else if (object instanceof Number) {
             // here we always keep the same type that was passed to us,
             // rather than figuring out if a Long would fit in an Int
@@ -346,7 +346,7 @@ public class ConfigImpl {
         for (Map.Entry<String, String> entry : env.entrySet()) {
             String key = entry.getKey();
             m.put(key,
-                    new ConfigString(SimpleConfigOrigin.newSimple("env var " + key), entry
+                    new ConfigString.Quoted(SimpleConfigOrigin.newSimple("env var " + key), entry
                             .getValue()));
         }
         return new SimpleConfigObject(SimpleConfigOrigin.newSimple("env variables"),

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

@@ -10,17 +10,59 @@ import com.typesafe.config.ConfigOrigin;
 import com.typesafe.config.ConfigRenderOptions;
 import com.typesafe.config.ConfigValueType;
 
-final class ConfigString extends AbstractConfigValue implements Serializable {
+abstract class ConfigString extends AbstractConfigValue implements Serializable {
 
     private static final long serialVersionUID = 2L;
 
-    final private String value;
+    final protected String value;
 
-    ConfigString(ConfigOrigin origin, String value) {
+    protected ConfigString(ConfigOrigin origin, String value) {
         super(origin);
         this.value = value;
     }
 
+
+    final static class Quoted extends ConfigString {
+        Quoted(ConfigOrigin origin, String value) {
+            super(origin, value);
+        }
+        @Override
+        protected Quoted newCopy(ConfigOrigin origin) {
+            return new Quoted(origin, value);
+        }
+        // serialization all goes through SerializedConfigValue
+        private Object writeReplace() throws ObjectStreamException {
+            return new SerializedConfigValue(this);
+        }
+    }
+
+    // this is sort of a hack; we want to preserve whether whitespace
+    // was quoted until we process substitutions, so we can ignore
+    // unquoted whitespace when concatenating lists or objects.
+    // We dump this distinction when serializing and deserializing,
+    // but that's OK because it isn't in equals/hashCode, and we
+    // don't allow serializing unresolved objects which is where
+    // quoted-ness matters. If we later make ConfigOrigin point
+    // to the original token range, we could use that to implement
+    // wasQuoted()
+    final static class Unquoted extends ConfigString {
+        Unquoted(ConfigOrigin origin, String value) {
+            super(origin, value);
+        }
+        @Override
+        protected Unquoted newCopy(ConfigOrigin origin) {
+            return new Unquoted(origin, value);
+        }
+        // serialization all goes through SerializedConfigValue
+        private Object writeReplace() throws ObjectStreamException {
+            return new SerializedConfigValue(this);
+        }
+    }
+
+    boolean wasQuoted() {
+        return (this instanceof Quoted);
+    }
+
     @Override
     public ConfigValueType valueType() {
         return ConfigValueType.STRING;
@@ -45,14 +87,4 @@ final class ConfigString extends AbstractConfigValue implements Serializable {
             rendered = ConfigImplUtil.renderStringUnquotedIfPossible(value);
         sb.append(rendered);
     }
-
-    @Override
-    protected ConfigString newCopy(ConfigOrigin origin) {
-        return new ConfigString(origin, value);
-    }
-
-    // serialization all goes through SerializedConfigValue
-    private Object writeReplace() throws ObjectStreamException {
-        return new SerializedConfigValue(this);
-    }
 }

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

@@ -64,7 +64,7 @@ final class DefaultTransformer {
             switch (value.valueType()) {
             case NUMBER: // FALL THROUGH
             case BOOLEAN:
-                return new ConfigString(value.origin(),
+                return new ConfigString.Quoted(value.origin(),
                         value.transformToString());
             case NULL:
                 // want to be sure this throws instead of returning "null" as a

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

@@ -513,7 +513,7 @@ final class Parser {
                 // or substitution already.
                 v = Tokens.getValue(t.token);
             } else if (Tokens.isUnquotedText(t.token)) {
-                v = new ConfigString(t.token.origin(), Tokens.getUnquotedText(t.token));
+                v = new ConfigString.Unquoted(t.token.origin(), Tokens.getUnquotedText(t.token));
             } else if (Tokens.isSubstitution(t.token)) {
                 v = new ConfigReference(t.token.origin(), tokenToSubstitutionExpression(t.token));
             } else if (t.token == Tokens.OPEN_CURLY) {

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

@@ -143,7 +143,7 @@ final class PropertiesParser {
             AbstractConfigValue value;
             if (convertedFromProperties) {
                 if (rawValue instanceof String) {
-                    value = new ConfigString(origin, (String) rawValue);
+                    value = new ConfigString.Quoted(origin, (String) rawValue);
                 } else {
                     // silently ignore non-string values in Properties
                     value = null;

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

@@ -353,7 +353,7 @@ class SerializedConfigValue extends AbstractConfigValue implements Externalizabl
             String sd = in.readUTF();
             return new ConfigDouble(origin, vd, sd);
         case STRING:
-            return new ConfigString(origin, in.readUTF());
+            return new ConfigString.Quoted(origin, in.readUTF());
         case LIST:
             int listSize = in.readInt();
             List<AbstractConfigValue> list = new ArrayList<AbstractConfigValue>(listSize);

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

@@ -403,7 +403,7 @@ final class Tokens {
     }
 
     static Token newString(ConfigOrigin origin, String value) {
-        return newValue(new ConfigString(origin, value));
+        return newValue(new ConfigString.Quoted(origin, value));
     }
 
     static Token newInt(ConfigOrigin origin, int value, String originalText) {

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

@@ -454,9 +454,53 @@ class ConcatenationTest extends TestUtils {
         assertEquals(" ", conf.getString("a"))
     }
 
+    @Test
+    def concatTwoDefinedSubstitutionsWithASpace() {
+        val conf = parseConfig("""foo=abc, bar=def, a = ${foo} ${bar}""").resolve()
+        assertEquals("abc def", conf.getString("a"))
+    }
+
     @Test
     def concatTwoUndefinedSubstitutionsWithEmptyString() {
         val conf = parseConfig("""a = ""${?foo}${?bar}""").resolve()
         assertEquals("", conf.getString("a"))
     }
+
+    @Test
+    def concatSubstitutionsThatAreObjectsWithNoSpace() {
+        val conf = parseConfig("""foo = { a : 1}, bar = { b : 2 }, x = ${foo}${bar}""").resolve()
+        assertEquals(1, conf.getInt("x.a"))
+        assertEquals(2, conf.getInt("x.b"))
+    }
+
+    // whitespace is insignificant if substitutions don't turn out to be a string
+    @Test
+    def concatSubstitutionsThatAreObjectsWithSpace() {
+        val conf = parseConfig("""foo = { a : 1}, bar = { b : 2 }, x = ${foo} ${bar}""").resolve()
+        assertEquals(1, conf.getInt("x.a"))
+        assertEquals(2, conf.getInt("x.b"))
+    }
+
+    // whitespace is insignificant if substitutions don't turn out to be a string
+    @Test
+    def concatSubstitutionsThatAreListsWithSpace() {
+        val conf = parseConfig("""foo = [1], bar = [2], x = ${foo} ${bar}""").resolve()
+        assertEquals(List(1, 2), conf.getIntList("x").asScala)
+    }
+
+    // but quoted whitespace should be an error
+    @Test
+    def concatSubstitutionsThatAreObjectsWithQuotedSpace() {
+        val e = intercept[ConfigException.WrongType] {
+            parseConfig("""foo = { a : 1}, bar = { b : 2 }, x = ${foo}"  "${bar}""").resolve()
+        }
+    }
+
+    // but quoted whitespace should be an error
+    @Test
+    def concatSubstitutionsThatAreListsWithQuotedSpace() {
+        val e = intercept[ConfigException.WrongType] {
+            parseConfig("""foo = [1], bar = [2], x = ${foo}"  "${bar}""").resolve()
+        }
+    }
 }

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

@@ -70,7 +70,7 @@ class JsonTest extends TestUtils {
             case lift.JDouble(d) =>
                 doubleValue(d)
             case lift.JString(s) =>
-                new ConfigString(fakeOrigin(), s)
+                new ConfigString.Quoted(fakeOrigin(), s)
             case lift.JNull =>
                 new ConfigNull(fakeOrigin())
             case lift.JNothing =>

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

@@ -573,7 +573,7 @@ abstract trait TestUtils {
     protected def longValue(l: Long) = new ConfigLong(fakeOrigin(), l, null)
     protected def boolValue(b: Boolean) = new ConfigBoolean(fakeOrigin(), b)
     protected def nullValue() = new ConfigNull(fakeOrigin())
-    protected def stringValue(s: String) = new ConfigString(fakeOrigin(), s)
+    protected def stringValue(s: String) = new ConfigString.Quoted(fakeOrigin(), s)
     protected def doubleValue(d: Double) = new ConfigDouble(fakeOrigin(), d, null)
 
     protected def parseObject(s: String) = {

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

@@ -136,7 +136,7 @@ class TokenizerTest extends TestUtils {
     @Test
     def tokenizerUnescapeStrings(): Unit = {
         case class UnescapeTest(escaped: String, result: ConfigString)
-        implicit def pair2unescapetest(pair: (String, String)): UnescapeTest = UnescapeTest(pair._1, new ConfigString(fakeOrigin(), pair._2))
+        implicit def pair2unescapetest(pair: (String, String)): UnescapeTest = UnescapeTest(pair._1, new ConfigString.Quoted(fakeOrigin(), pair._2))
 
         // getting the actual 6 chars we want in a string is a little pesky.
         // \u005C is backslash. Just prove we're doing it right here.