Replace json_utils with a modern extension type, add tests.

[matanlurey]

I started on a refactoring of flutter/flutter#147666, and was frustrated with the JSON decoding of gn desc --format=json, so I used a similar pattern from my micro-json library (https://pub.dev/packages/jsonut) and encapsulated it as an extension type.

My favorite is the JsonObject.map function which replaces:

/// Construct a RunTarget from a JSON map.
factory RunTarget.fromJson(Map<String, Object> map) {
  final List<String> errors = <String>[];
  final String name = stringOfJson(map, _nameKey, errors)!;
  final String id = stringOfJson(map, _idKey, errors)!;
  final String targetPlatform =
      stringOfJson(map, _targetPlatformKey, errors)!;

  if (errors.isNotEmpty) {
    throw FormatException('Failed to parse RunTarget: ${errors.join('\n')}');
  }
  return RunTarget._(name, id, targetPlatform);
}

... with:

/// Construct a RunTarget from a JSON map.
factory RunTarget.fromJson(Map<String, Object> map) {
  return JsonObject(map).map((JsonObject json) => RunTarget._(
    json.string(_nameKey),
    json.string(_idKey),
    json.string(_targetPlatformKey),
  ));
}

[zanderso]

This seems like a good pattern to follow, and I think it's necessary to use it when parsing JSON with this API in the situation that the errors that are accumulated are expected to be user-actionable. (For example, if we used this API for parsing the build config json under ci/builders.)

In this PR, the JSON that this API is parsing is internal. That is, if there's a problem with it, the user wouldn't necessarily be expected to fix it. So if the JSON exceptions aren't caught and make it to the terminal, that seems to be WAI.

But that leaves the awkward situation that the pattern noted here, which should be followed in certain situations, doesn't have a live example in this PR. My suggestion to avoid that awkwardness would be that we just always follow this pattern, so that future users of the API follow it as well even when it isn't strictly necessary.

[matanlurey]

But that leaves the awkward situation that the pattern noted here, which should be followed in certain situations, doesn't have a live example in this PR.

Do you mean that we don't have try/catch of JsonMapException?

I could imagine doing that, but as you mentioned we don't have any way to do recovery in this CLI - mal-formatted JSON means we'll have to stop, so the best we could do is try-catch and provide a more context-aware error message (for example 'Failed to parse process artifacts: $error').

[zanderso]

the best we could do is try-catch and provide a more context-aware error message (for example 'Failed to parse process artifacts: $error').

Yeah, I think if there's a nice place to try that in this PR, then that would be an improvement.

[matanlurey]

Sounds reasonable.

I might add a onError callback in that case so nesting doesn't get unnecessarily long.

[matanlurey]

Done, and added as discussed. Thanks!

[zanderso]

Needs the engine copyright header.

[matanlurey]

Done.

[zanderso]

I'm not sure how it would be useful, but if T can be a Future, then I'm not sure that the mechanism for saving and restoring the errors will work. Is there some way to say that T can't be a Future?

[matanlurey]

2018 Matan was ahead of us both: dart-lang/sdk#35024.

It is probably trivial to add such a hint assuming the analyzer team is on board. For now I'll do a runtime check that the result is not a Future.

[zanderso]

LGTM