Substitution with merged configs is incorrect

[macster84]

Hi dear maintainers,

with pyhocon 0.3.35 I get unexpected substitution results with merged configs.

I have following configurations:

base.conf

base {
  baseattr1: 1
  baseattr2: 1
}

derived = ${base} {
  derivedattr: 1
  lost: 1
}

test.conf

include "base.conf"

base {
  baseattr1: 2
  baseattr3: 1
}

derived = ${base} {
  derivedattr1: 2
  derivedattr2: 1
}

test.py:

#!/usr/bin/env python
from pyhocon import ConfigFactory
from pyhocon.tool import HOCONConverter

conf = ConfigFactory.parse_file('test.conf')
print(HOCONConverter.to_json(conf))

I executed the test.py with the configs being in the same directory.
Expected result:

{
  "base": {
    "baseattr1": 2,
    "baseattr2": 1,
    "baseattr3": 1
  },
  "derived": {
    "baseattr1": 2,
    "baseattr2": 1,
    "baseattr3": 1,
    "derivedattr1": 2,
    "lost": 1,
    "derivedattr2": 1
  }
}

Actual result:

{
  "base": {
    "baseattr1": 2,
    "baseattr2": 1,
    "baseattr3": 1
  },
  "derived": {
    "baseattr1": 2,
    "baseattr2": 1,
    "baseattr3": 1,
    "derivedattr1": 2,
    "derivedattr2": 1
  }
}

Notice that the key 'lost' is missing.

In fact, if I adapt the test.conf like this; the output is like expected.

include "base.conf"

base {
  baseattr1: 2
  baseattr3: 1
}

derived {
  derivedattr1: 1
}

Notice that the only change was to remove the object merge for key 'derived'.
I did not dig into the code yet, but I think it is related to merging and not the substitution. If I set 'resolve=False' in test.py, the output looks like this:

{
  "base": {
    "baseattr1": 2,
    "baseattr2": 1,
    "baseattr3": 1
  },
  "derived": [ConfigValues: [ConfigSubstitution: base],ConfigTree([(u'derivedattr1', 2), (u'derivedattr2', 1)])]
}

The key 'lost' is already missing.

I tested this stuff with the JVM implementation com.typesafe.config v1.3.0 which yields the expected results.

Do you have any insights on that?
Many thanks in advance

Greets,
Alexander

[hekaldama]

Hows this coming?

[aalba6675]

Minimal reproducer. It seems to be an issue with resolve_substitutions() If derived = ${base}... appears twice the first one is lost.

>>> test = ConfigFactory.parse_string('''
... base {}
... 
... derived =  ${base} {
...   lost: 1
... }
... 
... derived = ${base} {
...   found: 2
... }
... ''')
>>> 
>>> test['derived']
ConfigTree([('found', 2)])

Expected answer: ConfigTree([('found', 2), ('lost', 1)])

[aalba6675]

I think what is happening: if a key is ConfigValues, when it is resolved to a ConfigTree we do not walk down the linked list of overriden_value and perform resolution and merging if the overriden values are also ConfigTrees

derived = ${base}
derived = ${base2}

base  {
 a: 1
 b: 2
}

base2 {
 b:3
 c:4
}

The overriden list ConfigTree([('a', 1), ('b', 2)]), ConfigTree([('b', 3), ('c', 4)]) but we do check that the antecedents are ConfigTrees and merge from the bottom up

[aalba6675]

RFC:
Get substitutions from all overriden_values as well

@@ -423,7 +424,17 @@ class ConfigValues(object):
         return len(self.get_substitutions()) > 0
 
     def get_substitutions(self):
-        return [token for token in self.tokens if isinstance(token, ConfigSubstitution)]
+        lst = []
+        node = self
+        while node:
+            lst = [token for token in node.tokens if isinstance(token, ConfigSubstitution)] + lst
+            if hasattr(node, 'overriden_value'):
+                node = node.overriden_value
+                if not isinstance(node, ConfigValues):
+                    break
+            else:
+                break
+        return lst

If we are a ConfigTree, merge from the bottom up

@@ -459,7 +470,28 @@ class ConfigValues(object):
                         col=col(self._loc, self._instring)))
 
         if first_tok_type is ConfigTree:
+            child = []
+            if hasattr(self, 'overriden_value'):
+                node = self.overriden_value
+                while node:
+                    if isinstance(node, ConfigValues):
+                        value = node.transform()
+                        if isinstance(value, ConfigTree):
+                            child.append(value)
+                        else:
+                            break
+                    elif isinstance(node, ConfigTree):
+                        child.append(node)
+                    else:
+                        break
+                    if hasattr(node, 'overriden_value'):
+                        node = node.overriden_value
+                    else:
+                        break
+
             result = ConfigTree()
+            for conf in reversed(child):
+                ConfigTree.merge_configs(result, conf, copy_trees=True)
             for token in tokens:
                 ConfigTree.merge_configs(result, token, copy_trees=True)
             return result

[aalba6675]

PR #144