Fix strong mode errors, warnings, and hints.

[har79]

Part of dart-lang/tools#716.

• Added cast to jsonDocument in StreamChannel<String>.transform(jsonDocument). jsonDocument is a StreamChannelTransformer<Object, String> which doesn't implicitly cast to the required StreamChannelTransformer<String, dynamic>. (Removed because fix didn't work -- will investigate better fix).
• Change StreamChannel.transform(StreamChannelTransformer) to StreamChannelTransformer.bind(StreamChannel) which, according to the docs, is identical but has better type handling. (Instead of above initial attempt at fixing use of jsonDocument).
• Added types to ignoreFormatExceptions of StreamTransformer<Object, Object>.
• Changed declared return type of sendResponse and sendJsonResponse to void to match code.
• Removed syncFuture as it relies on a deprecated method and, since Dart 1.7, is just the same as new Future.sync.
• Some dartfmt churn.

[har79]

Ah, jsonDocument as StreamChannelTransformer<String, String> fails the tests.

[har79]

Fixed by using jsonDocument.bind(channel) instead of channel.transform(jsonDocument).

All tests pass and no output from dartanalyzer --strong.

[nex3]

Please remove these incidental reformattings. This package doesn't use dartfmt, and they make it harder to see the real changes.

[har79]

Done.

[nex3]

Just change this method to use async/await.

[har79]

Done.

[nex3]

Just catch (error, stackTrace)

[har79]

Doesn't work. Then error is final and the assignment in line 179 fails.

[nex3]

In that case, I'd just return the serialized exception directly in the is! RpcException case rather than re-assigning here. You could also pull out if (!request.containsKey('id')) return null to the beginning of the catch block.

[har79]

Done. After pulling the return null branch out; it seemed simpler to use a ternary for the two cases and assign to a temporary variable.

[nex3]

error.code may not be defined here, since we don't know anything about the type of error yet. I meant just pulling the request.containsKey() check here, and leaving the error.code check where it was.

[har79]

Just pulling the request.containsKey() check would change the logic since it's part of an &&. Although, actually my change also changes the logic as the created RpcException could pass this check.

There doesn't seem to be any way to easily simplify this; returning in the if (error is! RpcException would result in quite a bit of duplication.

[nex3]

I'm not sure I see the duplication... here's what I'd write:

} catch (error, stackTrace) {
  if (!request.containsKey("id")) return null;
  if (error is RpcException) return error.serialize(request);

  var chain = new Chain.forTrace(stackTrace);
  return new RpcException(error_code.SERVER_ERROR, getErrorMessage(error),
          data: {'full': error.toString(), 'stack': chain.toString()})
      .serialize(request);
}

[har79]

However that drops the error.code != error_code.INVALID_REQUEST. Before, if error were an RpcException with code INVALID_REQUEST, it would be serialised and returned; now null may be returned instead. To preserve that logic, you would have to change the first check to:

if ((error is RpcException && error.code != error_code.INVALID_REQUEST || error is! RpcException) &&
      !request.containsKey('id')) {
    return null;
}

[nex3]

Good point. What about:

} catch (error, stackTrace) {
  if (error is RpcException) {
    if (error.code != error_code.INVALID_REQUEST &&
        !request.containsKey("id")) {
      return null;
    } else {
      return error.serialize(request);
    }
  } else if (!request.containsKey("id")) {
    return null;
  }

  var chain = new Chain.forTrace(stackTrace);
  return new RpcException(error_code.SERVER_ERROR, getErrorMessage(error),
          data: {'full': error.toString(), 'stack': chain.toString()})
      .serialize(request);
}

There's still a little duplication, but it avoids allocating a RpcException just to return null.

[har79]

Cool. Gone for that, although with the booleans swapped around to use the positive conditions. Thanks!

[nex3]

Nit: try to avoid single-letter names.

[nex3]

One last nit: I prefer to avoid indentation by short-circuiting more than I prefer to use positive conditions, so I'd move the return null up earlier so you don't have to indent the RpcException creation.

[har79]

Done.