Address further PR feedback before merge

Implement an `equals()` method for AbstractConfigNodes. Add a
test to ensure an exception is thrown when calling setValue()
on a ConfigDocument when passing in a value with HOCON syntax
when the document was parsed as JSON. Add a setValue() method to
the ConfigDocument interface that takes a ConfigValue instead of
a String. When parsing a single value, throw an exception if a
concatenation is seen when parsing JSON.

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

@@ -25,12 +25,29 @@ public interface ConfigDocument {
      *
      * @param path the path at which to set the desired value
      * @param newValue the value to set at the desired path, represented as a string. This
-     *                 string will be parsed into a ConfigNode, and the text will be inserted
+     *                 string will be parsed into a ConfigNode using the same options used to
+     *                 parse the entire document, and the text will be inserted
      *                 as-is into the document, with leading and trailing whitespace removed.
+     *                 If a concatenation is passed in for newValue but the document was parsed
+     *                 with JSON, the first value in the concatenation will be parsed and inserted
+     *                 into the ConfigDocument.
      * @return a copy of the ConfigDocument with the desired value at the desired path
      */
     ConfigDocument setValue(String path, String newValue);
 
+    /**
+     * Returns a new ConfigDocument that is a copy of the current ConfigDocument,
+     * but with the desired value set at the desired path as with {@link #setValue(String, String)},
+     * but takes a ConfigValue instead of a string.
+     *
+     * @param path the path at which to set the desired value
+     * @param newValue the value to set at the desired path, represented as a ConfigValue.
+     *                 The rendered text of the ConfigValue will be inserted into the
+     *                 ConfigDocument.
+     * @return a copy of the ConfigDocument with the desired value at the desired path
+     */
+    ConfigDocument setValue(String path, ConfigValue newValue);
+
     /**
      * The original text of the input, modified if necessary with
      * any replaced or added values.

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

@@ -16,4 +16,9 @@ abstract class AbstractConfigNode implements ConfigNode {
         }
         return origText.toString();
     }
+
+    @Override
+    final public boolean equals(Object other) {
+        return other instanceof AbstractConfigNode && render().equals(((AbstractConfigNode)other).render());
+    }
 }

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

@@ -644,7 +644,16 @@ final class ConfigDocumentParser {
                 throw parseError("Empty value");
             }
             if (flavor == ConfigSyntax.JSON) {
-                return parseValue(t);
+                AbstractConfigNodeValue node = parseValue(t);
+                t = nextToken();
+                while (Tokens.isIgnoredWhitespace(t) || Tokens.isNewline(t) || isUnquotedWhitespace(t)) {
+                    t = nextToken();
+                }
+                if (t == Tokens.END) {
+                    return node;
+                } else {
+                    throw parseError("Tried to parse a concatenation. Concatenations not allowed in valid JSON");
+                }
             } else {
                 putBack(t);
                 ArrayList<AbstractConfigNode> nodes = new ArrayList();

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

@@ -33,7 +33,7 @@ final class ConfigNodeObject extends ConfigNodeComplexValue {
                 if (node.value() instanceof ConfigNodeObject) {
                     Path remainingPath = desiredPath.subPath(key.length());
                     childrenCopy.set(i, node.replaceValue(((ConfigNodeObject)node.value()).changeValueOnPath(remainingPath, valueCopy)));
-                    if (valueCopy != null && node.render() != super.children.get(i).render())
+                    if (valueCopy != null && !node.equals(super.children.get(i)))
                         valueCopy = null;
                 }
             }

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

@@ -30,6 +30,10 @@ final class SimpleConfigDocument implements ConfigDocument {
         return new SimpleConfigDocument(((ConfigNodeObject)configNodeTree).setValueOnPath(path, parsedValue, parseOptions.getSyntax()), parseOptions);
     }
 
+    public ConfigDocument setValue(String path, ConfigValue newValue) {
+        return setValue(path, newValue.render());
+    }
+
     public String render() {
         return configNodeTree.render();
     }

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

@@ -61,7 +61,6 @@ class ConfigDocumentParserTest extends TestUtils {
         assertTrue(exceptionThrown)
     }
 
-
     @Test
     def parseSuccess {
         parseTest("foo:bar")
@@ -250,9 +249,17 @@ class ConfigDocumentParserTest extends TestUtils {
         var node = ConfigDocumentParser.parseValue(tokenize(origText), ConfigParseOptions.defaults())
         assertEquals(origText, node.render())
 
-      // Check that concatenations in JSON will only return the first value passed in
+        // Check that concatenations in JSON will throw an error
         origText = "123 456 789"
+        var exceptionThrown = false
+        try {
             node = ConfigDocumentParser.parseValue(tokenize(origText), ConfigParseOptions.defaults().setSyntax(ConfigSyntax.JSON))
-      assertEquals("123", node.render())
+        } catch {
+            case e: Exception =>
+                exceptionThrown = true
+                assertTrue(e.isInstanceOf[ConfigException])
+                assertTrue(e.getMessage.contains("Tried to parse a concatenation. Concatenations not allowed in valid JSON"))
+        }
+        assertTrue(exceptionThrown)
     }
 }

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

@@ -4,7 +4,7 @@ import java.io.{BufferedReader, FileReader}
 import java.nio.charset.StandardCharsets
 import java.nio.file.{ Paths, Files }
 
-import com.typesafe.config.{ConfigException, ConfigSyntax, ConfigParseOptions, ConfigDocumentFactory}
+import com.typesafe.config._
 import org.junit.Assert._
 import org.junit.Test
 
@@ -72,7 +72,6 @@ class ConfigDocumentTest extends TestUtils {
         configDocumentReplaceConfTest(origText, finalText, """"i am now a string"""", "h.b.a")
         configDocumentReplaceJsonTest(origText, finalText, """"i am now a string"""", "h.b.a")
 
-
         // Can handle replacing values with maps
         finalText =
             """{
@@ -167,7 +166,20 @@ class ConfigDocumentTest extends TestUtils {
     }
 
     @Test
-    def configDocumentReplaceFailure {
+    def configDocumentSetNewConfigValue {
+        val origText = "{\"a\": \"b\"}"
+        val finalText = "{\"a\": 12}"
+        val configDocHOCON = ConfigDocumentFactory.parseString(origText)
+        val configDocJSON = ConfigDocumentFactory.parseString(origText, ConfigParseOptions.defaults.setSyntax(ConfigSyntax.JSON))
+        val newValue = ConfigValueFactory.fromAnyRef(12)
+        assertEquals(origText, configDocHOCON.render())
+        assertEquals(origText, configDocJSON.render())
+        assertEquals(finalText, configDocHOCON.setValue("a", newValue).render())
+        assertEquals(finalText, configDocJSON.setValue("a", newValue).render())
+    }
+
+    @Test
+    def configDocumentArrayReplaceFailure {
         // Attempting a replace on a ConfigDocument parsed from an array throws an error
         val origText = "[1, 2, 3, 4, 5]"
         val document = ConfigDocumentFactory.parseString(origText)
@@ -183,6 +195,42 @@ class ConfigDocumentTest extends TestUtils {
         assertTrue(exceptionThrown)
     }
 
+    @Test
+    def configDocumentJSONReplaceFailure {
+        // Attempting a replace on a ConfigDocument parsed from JSON with a value using HOCON syntax
+        // will fail
+        val origText = "{\"foo\": \"bar\", \"baz\": \"qux\"}"
+        val document = ConfigDocumentFactory.parseString(origText, ConfigParseOptions.defaults().setSyntax(ConfigSyntax.JSON))
+        var exceptionThrown = false
+        try {
+            document.setValue("foo", "unquoted")
+        } catch {
+            case e: Exception =>
+                exceptionThrown = true
+                assertTrue(e.isInstanceOf[ConfigException])
+                assertTrue(e.getMessage.contains("Token not allowed in valid JSON"))
+        }
+        assertTrue(exceptionThrown)
+    }
+
+  @Test
+    def configDocumentJSONReplaceWithConcatenationFailure {
+        // Attempting a replace on a ConfigDocument parsed from JSON with a concatenation will
+        // fail
+        val origText = "{\"foo\": \"bar\", \"baz\": \"qux\"}"
+        val document = ConfigDocumentFactory.parseString(origText, ConfigParseOptions.defaults().setSyntax(ConfigSyntax.JSON))
+        var exceptionThrown = false
+        try {
+            document.setValue("foo", "1 2 3 concatenation")
+        } catch {
+            case e: Exception =>
+                exceptionThrown = true
+                assertTrue(e.isInstanceOf[ConfigException])
+                assertTrue(e.getMessage.contains("Tried to parse a concatenation. Concatenations not allowed in valid JSON"))
+        }
+        assertTrue(exceptionThrown)
+    }
+
     @Test
     def configDocumentFileParse {
         val configDocument = ConfigDocumentFactory.parseFile(resourceFile("/test03.conf"))
@@ -205,4 +253,5 @@ class ConfigDocumentTest extends TestUtils {
         val configDocumentFile = ConfigDocumentFactory.parseFile(resourceFile("/test03.conf"))
         assertEquals(configDocumentFile.render(), configDocument.render())
     }
+
 }

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

@@ -168,7 +168,6 @@ class ConfigNodeTest extends TestUtils {
         topLevelValueReplaceTest(array, concatenation)
         topLevelValueReplaceTest(concatenation, array)
 
-
         //Ensure a key with format "a.b" will be properly replaced
         topLevelValueReplaceTest(nodeInt(10), nestedMap, "foo.bar")
     }