Fix substitute inside includes to respect lists

This commit is contained in:
Ryan O'Neill 2015-04-22 16:34:44 -07:00
parent 462181578a
commit 24c6267362
5 changed files with 57 additions and 12 deletions

View File

@ -190,10 +190,10 @@ final class ConfigParser {
// we really should make this work, but for now throwing an
// exception is better than producing an incorrect result.
// See https://github.com/typesafehub/config/issues/160
if (arrayCount > 0 && obj.resolveStatus() != ResolveStatus.RESOLVED)
throw parseError("Due to current limitations of the config parser, when an include statement is nested inside a list value, "
+ "${} substitutions inside the included file cannot be resolved correctly. Either move the include outside of the list value or "
+ "remove the ${} statements from the included file.");
// if (arrayCount > 0 && obj.resolveStatus() != ResolveStatus.RESOLVED)
// throw parseError("Due to current limitations of the config parser, when an include statement is nested inside a list value, "
// + "${} substitutions inside the included file cannot be resolved correctly. Either move the include outside of the list value or "
// + "remove the ${} statements from the included file.");
if (!pathStack.isEmpty()) {
Path prefix = fullCurrentPath();
@ -245,10 +245,10 @@ final class ConfigParser {
// an exception is better than producing an incorrect
// result. See
// https://github.com/typesafehub/config/issues/160
if (arrayCount > 0)
throw parseError("Due to current limitations of the config parser, += does not work nested inside a list. "
+ "+= expands to a ${} substitution and the path in ${} cannot currently refer to list elements. "
+ "You might be able to move the += outside of the list and then refer to it from inside the list with ${}.");
// if (arrayCount > 0)
// throw parseError("Due to current limitations of the config parser, += does not work nested inside a list. "
// + "+= expands to a ${} substitution and the path in ${} cannot currently refer to list elements. "
// + "You might be able to move the += outside of the list and then refer to it from inside the list with ${}.");
// because we will put it in an array after the fact so
// we want this to be incremented during the parseValue
@ -356,6 +356,8 @@ final class ConfigParser {
AbstractConfigValue v = null;
int index = 0;
for (AbstractConfigNode node : n.children()) {
if (node instanceof ConfigNodeComment) {
comments.add(((ConfigNodeComment) node).commentText());
@ -376,7 +378,9 @@ final class ConfigParser {
values.add(v.withOrigin(v.origin().appendComments(new ArrayList<String>(comments))));
comments.clear();
}
pathStack.push(new Path(Integer.toString(index)));
v = parseValue((AbstractConfigNodeValue)node, comments);
pathStack.pop();
}
}
// There shouldn't be any comments at this point, but add them just in case

View File

@ -55,11 +55,15 @@ final class ResolveSource {
}
}
static private ValueWithPath findInObject(AbstractConfigObject obj, Path path) {
static private ValueWithPath findInObject(AbstractConfigValue obj, Path path) {
try {
// we'll fail if anything along the path can't
// be looked at without resolving.
return findInObject(obj, path, null);
if (obj instanceof AbstractConfigObject) {
return findInObject((AbstractConfigObject) obj, path, null);
} else {
return findInList((SimpleConfigList) obj, path, null);
}
} catch (ConfigException.NotResolved e) {
throw ConfigImpl.improveNotResolved(path, e);
}
@ -78,6 +82,29 @@ final class ResolveSource {
} else {
if (v instanceof AbstractConfigObject) {
return findInObject((AbstractConfigObject) v, next, newParents);
} else if (v instanceof SimpleConfigList) {
return findInList((SimpleConfigList) v, next, newParents);
} else {
return new ValueWithPath(null, newParents);
}
}
}
static private ValueWithPath findInList(SimpleConfigList list, Path path, Node<Container> parents) {
String key = path.first();
Path next = path.remainder();
if (ConfigImpl.traceSubstitutionsEnabled())
ConfigImpl.trace("*** looking up '" + key + "' in " + list);
AbstractConfigValue v = list.get(Integer.parseInt(key));
Node<Container> newParents = parents == null ? new Node<Container>(list) : parents.prepend(list);
if (next == null) {
return new ValueWithPath(v, newParents);
} else {
if (v instanceof AbstractConfigObject) {
return findInObject((AbstractConfigObject) v, next, newParents);
} else if (v instanceof SimpleConfigList) {
return findInList((SimpleConfigList) v, next, newParents);
} else {
return new ValueWithPath(null, newParents);
}

View File

@ -75,8 +75,9 @@ final class SimpleConfig implements Config, MergeableValue, Serializable {
if (resolved == object)
return this;
else
else {
return new SimpleConfig((AbstractConfigObject) resolved);
}
}
private ConfigValue hasPathPeek(String pathExpression) {

View File

@ -1,5 +1,10 @@
// The {} inside the [] is needed because
// just [ include ] means an array with the
// string "include" in it.
a = [ { include "test01.conf" } ]
a = [
{
include "test12.conf"
replace-key: "replaced"
}
]

View File

@ -801,6 +801,14 @@ class ConfParserTest extends TestUtils {
assertTrue("including basename URL doesn't load anything", conf.isEmpty())
}
@Test
def includeWithSubstitutionsInArray() {
val conf = ConfigFactory.parseString("include file(" + jsonQuotedResourceFile("include-from-list") + ")")
val resolved = conf.resolve()
assertEquals(resolved.getConfigList("a").get(0).getString("replace-me"), "replaced")
}
@Test
def acceptBOMStartingFile() {
// BOM at start of file should be ignored