gvvaughan / lyaml

LibYAML binding for Lua.
gvvaughan.github.io/lyaml
Other
208 stars 34 forks source link

Prevent losing entries with mixed key tables #40

Closed iwanders closed 3 years ago

iwanders commented 3 years ago

Hi, currently, the serialization is losing entries when serializing tables with a mix of integer keys and string keys or non consecutive integer keys.

Take the example:

local lyaml   = require "lyaml"
local test = {}
test[1] = "one"
test[2] = "two"
test["three"] = "three"
print(lyaml.dump({test}))

This prints:

---
- one
- two
...

Thereby dropping the three: "three" entry, I would argue that the expected behaviour is preserving all entries in the table and thus serializing this as a mapping instead of a sequence.

First commit commit that introduces unit tests contain mixed keys and fail. The second commit makes the check here more stringent, by only using sequences if the table only has integer keys that are sequential, start at one and don't skip any values. At first I tried to compare #node with the number of elements in the table, but that is broken for cases such as t in the following snippet:

local t = {}
t[2] = 2
t[3] = 3
t[4] = 4
t.foo = "bar"

local t2 = {[2]=2, [3]=3, [4]=4, ["foo"] = "bar"}

Where the count #t will be equal to the number of elements, but it should still be serialized as a mapping. I couldn't actually get t into the unit test system being used in this package. I instead added t2, but these are not equivalent, #t is equal to 4, whereas #t2 is equal to 0. In commit three I added some more unit tests.

All testing done on Lua 5.3 (installed via apt) on Focal, initially also installed this module from apt.

gvvaughan commented 3 years ago

Nice catch. And thanks for the fix and improved tests, much appreciated!