Fix broken overrides

pyhocon/config_parser.py

@@ -27,6 +27,7 @@ def fixed_get_attr(self, item):
         except KeyError:
             return ""
 
+
     pyparsing.ParseResults.__getattr__ = fixed_get_attr
 
 from pyhocon.config_tree import (ConfigInclude, ConfigList, ConfigQuotedString,
@@ -35,7 +36,6 @@ def fixed_get_attr(self, item):
 from pyhocon.exceptions import (ConfigException, ConfigMissingException,
                                 ConfigSubstitutionException)
 
-
 use_urllib2 = False
 try:
     # For Python 3.0 and later
@@ -62,12 +62,12 @@ def glob(pathname, recursive=False):
 else:
     from glob import glob
 
-
 # Fix deprecated warning with 'imp' library and Python 3.4+.
 # See: https://github.com/chimpler/pyhocon/issues/248
 if sys.version_info >= (3, 4):
     import importlib.util
 
+
     def find_package_dirs(name):
         spec = importlib.util.find_spec(name)
         # When `imp.find_module()` cannot find a package it raises ImportError.
@@ -80,12 +80,13 @@ def find_package_dirs(name):
     import imp
     import importlib
 
+
     def find_package_dirs(name):
         return [imp.find_module(name)[1]]
 
-
 logger = logging.getLogger(__name__)
 
+
 #
 # Substitution Defaults
 #
@@ -461,7 +462,8 @@ def set_default_white_spaces():
             # Allow only spaces as a valid separator between value and unit.
             # E.g. \t as a separator is invalid: '10<TAB>weeks'.
             period_expr = (
-                Word(nums)('value') + ZeroOrMore(White(ws=' ')).suppress() + Or(period_types)('unit') + WordEnd(alphanums).suppress()
+                    Word(nums)('value') + ZeroOrMore(White(ws=' ')).suppress() + Or(period_types)('unit') + WordEnd(
+                alphanums).suppress()
             ).setParseAction(convert_period)
 
             # multi line string using """
@@ -474,53 +476,61 @@ def set_default_white_spaces():
             # line1  \
             # line2 \
             # so a backslash precedes the \n
-            unquoted_string = Regex(r'(?:[^^`+?!@*&"\[\{\s\]\}#,=\$\\]|\\.)+[ \t]*', re.UNICODE).setParseAction(unescape_string)
+            unquoted_string = Regex(r'(?:[^^`+?!@*&"\[\{\s\]\}#,=\$\\]|\\.)+[ \t]*', re.UNICODE).setParseAction(
+                unescape_string)
             substitution_expr = Regex(r'[ \t]*\$\{[^\}]+\}[ \t]*').setParseAction(create_substitution)
             string_expr = multiline_string | quoted_string | unquoted_string
 
             value_expr = period_expr | number_expr | true_expr | false_expr | null_expr | string_expr
 
             include_content = (
-                quoted_string | ((Keyword('url') | Keyword('file') | Keyword('package')) - Literal('(').suppress() - quoted_string - Literal(')').suppress())
+                    quoted_string | ((Keyword('url') | Keyword('file') | Keyword('package')) - Literal(
+                '(').suppress() - quoted_string - Literal(')').suppress())
             )
             include_expr = (
-                Keyword("include", caseless=True).suppress() + (
+                    Keyword("include", caseless=True).suppress() + (
                     include_content | (
-                        Keyword("required") - Literal('(').suppress() - include_content - Literal(')').suppress()
-                    )
-                )
+                    Keyword("required") - Literal('(').suppress() - include_content - Literal(')').suppress()
+            )
+            )
             ).setParseAction(include_config)
 
             root_dict_expr = Forward()
             dict_expr = Forward()
             list_expr = Forward()
-            multi_value_expr = ZeroOrMore(comment_eol | include_expr | substitution_expr | dict_expr | list_expr | value_expr | (Literal(
-                '\\') - eol).suppress())
+            multi_value_expr = ZeroOrMore(
+                comment_eol | include_expr | substitution_expr | dict_expr | list_expr | value_expr | (Literal(
+                    '\\') - eol).suppress())
             # for a dictionary : or = is optional
             # last zeroOrMore is because we can have t = {a:4} {b: 6} {c: 7} which is dictionary concatenation
             inside_dict_expr = ConfigTreeParser(ZeroOrMore(comment_eol | include_expr | assign_expr | eol_comma))
