flyx / NimYAML

YAML implementation for Nim
https://nimyaml.org
Other
186 stars 36 forks source link

ignore pragma doesn't work on nested maps #127

Closed daylinmorgan closed 1 year ago

daylinmorgan commented 1 year ago

When using the {.ignore: [].} pragma to ignore all unknown fields an error is raised if the value is a map.

import yaml

type
  IgnoreMap {.ignore: [].} = object
    warbl: int

  IgnoreNamedMap {.ignore: ["rtuco"].} = object
    warbl: int

let nestedInput = """
rtuco:
  foo: bar
warbl: 1
"""

proc attemptLoad[T](s:string, x:typedesc[T], pragma:string) =
  var result: T
  try: 
    load(s, result)
  except YamlConstructionError as e:
    echo "failed to load with " & pragma
    echo "error:" & e.msg

attemptLoad(nestedInput,IgnoreMap,"ignore: []")
attemptLoad(nestedInput,IgnoreNamedMap,"ignore:[\"rtuco\"]")

The above fails with IgnoreMap and raises:

failed to load with ignore: []
error:Expected field name, got yamlStartMap

If this is intentional is there a way to specify that nested data should be ignored on an object without knowing the field name?

flyx commented 1 year ago

Certainly not intentional, this is an error.

Error is here:

https://github.com/flyx/NimYAML/blob/7dd8e88639b86f43d0a76f7c3763234c68741026/yaml/serialization.nim#L816-L821

This portion of the macro creates the branch that is executed when a field name is not mappable. The generated AST looks like this:

when failOnError:
  raise constructionError(…)

when ignore has been set and is empty, this branch is reached with failOnError being false, so… nothing happens. This is bad.

Compare what happens when there are explicitly ignored field names and a field is encountered with one of those names:

https://github.com/flyx/NimYAML/blob/7dd8e88639b86f43d0a76f7c3763234c68741026/yaml/serialization.nim#L892-L899

The fix would be:

My Nim is a bit rusty so if you want to tackle this a PR would be very welcome :)