apple / pkl

A configuration as code language with rich validation and tooling.
https://pkl-lang.org
Apache License 2.0
9.84k stars 259 forks source link

`json.Parser` should throw an error when `useMapping = false` and key "default" is encountered #561

Open HT154 opened 3 days ago

HT154 commented 3 days ago

The behavior as-is is very surprising! Check this out:

import "pkl:json"

hidden jsonString = """
  {
    "hello": "world",
    "default": "greeting"
  }
  """

asDynamic = new json.Parser {}.parse(jsonString)
asMapping = new json.Parser { useMapping = true }.parse(jsonString)

Result:

asDynamic {
  hello = "world"
}
asMapping {
  ["hello"] = "world"
  ["default"] = "greeting"
}

I think throwing an error (or at least some kind of warning) here instead of returning unexpected results is desirable. Something like "encountered object key default when parsing JSON, resulting Dynamic will be incomplete or invalid" might make sense.

N.B.: Attempting to access the default yields further surprises:

asDynamicDefault = asDynamic.default

Result:

❯ pkl eval test.pkl
–– Pkl Error ––
Expected value of type `Function1`, but got type `String`.
Value: "greeting"

1741 | hidden default: (unknown) -> Any = (_) -> new Dynamic {}
                       ^^^^^^^^^^^^^^^^
at pkl.base#Dynamic.default (https://github.com/apple/pkl/blob/0.26.0/stdlib/base.pkl#L1741)

1 | 
    ^
at generated (source:unavailable)

11 | asDynamicDefault = asDynamic.default
                        ^^^^^^^^^^^^^^^^^
at test#asDynamicDefault (file:///Users/jbasch/src/tsukemono/test.pkl, line 11)

106 | text = renderer.renderDocument(value)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (https://github.com/apple/pkl/blob/0.26.0/stdlib/base.pkl#L106)