Support empty keys in block and flow mappings - #558
Conversation
Coverage Report for CI Build 32262347650Coverage remained the same at 100.0%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
An empty key was a special case at the document root only, where it produced
an entry with an empty string key. Everywhere else a key separator without a
preceding key was rejected, and in a flow mapping it was ignored, which
dropped the entry:
```yaml
key: value
: empty key # parse_error: Detected invalid indentation
- : foo # parse_error: mapping key should not be empty
{ key: value, : empty key }
# parse_error: The ":" mapping value indicator must be
# followed after a mapping key
```
Parse an empty key as a null node, as the YAML specification does, and add the
entry wherever a key separator is found with no key before it: at the start of
a line in a block mapping, in a block sequence entry, and in a flow mapping.
The value of an empty key can be omitted as well, so add_empty_key_entry()
also moves back to the parent mapping when the following token is not indented
deeper, the same way an omitted value is handled for a normal key.
This changes the type of an empty key from an empty string to a null node,
which is a breaking change for anyone reading such an entry as node[""].
A quoted empty key ("": value) is unaffected and remains a string.
Spec example 8.18 (test suite case S3PD) and the empty key cases NKF9 and PW8X
of the yaml-test-suite now parse. CFD4 (an empty key in a single pair flow
sequence) is still rejected: it needs single pair flow mappings, which are not
supported for non-empty keys either ([foo: 1] is rejected as well).
|
I added the The reasoning follows yours:
The last two rows come along for free, and I think they are worth pointing out: I'll open the two issues you suggested, for an omitted value in a flow mapping ( |
The contents of an explicit key may themselves be a mapping entry with an empty key, as pointed out in fktn-k#557: `? ` introduces an explicit key, and the `:` which follows it on the same line belongs to that key rather than to the entry the key is part of. Handle a key separator which begins the contents of an explicit key by turning the key node into a mapping and adding an entry with an empty key to it, the same way a scalar key does. The key separator which later terminates the explicit key can now find its context below the ones its own contents left on the stack, and completing a pending explicit key at the end of a document unwinds those contexts too. | input | before | after | |---|---|---| | `? :` | `{null: null}` | `{{null: null}: null}` | | `? : foo` | `{foo: null}` | `{{null: foo}: null}` | | `? foo: bar` | `{}` | `{{foo: bar}: null}` | | `? :` / `: baz` | parse_error | `{{null: null}: baz}` | The third row was silently dropping the whole entry before, and the fourth was rejected outright; both are the same shape as the case this issue is about.
This PR fixes #557.
An empty key was only handled at the very start of a document, and it produced an entry whose key
is an empty string. Everywhere else the key separator was rejected, and inside a flow mapping it
was ignored, which dropped the entry:
The change
add_empty_key_entry()adds an entry with a null key and moves to the token after the separator.It is called wherever a key separator turns up with no key before it: at the document root, at the
start of a line in a block mapping, in a block sequence entry, and in a flow mapping. The
deliberate
mapping key should not be emptyrejection for block sequence entries is gone.The value of an empty key can be omitted too, so the helper also moves back to the parent mapping
when the following token is not indented deeper - the same rule a normal key already follows:
": foo"{"": foo}{null: foo}":"{"": null}{null: null}"key: value\n: empty key"parse_error{key: value, null: 'empty key'}":\nb: 1"parse_error{null: null, b: 1}"- : foo"parse_error[{null: foo}]"{key: value, : empty key}"parse_error{key: value, null: 'empty key'}"\"\": foo"{"": foo}Breaking change
As you noted in #553, an empty key is now a null node rather than an empty string, so code reading
such an entry as
node[""]has to readnode[nullptr]instead. Two existing subcases intest_deserializer_class.cppasserted the old behavior and are updated here. I did not touch anyversion macro, assuming the v0.5.0 bump happens at release time as usual.
Pull Request Checklist
Read the CONTRIBUTING.md file for detailed information.
Please don't
#ifdefs or other means.