-            inside_root_dict_expr = ConfigTreeParser(ZeroOrMore(comment_eol | include_expr | assign_expr | eol_comma), root=True)
+            inside_root_dict_expr = ConfigTreeParser(ZeroOrMore(comment_eol | include_expr | assign_expr | eol_comma),
+                                                     root=True)
             dict_expr << Suppress('{') - inside_dict_expr - Suppress('}')
             root_dict_expr << Suppress('{') - inside_root_dict_expr - Suppress('}')
             list_entry = ConcatenatedValueParser(multi_value_expr)
             list_expr << Suppress('[') - ListParser(list_entry - ZeroOrMore(eol_comma - list_entry)) - Suppress(']')
 
             # special case when we have a value assignment where the string can potentially be the remainder of the line
             assign_expr << Group(
-                key - ZeroOrMore(comment_no_comma_eol) - (dict_expr | (Literal('=') | Literal(':') | Literal('+=')) - ZeroOrMore(
+                key - ZeroOrMore(comment_no_comma_eol) - (
+                        dict_expr | (Literal('=') | Literal(':') | Literal('+=')) - ZeroOrMore(
                     comment_no_comma_eol) - ConcatenatedValueParser(multi_value_expr))
             )
 
             # the file can be { ... } where {} can be omitted or []
-            config_expr = ZeroOrMore(comment_eol | eol) + (list_expr | root_dict_expr | inside_root_dict_expr) + ZeroOrMore(
+            config_expr = ZeroOrMore(comment_eol | eol) + (
+                    list_expr | root_dict_expr | inside_root_dict_expr) + ZeroOrMore(
                 comment_eol | eol_comma)
             config = config_expr.parseString(content, parseAll=True)[0]
 
             if resolve:
-                allow_unresolved = resolve and unresolved_value is not DEFAULT_SUBSTITUTION and unresolved_value is not MANDATORY_SUBSTITUTION
+                allow_unresolved = resolve and unresolved_value is not DEFAULT_SUBSTITUTION \
+                                   and unresolved_value is not MANDATORY_SUBSTITUTION
                 has_unresolved = cls.resolve_substitutions(config, allow_unresolved)
                 if has_unresolved and unresolved_value is MANDATORY_SUBSTITUTION:
-                    raise ConfigSubstitutionException('resolve cannot be set to True and unresolved_value to MANDATORY_SUBSTITUTION')
+                    raise ConfigSubstitutionException(
+                        'resolve cannot be set to True and unresolved_value to MANDATORY_SUBSTITUTION')
 
             if unresolved_value is not NO_SUBSTITUTION and unresolved_value is not DEFAULT_SUBSTITUTION:
                 cls.unresolve_substitutions_to_value(config, unresolved_value)
@@ -571,14 +581,16 @@ def _fixup_self_references(cls, config, accept_unresolved=False):
                         if len(prop_path) > 1 and config.get(substitution.variable, None) is not None:
                             continue  # If value is present in latest version, don't do anything
                         if prop_path[0] == key:
-                            if isinstance(previous_item, ConfigValues) and not accept_unresolved:  # We hit a dead end, we cannot evaluate
+                            if isinstance(previous_item, ConfigValues) and not accept_unresolved:
+                                # We hit a dead end, we cannot evaluate
                                 raise ConfigSubstitutionException(
                                     "Property {variable} cannot be substituted. Check for cycles.".format(
                                         variable=substitution.variable
                                     )
                                 )
                             else:
-                                value = previous_item if len(prop_path) == 1 else previous_item.get(".".join(prop_path[1:]))
+                                value = previous_item if len(prop_path) == 1 else previous_item.get(
+                                    ".".join(prop_path[1:]))
                                 _, _, current_item = cls._do_substitute(substitution, value)
                     previous_item = current_item
 
@@ -632,8 +644,8 @@ def _do_substitute(cls, substitution, resolved_value, is_optional_resolved=True)
             # 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 \
                 if resolved_value is None \
-                or isinstance(resolved_value, (dict, list)) \
-                or substitution.index == len(config_values.tokens) - 1 \
+                   or isinstance(resolved_value, (dict, list)) \
+                   or substitution.index == len(config_values.tokens) - 1 \
                 else (str(resolved_value) + substitution.ws)
             # use a deepcopy of resolved_value to avoid mutation
             config_values.put(substitution.index, copy.deepcopy(formatted_resolved_value))
@@ -652,7 +664,7 @@ def _do_substitute(cls, substitution, resolved_value, is_optional_resolved=True)
                     new_substitutions = s
                     unresolved = True
 
-        return (unresolved, new_substitutions, result)
+        return unresolved, new_substitutions, result
 
     @classmethod
     def _final_fixup(cls, item):
@@ -684,55 +696,70 @@ def resolve_substitutions(cls, config, accept_unresolved=False):
         cls._fixup_self_references(config, accept_unresolved)
         substitutions = cls._find_substitutions(config)
         if len(substitutions) > 0:
-            unresolved = True
             any_unresolved = True
             _substitutions = []
             cache = {}
             while any_unresolved and len(substitutions) > 0 and set(substitutions) != set(_substitutions):
-                unresolved = False
-                any_unresolved = True
+                any_unresolved = False
                 _substitutions = substitutions[:]
 
                 for substitution in _substitutions:
+                    unresolved = False
+                    overridden_value = substitution.parent.overriden_value
+                    if isinstance(overridden_value, ConfigValues):
+                        overridden_value = overridden_value.transform()
                     # If this substitution is an override, and the parent is still being processed,
                     # skip this entry, it will be processed on the next loop.
-                    if substitution.parent.overriden_value:
-                        if substitution.parent.overriden_value in [s.parent for s in substitutions]:
-                            continue
+                    if overridden_value in [s.parent for s in substitutions]:
+                        continue
 
                     is_optional_resolved, resolved_value = cls._resolve_variable(config, substitution)
+                    if isinstance(resolved_value, ConfigValues) and overridden_value and not isinstance(
+                            overridden_value, ConfigValues):
+                        unresolved, _, _ = cls._do_substitute(substitution, overridden_value, is_optional_resolved)
+                        any_unresolved = unresolved or any_unresolved
+                        if not unresolved and substitution in substitutions:
+                            substitutions.remove(substitution)
+                        continue
+
+                    if isinstance(resolved_value, ConfigValues) and isinstance(overridden_value, ConfigValues):
+                        any_unresolved = True
+                        continue
+
+                    cache_values = []
+                    if isinstance(overridden_value, ConfigValues):
+                        cache_values = cache.get(substitution)
+                        if cache_values is None:
+                            continue
+                    if not isinstance(resolved_value, ConfigValues):
+                        cache_values.append(substitution)
+                        overrides = [s for s in substitutions if s.parent.overriden_value == substitution.parent]
+                        if len(overrides) > 0:
+                            for o in overrides:
+                                values = cache.get(o) if cache.get(o) is not None else []
+                                values.extend(cache_values)
+                                cache[o] = values
+                            substitutions.remove(substitution)
+                            continue
 
-                    # if the substitution is optional
-                    if not is_optional_resolved and substitution.optional:
-                        resolved_value = None
-                    if isinstance(resolved_value, ConfigValues):
-                        parents = cache.get(resolved_value)
-                        if parents is None:
-                            parents = []
-                            link = resolved_value
-                            while isinstance(link, ConfigValues):
-                                parents.append(link)
-                                link = link.overriden_value
-                            cache[resolved_value] = parents
-
-                    if isinstance(resolved_value, ConfigValues) \
-                       and substitution.parent in parents \
-                       and hasattr(substitution.parent, 'overriden_value') \
-                       and substitution.parent.overriden_value:
-
-                        # self resolution, backtrack
-                        resolved_value = substitution.parent.overriden_value
-
-                    unresolved, new_substitutions, result = cls._do_substitute(substitution, resolved_value, is_optional_resolved)
+                    for s in cache_values:
+                        is_optional_resolved, resolved_value = cls._resolve_variable(config, s)
+                        # if the substitution is optional
+                        if not is_optional_resolved and s.optional:
+                            resolved_value = None
+                        unresolved, new_subs, _ = cls._do_substitute(s, resolved_value, is_optional_resolved)
+                        if s in substitutions:
+                            substitutions.remove(s)
+                        # Detected substitutions may already be listed to process
+                        new_subs = [n for n in new_subs if n not in (substitutions, cache_values)]
+                        substitutions.extend(new_subs)
                     any_unresolved = unresolved or any_unresolved
-                    # Detected substitutions may already be listed to process
-                    new_substitutions = [n for n in new_substitutions if n not in substitutions]
-                    substitutions.extend(new_substitutions)
-                    if not isinstance(result, ConfigValues):
-                        substitutions.remove(substitution)
+
+                    if len(cache_values) == 0:
+                        any_unresolved = True
 
             cls._final_fixup(config)
-            if unresolved:
+            if any_unresolved:
                 has_unresolved = True
                 if not accept_unresolved:
                     raise ConfigSubstitutionException("Cannot resolve {variables}. Check for cycles.".format(