Repository: chimpler/pyhocon

Issue:
String substitution broken?
Could it be that the fix for object merging (issue #10) broken string substitution?
Expanding on the simple unit test from pyhocon codebase:
```
In [54]: config = ConfigFactory.parse_string(
   ....:             """
   ....:             {
   ....:                 a: {
   ....:                     b: {
   ....:                         c = buzz
   ....:                     }
   ....:                 }
   ....:                 d = test ${a.b.c} me
   ....:                 e = ${a.b.c} me
   ....:             }
   ....:             """
   ....:         )

In [55]: config['d']
Out[55]: 'test buzzme'

In [56]: config['e']
Out[56]: 'buzz'
```
Whereas I expected to get `test buzz me` (with the space between subst expr and the last string) and `buzz me` (here the appended string got lost entirely)

Let me know if I'm misunderstanding how string subst is supposed to work.


Discussion:

Patch:
diff --git a/pyhocon/__init__.py b/pyhocon/__init__.py
index 2a467e8..5b3b7a5 100644
--- a/pyhocon/__init__.py
+++ b/pyhocon/__init__.py
@@ -145,7 +145,7 @@ class ConfigParser(object):
 
         eol = Word('\n\r').suppress()
         eol_comma = Word('\n\r,').suppress()
-        comment = (Optional(eol_comma) + (Literal('#') | Literal('//')) - SkipTo(eol)).suppress()
+        comment = Suppress(Optional(eol_comma) + (Literal('#') | Literal('//')) - SkipTo(eol))
         number_expr = Regex('[+-]?(\d*\.\d+|\d+(\.\d+)?)([eE]\d+)?(?=[ \t]*([\$\}\],#\n\r]|//))', re.DOTALL).setParseAction(convert_number)
 
         # multi line string using """
@@ -162,9 +162,7 @@ class ConfigParser(object):
         substitution_expr = Regex('\$\{[^\}]+\}\s*').setParseAction(create_substitution)
         string_expr = multiline_string | quoted_string | unquoted_string
 
-        value_expr = number_expr | true_expr | false_expr | null_expr | string_expr | substitution_expr
-        values_expr = ConcatenatedValueParser(value_expr - ZeroOrMore(comment | (value_expr - Optional(Literal('\\') - eol).suppress()) | value_expr))
-        # multiline if \ at the end of the line
+        value_expr = number_expr | true_expr | false_expr | null_expr | string_expr
 
         include_expr = (Keyword("include", caseless=True).suppress() - (
             quoted_string | ((Keyword('url') | Keyword('file')) - Literal('(').suppress() - quoted_string - Literal(')').suppress())))\
@@ -174,11 +172,13 @@ class ConfigParser(object):
         # last zeroOrMore is because we can have t = {a:4} {b: 6} {c: 7} which is dictionary concatenation
         inside_dict_expr = ConfigTreeParser(ZeroOrMore(comment | include_expr | assign_expr | eol_comma))
         dict_expr = Suppress('{') - inside_dict_expr - Suppress('}')
-        list_expr = Suppress('[') - ListParser(ZeroOrMore(comment | dict_expr | values_expr | eol_comma)) - Suppress(']')
+        list_expr = Suppress('[') - ListParser(ZeroOrMore(comment | dict_expr | value_expr | eol_comma)) - Suppress(']')
 
         # special case when we have a value assignment where the string can potentially be the remainder of the line
-        assign_expr << Group(key - Suppress(Optional(Literal('=') | Literal(':'))) +
-                             (ConcatenatedValueParser(OneOrMore(substitution_expr | list_expr | dict_expr)) | comment | values_expr | eol_comma))
+        assign_expr << Group(key - Suppress(Optional(Literal('=') | Literal(':'))) -
+                             (ConcatenatedValueParser(
+                                 ZeroOrMore(substitution_expr | list_expr | dict_expr | comment | value_expr | (Literal('\\') - eol).suppress())
+                             )))
 
         # the file can be { ... } where {} can be omitted or []
         config_expr = ZeroOrMore(comment | eol) \
@@ -219,8 +219,11 @@ class ConfigParser(object):
                     else:
                         # replace token by substitution
                         config_values = substitution.parent
-                        # if there is more than one element in the config values then it's a string
-                        config_values.put(substitution.index, resolved_value)
+                        # if it is a string, then add the extra ws that was present in the original string after the substitution
+                        formatted_resolved_value = \
+                            resolved_value + substitution.ws if isinstance(resolved_value, str) \
+                            and substitution.index < len(config_values.tokens) - 1 else resolved_value
+                        config_values.put(substitution.index, formatted_resolved_value)
                         transformation = config_values.transform()
                         result = transformation[0] if isinstance(transformation, list) else transformation
                         config_values.parent[config_values.key] = result
diff --git a/pyhocon/config_tree.py b/pyhocon/config_tree.py
index 42a52ec..f4f5e67 100644
--- a/pyhocon/config_tree.py
+++ b/pyhocon/config_tree.py
@@ -203,12 +203,18 @@ class ConfigValues(object):
         self.tokens = iterable
         self.parent = None
         self.key = None
+
         for index, token in enumerate(self.tokens):
             if isinstance(token, ConfigSubstitution):
                 token.parent = self
                 token.index = index
 
         # if the last token is an unquoted string then right strip it
+
+        # no value return empty string
+        if len(self.tokens) == 0:
+            self.tokens = ['']
+
         if isinstance(self.tokens[-1], ConfigUnquotedString):
             self.tokens[-1] = self.tokens[-1].rstrip()
 


Test patch:
diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py
index be865ba..492816d 100644
--- a/tests/test_config_parser.py
+++ b/tests/test_config_parser.py
@@ -1,6 +1,6 @@
 import pytest
-from pyhocon import ConfigFactory
-from pyhocon.exceptions import ConfigMissingException
+from pyhocon import ConfigFactory, ConfigSubstitutionException
+from pyhocon.exceptions import ConfigMissingException, ConfigWrongTypeException
 
 
 class TestConfigParser(object):
@@ -213,8 +213,98 @@ class TestConfigParser(object):
         assert config.get('a') == [1, 2, 3, 4, 5, 6]
         assert config.get_list('a') == [1, 2, 3, 4, 5, 6]
 
-    def test_substitutions(self):
-        config = ConfigFactory.parse_string(
+    def test_string_substitutions(self):
+        config1 = ConfigFactory.parse_string(
+            """
+            {
+                a: {
+                    b: {
+                        c = str
+                        e = "str      "
+                    }
+                }
+                d = ${a.b.c}
+                f = ${a.b.e}
+            }
+            """
+        )
+
+        assert config1.get('a.b.c') == 'str'
+        assert config1.get('d') == 'str'
+        assert config1.get('f') == 'str      '
+
+        config2 = ConfigFactory.parse_string(
+            """
+            {
+                a: {
+                    b: {
+                        c = str
+                        e = "str      "
+                    }
+                }
+                d = test  ${a.b.c}
+                f = test  ${a.b.e}
+            }
+            """
+        )
+
+        assert config2.get('a.b.c') == 'str'
+        assert config2.get('d') == 'test  str'
+        assert config2.get('f') == 'test  str      '
+
+        config3 = ConfigFactory.parse_string(
+            """
+            {
+                a: {
+                    b: {
+                        c = str
+                        e = "str      "
+                    }
+                }
+                d = test  ${a.b.c}  me
+                f = test  ${a.b.e}  me
+            }
+            """
+        )
+
+        assert config3.get('a.b.c') == 'str'
+        assert config3.get('d') == 'test  str  me'
+        assert config3.get('f') == 'test  str        me'
+
+    def test_int_substitutions(self):
+        config1 = ConfigFactory.parse_string(
+            """
+            {
+                a: {
+                    b: {
+                        c = 5
+                    }
+                }
+                d = ${a.b.c}
+            }
+            """
+        )
+
+        assert config1.get('a.b.c') == 5
+        assert config1.get('d') == 5
+
+        config2 = ConfigFactory.parse_string(
+            """
+            {
+                a: {
+                    b: {
+                        c = 5
+                    }
+                }
+                d = test ${a.b.c}
+            }
+            """
+        )
+
+        assert config2.get('a.b.c') == 5
+        assert config2.get('d') == 'test 5'
+
+        config3 = ConfigFactory.parse_string(
             """
             {
                 a: {
@@ -227,10 +317,10 @@ class TestConfigParser(object):
             """
         )
 
-        assert config.get('a.b.c') == 5
-        assert config.get('d') == 'test 5 me'
+        assert config3.get('a.b.c') == 5
+        assert config3.get('d') == 'test 5 me'
 
-    def test_cascade_substitutions(self):
+    def test_cascade_string_substitutions(self):
         config = ConfigFactory.parse_string(
             """
             {
@@ -334,6 +424,117 @@ class TestConfigParser(object):
         assert config4.get('host_modules') == ['java', 'php', 'python', 'perl']
         assert config4.get('full_modules') == ['java', 'php', 'python', 'perl', 'c', 'go']
 
+    def test_non_existent_substitution(self):
+        with pytest.raises(ConfigSubstitutionException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = ${non_existent}
+                """
+            )
+
+        with pytest.raises(ConfigSubstitutionException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = abc ${non_existent}
+                """
+            )
+
+        with pytest.raises(ConfigSubstitutionException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = ${non_existent} abc
+                """
+            )
+
+        with pytest.raises(ConfigSubstitutionException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = abc ${non_existent} def
+                """
+            )
+
+    def test_non_compatible_substitution(self):
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = 55 ${common_modules}
+                """
+            )
+
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = ${common_modules} 55
+                """
+            )
+
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = aa ${common_modules} bb
+                """
+            )
+
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = aa ${common_modules}
+                """
+            )
+
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = ${common_modules} aa
+                """
+            )
+
+        with pytest.raises(ConfigWrongTypeException):
+            ConfigFactory.parse_string(
+                """
+                    common_modules = [perl]
+                    host_modules = aa ${common_modules} bb
+                """
+            )
+
+    def test_concat_multi_line_string(self):
+        config = ConfigFactory.parse_string(
+            """
+                common_modules = perl \
+                java \
+                python
+            """
+        )
+
+        assert [x.strip() for x in config['common_modules'].split() if x.strip(' ') != ''] == ['perl', 'java', 'python']
+
+    def test_concat_multi_line_list(self):
+        config = ConfigFactory.parse_string(
+            """
+                common_modules = [perl] \
+                [java] \
+                [python]
+            """
+        )
+
+        assert config['common_modules'] == ['perl', 'java', 'python']
+
+    def test_concat_multi_line_dict(self):
+        config = ConfigFactory.parse_string(
+            """
+                common_modules = {a:perl} \
+                {b:java} \
+                {c:python}
+            """
+        )
+
+        assert config['common_modules'] == {'a': 'perl', 'b': 'java', 'c': 'python'}
+
     def test_include_dict(self):
         config = ConfigFactory.parse_file("samples/animals.conf")
         assert config.get('cat.garfield.say') == 'meow'