Improve handling of git dependencies and improve associated tests
diff --git a/pkgs/pubspec_parse/build.yaml b/pkgs/pubspec_parse/build.yaml index ec706e4..930aad1 100644 --- a/pkgs/pubspec_parse/build.yaml +++ b/pkgs/pubspec_parse/build.yaml
@@ -6,6 +6,7 @@ json_serializable: generate_for: - lib/src/pubspec.dart + - lib/src/dependency.dart options: any_map: true checked: true
diff --git a/pkgs/pubspec_parse/lib/src/dependency.dart b/pkgs/pubspec_parse/lib/src/dependency.dart index 9353250..5260239 100644 --- a/pkgs/pubspec_parse/lib/src/dependency.dart +++ b/pkgs/pubspec_parse/lib/src/dependency.dart
@@ -4,6 +4,9 @@ import 'package:json_annotation/json_annotation.dart'; import 'package:pub_semver/pub_semver.dart'; +import 'package:pubspec_parse/src/errors.dart'; + +part 'dependency.g.dart'; Map<String, Dependency> parseDeps(Map source) => source?.map((k, v) { @@ -96,36 +99,43 @@ String get _info => name; } +@JsonSerializable(createToJson: false) class GitDependency extends Dependency { + @JsonKey(fromJson: _parseUri) final Uri url; final String ref; final String path; - GitDependency(this.url, this.ref, this.path) : super._(); + GitDependency(this.url, this.ref, this.path) : super._() { + if (url == null) { + throw new ArgumentError.value(url, 'url', '"url" cannot be null.'); + } + } factory GitDependency.fromData(Object data) { - String url; - String path; - String ref; - if (data is String) { - url = data; - } else if (data is Map) { - url = data['url'] as String; - path = data['path'] as String; - ref = data['ref'] as String; - } else { - throw new ArgumentError.value(data, 'git', 'Must be a String or a Map.'); + data = {'url': data}; } - // TODO: validate `url` is a valid URI - return new GitDependency(Uri.parse(url), ref, path); + if (data is Map) { + // TODO: Need JsonKey.required + // https://github.com/dart-lang/json_serializable/issues/216 + if (!data.containsKey('url')) { + throw new BadKeyException(data, 'url', '"url" is required.'); + } + + return _$GitDependencyFromJson(data); + } + + throw new ArgumentError.value(data, 'git', 'Must be a String or a Map.'); } @override String get _info => 'url@$url'; } +Uri _parseUri(String value) => Uri.parse(value); + class PathDependency extends Dependency { final String path;
diff --git a/pkgs/pubspec_parse/lib/src/dependency.g.dart b/pkgs/pubspec_parse/lib/src/dependency.g.dart new file mode 100644 index 0000000..cdb06bb --- /dev/null +++ b/pkgs/pubspec_parse/lib/src/dependency.g.dart
@@ -0,0 +1,20 @@ +// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// GENERATED CODE - DO NOT MODIFY BY HAND + +part of 'dependency.dart'; + +// ************************************************************************** +// Generator: JsonSerializableGenerator +// ************************************************************************** + +GitDependency _$GitDependencyFromJson(Map json) => $checkedNew( + 'GitDependency', + json, + () => new GitDependency( + $checkedConvert( + json, 'url', (v) => v == null ? null : _parseUri(v as String)), + $checkedConvert(json, 'ref', (v) => v as String), + $checkedConvert(json, 'path', (v) => v as String)));
diff --git a/pkgs/pubspec_parse/lib/src/errors.dart b/pkgs/pubspec_parse/lib/src/errors.dart index 9b1489c..288553c 100644 --- a/pkgs/pubspec_parse/lib/src/errors.dart +++ b/pkgs/pubspec_parse/lib/src/errors.dart
@@ -14,14 +14,14 @@ if (innerError is BadKeyException) { var map = innerError.map; if (map is YamlMap) { - var key = map.nodes.keys.singleWhere((key) { - return (key as YamlScalar).value == innerError.key; - }, orElse: () => null); + // if the associated key exists, use that as the error node, + // otherwise use the map itself + var node = map.nodes.keys.cast<YamlNode>().singleWhere((key) { + return key.value == innerError.key; + }, orElse: () => map); - if (key is YamlScalar) { - return new ParsedYamlException._(innerError.message, key, - innerError: error, innerStack: stack); - } + return new ParsedYamlException._(innerError.message, node, + innerError: error, innerStack: stack); } } else if (innerError is ParsedYamlException) { return innerError;
diff --git a/pkgs/pubspec_parse/test/dependency_test.dart b/pkgs/pubspec_parse/test/dependency_test.dart index 21c621d..8ede482 100644 --- a/pkgs/pubspec_parse/test/dependency_test.dart +++ b/pkgs/pubspec_parse/test/dependency_test.dart
@@ -29,10 +29,22 @@ expect(dep.toString(), 'SdkDependency: flutter'); }); - test('GitDependency', () { - var dep = _dependency<GitDependency>({'git': 'bob'}); - expect(dep.url.toString(), 'bob'); - expect(dep.toString(), 'GitDependency: url@bob'); + test('GitDependency - string', () { + var dep = _dependency<GitDependency>({'git': 'url'}); + expect(dep.url.toString(), 'url'); + expect(dep.path, isNull); + expect(dep.ref, isNull); + expect(dep.toString(), 'GitDependency: url@url'); + }); + + test('GitDependency - map', () { + var dep = _dependency<GitDependency>({ + 'git': {'url': 'url', 'path': 'path', 'ref': 'ref'} + }); + expect(dep.url.toString(), 'url'); + expect(dep.path, 'path'); + expect(dep.ref, 'ref'); + expect(dep.toString(), 'GitDependency: url@url'); }); test('HostedDepedency', () { @@ -90,6 +102,31 @@ ^^^'''); }); + test('git - empty map', () { + _expectThrows({'git': {}}, r''' +line 5, column 11: "url" is required. + "git": {} + ^^'''); + }); + + test('git - null url', () { + _expectThrows({ + 'git': {'url': null} + }, r''' +line 6, column 12: "url" cannot be null. + "url": null + ^^^^^'''); + }); + + test('git - int url', () { + _expectThrows({ + 'git': {'url': 42} + }, r''' +line 6, column 12: Unsupported value for `url`. + "url": 42 + ^^^'''); + }); + test('path - null content', () { _expectThrows({'path': null}, r''' line 5, column 12: Cannot be null.