json icon indicating copy to clipboard operation
json copied to clipboard

Json.Decode.index checks for too large index but not too small

Open lydell opened this issue 5 years ago • 0 comments

If the index given to Json.Decode.index is too large we get this message:

> JD.decodeString (JD.index 1 JD.int) """[1]""" |> Result.mapError JD.errorToString
Err ("Problem with the given value:\n\n[\n        1\n    ]\n\nExpecting a LONGER array. Need index 1 but only see 1 entries")
    : Result String Int

In other words, there is an out-of-bounds check that produces that nice message. But the index can also be out-of-bounds by being negative. In that case, there is currently no check for this, so array[-1] (which is undefined) is attempted to be decoded:

> JD.decodeString (JD.index -1 JD.int) """[1]""" |> Result.mapError JD.errorToString
Err ("Problem with the value at json[-1]:\n\n    undefined\n\nExpecting an INT")
    : Result String Int

That’s still an error message, but not quite as nice. But using -1 doesn’t always fail:

> JD.decodeString (JD.index 1 (JD.succeed ())) """[1]""" |> Result.mapError JD.errorToString
Err ("Problem with the given value:\n\n[\n        1\n    ]\n\nExpecting a LONGER array. Need index 1 but only see 1 entries")
    : Result String ()
> JD.decodeString (JD.index -1 (JD.succeed ())) """[1]""" |> Result.mapError JD.errorToString
Ok () : Result String ()

I think the error message could be nicer for a negative index. Similar to how oneOf has a nice error message for an empty list:

> JD.decodeString (JD.oneOf []) """true""" |> Result.mapError JD.errorToString
Err ("Ran into a Json.Decode.oneOf with no possibilities!")

Maybe something like this: "Ran into a Json.Decode.index with a negative index: -1".

In summary:

  • If there’s an upper bounds check, why is there no lower?
  • The error message for negative index could be better.
  • An out-of-bounds index should always fail (not just when too large) for consistency.

Note: I only noticed this because I’m re-implementing this package in Elm as a learning exercise. I’ve never accidentally passed a negative index in real code.

lydell avatar Sep 26 '19 21:09 lydell