Skip to content

Commit f161db1

Browse files
committed
fix(read): Improve handling of Key UDA for empty mapping
There were several issues uncovered by those test cases: - The path and location information of the error message pointed to the parent node, not the node that was triggering an error. So if the error was for the 5th entry in a list, the user would be unhelpfully and confusingly pointed to the beginning of the list and not the offending node; - Getting a YAML scalar node that was null would return 'null', not a D null value; This might need to be handled differently in the future if we need to distinguish between actual null and empty strings; - Finally, empty mapping are parsed as null scalar by default by D-YAML, which makes sense as D-YAML does not know what structure to expect, but we do, so we need to explicitly handle the null case as an empty object;
1 parent 75d6f43 commit f161db1

File tree

2 files changed

+73
-9
lines changed

2 files changed

+73
-9
lines changed

source/configy/read.d

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -782,15 +782,31 @@ package FR.Type parseField (alias FR)
782782
string key = getUDAs!(FR.Ref, Key)[0].name;
783783
return node.mapping().map!(
784784
(Node.Pair pair) {
785-
if (pair.value.nodeID != NodeID.mapping)
786-
throw new TypeConfigException(
787-
"sequence of " ~ pair.value.nodeTypeString(),
788-
"sequence of mapping (array of objects)",
789-
path, Location.get(node));
790-
791-
return pair.value.parseMapping!(StructFieldRef!E)(
792-
path.addPath(pair.key.as!string),
793-
E.init, ctx, key.length ? [ key: pair.key ] : null);
785+
scope npath = path.addPath(pair.key.as!string);
786+
// Normal / expected path
787+
if (pair.value.nodeID == NodeID.mapping)
788+
return pair.value.parseMapping!(StructFieldRef!E)(npath,
789+
E.init, ctx, key.length ? [ key: pair.key ] : null);
790+
791+
// It might be a single entry, e.g.
792+
// ---
793+
// values:
794+
// morevalues:
795+
// key: value
796+
// ---
797+
// In this instance, `values` is an empty mapping but might be
798+
// interpreted as a scalar.
799+
if (pair.value.type() == NodeType.null_) {
800+
string[string] aa;
801+
Node empty = aa;
802+
// Ugly hack to keep the location information
803+
__traits(getMember, empty, "startMark_") = pair.value.startMark();
804+
return empty.parseMapping!(StructFieldRef!E)(npath,
805+
E.init, ctx, key.length ? [ key: pair.key ] : null);
806+
}
807+
808+
throw new TypeConfigException(pair.value.nodeTypeString(),
809+
"mapping", npath, Location.get(pair.value));
794810
}).array();
795811
}
796812
else

source/configy/test.d

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,3 +986,51 @@ ds:
986986
catch (ConfigException exc)
987987
assert(exc.toString() == "/dev/null(1:11): es.enabled: Expected to be a value of type bool, but is a scalar");
988988
}
989+
990+
unittest {
991+
static struct ImageConfig {
992+
// Only this field is required
993+
public string name;
994+
public string containerfile = "Containerfile";
995+
public @Optional string[] tags;
996+
}
997+
998+
static struct ImageConfig2 {
999+
// Both fields are required
1000+
public string name;
1001+
public string file;
1002+
}
1003+
1004+
static struct Config {
1005+
public @Optional @Key("name") ImageConfig[] images;
1006+
public @Optional @Key("name") ImageConfig2[] images2;
1007+
}
1008+
1009+
auto c = parseConfigString!Config(`images:
1010+
foo/bar/root:
1011+
containerfile: path/Containerfile
1012+
tags: [ "latest" ]
1013+
far/boo/runner:
1014+
`, "/dev/null");
1015+
1016+
assert(c.images[0].name == "foo/bar/root");
1017+
assert(c.images[0].containerfile == "path/Containerfile");
1018+
assert(c.images[1].name == "far/boo/runner");
1019+
1020+
try parseConfigString!Config(`images:
1021+
foo/bar/root:
1022+
containerfile: path/Containerfile
1023+
tags: [ "latest" ]
1024+
far/boo/runner: [ 1, 2, 3]
1025+
`, "/dev/null");
1026+
catch (ConfigException exc)
1027+
assert(exc.toString() == "/dev/null(4:18): images.far/boo/runner: Expected to be mapping, but is a sequence");
1028+
1029+
try parseConfigString!Config(`images2:
1030+
foo/bar/root:
1031+
file: path/Containerfile
1032+
far/boo/runner:
1033+
`, "/dev/null");
1034+
catch (ConfigException exc)
1035+
assert(exc.toString() == "/dev/null(3:17): images2.far/boo/runner.file: Required key was not found in configuration or command line arguments");
1036+
}

0 commit comments

Comments
 (0)