leafpetersen / cast

Dart library for type schemas for casting and parsing structured data
BSD 3-Clause "New" or "Revised" License
10 stars 2 forks source link

Meta issue for discussion of a debug API #4

Open leafpetersen opened 6 years ago

leafpetersen commented 6 years ago

Should there be a separate debug version of the classes? I deliberately avoided keeping a full trace of the path through the tree to the current parse point, since that requires allocation and/or putting handlers at every step. For performance, we should probably avoid that, at least in production. We could do nasty things with asserts, but it might be better to provide a separate version of the library cast_debug.dart that extends each class and intercepts the _cast calls to keep a stack. Is this necessary, or can we give good enough error messages without this?

natebosch commented 6 years ago

We definitely need to be able to give actionable errors to users if this data is coming from some configuration file and they wrote an incorrect type.

leafpetersen commented 6 years ago

Right - a question is, what level of detail is actionable? For example, given:

  var json = <String, dynamic>{
    "id": 0,
    "fields": <String, dynamic>{"field1": 1, "field2": 2},
    "extra1": "hello",
    "extra2": <dynamic>["hello"],
  };

    const typed = cast.Keyed<String, dynamic>({
      "id": cast.int,
      "fields": cast.Keyed({"field1": cast.int, "field2": cast.String}),
      "extra1": cast.OneOf(cast.String, cast.List(cast.String)),
      "extra2": cast.OneOf(cast.String, cast.List(cast.String)),
    });

Is this actionable enough?

  Failed cast at map entry field2: 2 is not a String
  package:cast/src/cast.dart 47:13               StringCast._cast
  package:cast/src/cast.dart 119:29              Keyed._cast
  package:cast/src/cast.dart 119:29              Keyed._cast
  package:cast/src/cast.dart 23:27               Cast.cast
  test/json_test.dart 57:11                      main.<fn>

Or do we need something more like:

Failed cast, "2 is not a String":
  at map entry for key "field2" 
  at map entry for key "fields"

The latter requires keeping a stack of the keys encountered along the path to the current cast point.

Not hard to do, and we could easily add a debug version that you could just swap in by importing cast_debug.dart, but I'm would be a little hesitant to put this on the fast path (especially for parsing).

natebosch commented 6 years ago

I suspect we need the latter. To get to parity with what we're able to do with package:yaml and hardcoded casts I think we'd need something even more complex since we can currently point to the line in the source file with an issue.

Hopefully with a path through keys (and indexes) we can figure out a way to get the source span from package:yaml

leafpetersen commented 6 years ago

For that though, I think you're not casting, you're parsing. That's what I'd like to get to. I'd like to either have (as part of the package) a parseYaml method on Cast, or have yaml subclass this in some way to provide it's own parsing. So then you don't do: schema.cast(parseYaml(string)), but rather schema.parseYaml(string) and your error messages come out in terms of source positions.

natebosch commented 6 years ago

Ah I see. The distinction between "parse" and "cast" makes sense to me. I don't have strong opinions on whether or when we need the extra context for "cast"