use pkg:checked_yaml
diff --git a/pkgs/pubspec_parse/lib/pubspec_parse.dart b/pkgs/pubspec_parse/lib/pubspec_parse.dart index 99886c4..132263a 100644 --- a/pkgs/pubspec_parse/lib/pubspec_parse.dart +++ b/pkgs/pubspec_parse/lib/pubspec_parse.dart
@@ -9,5 +9,4 @@ GitDependency, SdkDependency, PathDependency; -export 'src/errors.dart' show ParsedYamlException; export 'src/pubspec.dart' show Pubspec;
diff --git a/pkgs/pubspec_parse/lib/src/dependency.dart b/pkgs/pubspec_parse/lib/src/dependency.dart index 364e703..e2b0770 100644 --- a/pkgs/pubspec_parse/lib/src/dependency.dart +++ b/pkgs/pubspec_parse/lib/src/dependency.dart
@@ -47,20 +47,21 @@ if (data.isEmpty || (matchedKeys.isEmpty && data.containsKey('version'))) { return _$HostedDependencyFromJson(data); } else { - final weirdKey = matchedKeys.firstWhere((k) => !_sourceKeys.contains(k), - orElse: () => null); + final firstUnrecognizedKey = matchedKeys + .firstWhere((k) => !_sourceKeys.contains(k), orElse: () => null); - if (weirdKey != null) { - throw UnrecognizedKeysException([weirdKey], data, _sourceKeys); - } - if (matchedKeys.length > 1) { - throw CheckedFromJsonException(data, matchedKeys[1], 'Dependency', - 'A dependency may only have one source.'); - } + return $checkedNew<Dependency>('Dependency', data, () { + if (firstUnrecognizedKey != null) { + throw UnrecognizedKeysException( + [firstUnrecognizedKey], data, _sourceKeys); + } + if (matchedKeys.length > 1) { + throw CheckedFromJsonException(data, matchedKeys[1], 'Dependency', + 'A dependency may only have one source.'); + } - final key = matchedKeys.single; + final key = matchedKeys.single; - try { switch (key) { case 'git': return GitDependency.fromData(data[key]); @@ -72,10 +73,7 @@ return _$HostedDependencyFromJson(data); } throw StateError('There is a bug in pubspec_parse.'); - } on ArgumentError catch (e) { - throw CheckedFromJsonException( - data, e.name, 'Dependency', e.message.toString()); - } + }); } }
diff --git a/pkgs/pubspec_parse/lib/src/errors.dart b/pkgs/pubspec_parse/lib/src/errors.dart deleted file mode 100644 index 6d2b38c..0000000 --- a/pkgs/pubspec_parse/lib/src/errors.dart +++ /dev/null
@@ -1,74 +0,0 @@ -// 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. - -import 'package:json_annotation/json_annotation.dart'; -import 'package:yaml/yaml.dart'; - -ParsedYamlException parsedYamlException(String message, YamlNode yamlNode) => - ParsedYamlException._(message, yamlNode); - -ParsedYamlException parsedYamlExceptionFromError( - CheckedFromJsonException error, StackTrace stack) { - final innerError = error.innerError; - if (innerError is UnrecognizedKeysException) { - final map = innerError.map; - if (map is YamlMap) { - // if the associated key exists, use that as the error node, - // otherwise use the map itself - final node = map.nodes.keys.cast<YamlNode>().firstWhere((key) { - return innerError.unrecognizedKeys.contains(key.value); - }, orElse: () => map); - - return ParsedYamlException._(innerError.message, node, - innerError: error, innerStack: stack); - } - } else if (innerError is ParsedYamlException) { - return innerError; - } - - final yamlMap = error.map as YamlMap; - var yamlNode = yamlMap.nodes[error.key]; - - String message; - if (yamlNode == null) { - assert(error.message != null); - message = error.message; - yamlNode = yamlMap; - } else { - if (error.message == null) { - message = 'Unsupported value for `${error.key}`.'; - } else { - message = error.message.toString(); - } - } - - return ParsedYamlException._(message, yamlNode, - innerError: error, innerStack: stack); -} - -/// Thrown when parsing a YAML document fails. -class ParsedYamlException implements Exception { - /// Describes the nature of the parse failure. - final String message; - - final YamlNode yamlNode; - - /// If this exception was thrown as a result of another error, - /// contains the source error object. - final Object innerError; - - /// If this exception was thrown as a result of another error, - /// contains the corresponding [StackTrace]. - final StackTrace innerStack; - - ParsedYamlException._(this.message, this.yamlNode, - {this.innerError, this.innerStack}); - - /// Returns [message] formatted with source information provided by - /// [yamlNode]. - String get formattedMessage => yamlNode.span.message(message); - - @override - String toString() => message; -}
diff --git a/pkgs/pubspec_parse/lib/src/pubspec.dart b/pkgs/pubspec_parse/lib/src/pubspec.dart index a8dacf6..d31e657 100644 --- a/pkgs/pubspec_parse/lib/src/pubspec.dart +++ b/pkgs/pubspec_parse/lib/src/pubspec.dart
@@ -4,10 +4,9 @@ import 'package:json_annotation/json_annotation.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:yaml/yaml.dart'; +import 'package:checked_yaml/checked_yaml.dart'; import 'dependency.dart'; -import 'errors.dart'; part 'pubspec.g.dart'; @@ -108,7 +107,7 @@ try { final targetUri = Uri.parse(publishTo); if (!(targetUri.isScheme('http') || targetUri.isScheme('https'))) { - throw const FormatException('must be an http or https URL.'); + throw const FormatException('Must be an http or https URL.'); } } on FormatException catch (e) { throw ArgumentError.value(publishTo, 'publishTo', e.message); @@ -144,25 +143,9 @@ factory Pubspec.parse(String yaml, {sourceUrl, bool lenient = false}) { lenient ??= false; - final item = loadYaml(yaml, sourceUrl: sourceUrl); - - if (item == null) { - throw ArgumentError.notNull('yaml'); - } - - if (item is! YamlMap) { - if (item is YamlNode) { - throw parsedYamlException('Does not represent a YAML map.', item); - } - - throw ArgumentError.value(yaml, 'yaml', 'Does not represent a YAML map.'); - } - - try { - return Pubspec.fromJson(item as YamlMap, lenient: lenient); - } on CheckedFromJsonException catch (error, stack) { - throw parsedYamlExceptionFromError(error, stack); - } + return checkedYamlDecode( + yaml, (map) => Pubspec.fromJson(map, lenient: lenient), + sourceUrl: sourceUrl); } static List<String> _normalizeAuthors(String author, List<String> authors) { @@ -185,7 +168,13 @@ if (key == 'dart') { // github.com/dart-lang/pub/blob/d84173eeb03c3/lib/src/pubspec.dart#L342 // 'dart' is not allowed as a key! - throw UnrecognizedKeysException(['dart'], source, ['sdk']); + throw CheckedFromJsonException( + source, + 'dart', + 'VersionConstraint', + 'Use "sdk" to for Dart SDK constraints.', + badKey: true, + ); } VersionConstraint constraint;
diff --git a/pkgs/pubspec_parse/pubspec.yaml b/pkgs/pubspec_parse/pubspec.yaml index c1df008..2ae7fb2 100644 --- a/pkgs/pubspec_parse/pubspec.yaml +++ b/pkgs/pubspec_parse/pubspec.yaml
@@ -10,6 +10,7 @@ sdk: '>=2.2.0 <3.0.0' dependencies: + checked_yaml: ^1.0.0 # Verified that no new features since 1.0.0 are used - be careful! json_annotation: '>=1.0.0 <3.0.0' pub_semver: ^1.3.2
diff --git a/pkgs/pubspec_parse/test/dependency_test.dart b/pkgs/pubspec_parse/test/dependency_test.dart index 9e90997..9480105 100644 --- a/pkgs/pubspec_parse/test/dependency_test.dart +++ b/pkgs/pubspec_parse/test/dependency_test.dart
@@ -18,32 +18,41 @@ group('errors', () { test('List', () { - _expectThrows([], r''' -line 4, column 10: Not a valid dependency value. + _expectThrows( + [], + r''' +line 4, column 10: Unsupported value for "dep". Not a valid dependency value. ╷ 4 │ "dep": [] │ ^^ - ╵'''); + ╵''', + ); }); test('int', () { - _expectThrows(42, r''' -line 4, column 10: Not a valid dependency value. + _expectThrows( + 42, + r''' +line 4, column 10: Unsupported value for "dep". Not a valid dependency value. ╷ 4 │ "dep": 42 │ ┌──────────^ 5 │ │ } │ └─^ - ╵'''); + ╵''', + ); }); test('map with too many keys', () { - _expectThrows({'path': 'a', 'git': 'b'}, r''' -line 5, column 12: A dependency may only have one source. + _expectThrows( + {'path': 'a', 'git': 'b'}, + r''' +line 5, column 12: Unsupported value for "path". A dependency may only have one source. ╷ 5 │ "path": "a", │ ^^^ - ╵'''); + ╵''', + ); }); test('map with unsupported keys', () { @@ -79,12 +88,15 @@ }); test('bad string version', () { - _expectThrows('not a version', r''' -line 4, column 10: Could not parse version "not a version". Unknown text at "not a version". + _expectThrows( + 'not a version', + r''' +line 4, column 10: Unsupported value for "dep". Could not parse version "not a version". Unknown text at "not a version". ╷ 4 │ "dep": "not a version" │ ^^^^^^^^^^^^^^^ - ╵'''); + ╵''', + ); }); test('map w/ just version', () { @@ -106,15 +118,18 @@ }); test('map w/ bad version value', () { - _expectThrows({ - 'version': 'not a version', - 'hosted': {'name': 'hosted_name', 'url': 'hosted_url'} - }, r''' -line 5, column 15: Could not parse version "not a version". Unknown text at "not a version". + _expectThrows( + { + 'version': 'not a version', + 'hosted': {'name': 'hosted_name', 'url': 'hosted_url'} + }, + r''' +line 5, column 15: Unsupported value for "version". Could not parse version "not a version". Unknown text at "not a version". ╷ 5 │ "version": "not a version", │ ^^^^^^^^^^^^^^^ - ╵'''); + ╵''', + ); }); test('map w/ extra keys should fail', () { @@ -148,14 +163,15 @@ }); test('map w/ null hosted should error', () { - _expectThrows({'hosted': null}, r''' -line 5, column 14: These keys had `null` values, which is not allowed: [hosted] + _expectThrows( + {'hosted': null}, + r''' +line 5, column 4: These keys had `null` values, which is not allowed: [hosted] ╷ -5 │ "hosted": null - │ ┌──────────────^ -6 │ │ } - │ └──^ - ╵'''); +5 │ "hosted": null + │ ^^^^^^^^ + ╵''', + ); }); test('map w/ null version is fine', () { @@ -183,25 +199,29 @@ }); test('null content', () { - _expectThrows({'sdk': null}, r''' -line 5, column 11: These keys had `null` values, which is not allowed: [sdk] + _expectThrows( + {'sdk': null}, + r''' +line 5, column 4: These keys had `null` values, which is not allowed: [sdk] ╷ -5 │ "sdk": null - │ ┌───────────^ -6 │ │ } - │ └──^ - ╵'''); +5 │ "sdk": null + │ ^^^^^ + ╵''', + ); }); test('number content', () { - _expectThrows({'sdk': 42}, r''' -line 5, column 11: Unsupported value for `sdk`. + _expectThrows( + {'sdk': 42}, + r''' +line 5, column 11: Unsupported value for "sdk". ╷ 5 │ "sdk": 42 │ ┌───────────^ 6 │ │ } │ └──^ - ╵'''); + ╵''', + ); }); } @@ -256,25 +276,31 @@ }); test('git - null content', () { - _expectThrows({'git': null}, r''' -line 5, column 11: Must be a String or a Map. + _expectThrows( + {'git': null}, + r''' +line 5, column 11: Unsupported value for "git". Must be a String or a Map. ╷ 5 │ "git": null │ ┌───────────^ 6 │ │ } │ └──^ - ╵'''); + ╵''', + ); }); test('git - int content', () { - _expectThrows({'git': 42}, r''' -line 5, column 11: Must be a String or a Map. + _expectThrows( + {'git': 42}, + r''' +line 5, column 11: Unsupported value for "git". Must be a String or a Map. ╷ 5 │ "git": 42 │ ┌───────────^ 6 │ │ } │ └──^ - ╵'''); + ╵''', + ); }); test('git - empty map', () { @@ -287,29 +313,33 @@ }); test('git - null url', () { - _expectThrows({ - 'git': {'url': null} - }, r''' -line 6, column 12: These keys had `null` values, which is not allowed: [url] + _expectThrows( + { + 'git': {'url': null} + }, + r''' +line 6, column 5: These keys had `null` values, which is not allowed: [url] ╷ -6 │ "url": null - │ ┌────────────^ -7 │ │ } - │ └───^ - ╵'''); +6 │ "url": null + │ ^^^^^ + ╵''', + ); }); test('git - int url', () { - _expectThrows({ - 'git': {'url': 42} - }, r''' -line 6, column 12: Unsupported value for `url`. + _expectThrows( + { + 'git': {'url': 42} + }, + r''' +line 6, column 12: Unsupported value for "url". ╷ 6 │ "url": 42 │ ┌────────────^ 7 │ │ } │ └───^ - ╵'''); + ╵''', + ); }); } @@ -337,25 +367,31 @@ }); test('null content', () { - _expectThrows({'path': null}, r''' -line 5, column 12: Must be a String. + _expectThrows( + {'path': null}, + r''' +line 5, column 12: Unsupported value for "path". Must be a String. ╷ 5 │ "path": null │ ┌────────────^ 6 │ │ } │ └──^ - ╵'''); + ╵''', + ); }); test('int content', () { - _expectThrows({'path': 42}, r''' -line 5, column 12: Must be a String. + _expectThrows( + {'path': 42}, + r''' +line 5, column 12: Unsupported value for "path". Must be a String. ╷ 5 │ "path": 42 │ ┌────────────^ 6 │ │ } │ └──^ - ╵'''); + ╵''', + ); }); }
diff --git a/pkgs/pubspec_parse/test/parse_test.dart b/pkgs/pubspec_parse/test/parse_test.dart index 3c79d87..ca822e7 100644 --- a/pkgs/pubspec_parse/test/parse_test.dart +++ b/pkgs/pubspec_parse/test/parse_test.dart
@@ -75,29 +75,29 @@ group('publish_to', () { for (var entry in { 42: r''' -line 3, column 16: Unsupported value for `publish_to`. +line 3, column 16: Unsupported value for "publish_to". ╷ 3 │ "publish_to": 42 │ ^^ ╵''', '##not a uri!': r''' -line 3, column 16: must be an http or https URL. +line 3, column 16: Unsupported value for "publish_to". Must be an http or https URL. ╷ 3 │ "publish_to": "##not a uri!" │ ^^^^^^^^^^^^^^ ╵''', '/cool/beans': r''' -line 3, column 16: must be an http or https URL. +line 3, column 16: Unsupported value for "publish_to". Must be an http or https URL. ╷ 3 │ "publish_to": "/cool/beans" │ ^^^^^^^^^^^^^ ╵''', 'file:///Users/kevmoo/': r''' -line 3, column 16: must be an http or https URL. +line 3, column 16: Unsupported value for "publish_to". Must be an http or https URL. ╷ 3 │ "publish_to": "file:///Users/kevmoo/" │ ^^^^^^^^^^^^^^^^^^^^^^^ - ╵''' + ╵''', }.entries) { test('cannot be `${entry.key}`', () { expectParseThrows( @@ -182,14 +182,30 @@ group('invalid', () { test('null', () { - expect(() => parse(null), throwsArgumentError); + expectParseThrows( + null, + r''' +line 1, column 1: Not a map + ╷ +1 │ null + │ ^^^^ + ╵''', + ); }); test('empty string', () { - expect(() => parse(''), throwsArgumentError); + expectParseThrows( + '', + r''' +line 1, column 1: Not a map + ╷ +1 │ "" + │ ^^ + ╵''', + ); }); test('array', () { expectParseThrows([], r''' -line 1, column 1: Does not represent a YAML map. +line 1, column 1: Not a map ╷ 1 │ [] │ ^^ @@ -210,7 +226,7 @@ 'name': 'sample', 'environment': {'dart': 'cool'} }, r''' -line 4, column 3: Unrecognized keys: [dart]; supported keys: [sdk] +line 4, column 3: Use "sdk" to for Dart SDK constraints. ╷ 4 │ "dart": "cool" │ ^^^^^^ @@ -218,26 +234,32 @@ }); test('environment values cannot be int', () { - expectParseThrows({ - 'name': 'sample', - 'environment': {'sdk': 42} - }, r''' -line 4, column 10: `42` is not a String. + expectParseThrows( + { + 'name': 'sample', + 'environment': {'sdk': 42} + }, + r''' +line 4, column 10: Unsupported value for "sdk". `42` is not a String. ╷ 4 │ "sdk": 42 │ ┌──────────^ 5 │ │ } │ └─^ - ╵'''); + ╵''', + ); }); test('version', () { - expectParseThrows({'name': 'sample', 'version': 'invalid'}, r''' -line 3, column 13: Could not parse "invalid". + expectParseThrows( + {'name': 'sample', 'version': 'invalid'}, + r''' +line 3, column 13: Unsupported value for "version". Could not parse "invalid". ╷ 3 │ "version": "invalid" │ ^^^^^^^^^ - ╵'''); + ╵''', + ); }); test('invalid environment value', () { @@ -245,7 +267,7 @@ 'name': 'sample', 'environment': {'sdk': 'silly'} }, r''' -line 4, column 10: Could not parse version "silly". Unknown text at "silly". +line 4, column 10: Unsupported value for "sdk". Could not parse version "silly". Unknown text at "silly". ╷ 4 │ "sdk": "silly" │ ^^^^^^^ @@ -253,45 +275,80 @@ }); test('bad repository url', () { - expectParseThrows({ - 'name': 'foo', - 'repository': {'x': 'y'}, - }, r''' -line 3, column 16: Unsupported value for `repository`. + expectParseThrows( + { + 'name': 'foo', + 'repository': {'x': 'y'}, + }, + r''' +line 3, column 16: Unsupported value for "repository". ╷ 3 │ "repository": { │ ┌────────────────^ 4 │ │ "x": "y" 5 │ └ } - ╵''', skipTryPub: true); + ╵''', + skipTryPub: true, + ); }); test('bad issue_tracker url', () { - expectParseThrows({ - 'name': 'foo', - 'issue_tracker': {'x': 'y'}, - }, r''' -line 3, column 19: Unsupported value for `issue_tracker`. + expectParseThrows( + { + 'name': 'foo', + 'issue_tracker': {'x': 'y'}, + }, + r''' +line 3, column 19: Unsupported value for "issue_tracker". ╷ 3 │ "issue_tracker": { │ ┌───────────────────^ 4 │ │ "x": "y" 5 │ └ } - ╵''', skipTryPub: true); + ╵''', + skipTryPub: true, + ); }); }); group('lenient', () { test('null', () { - expect(() => parse(null, lenient: true), throwsArgumentError); + expectParseThrows( + null, + r''' +line 1, column 1: Not a map + ╷ +1 │ null + │ ^^^^ + ╵''', + lenient: true, + ); }); test('empty string', () { - expect(() => parse('', lenient: true), throwsArgumentError); + expectParseThrows( + '', + r''' +line 1, column 1: Not a map + ╷ +1 │ "" + │ ^^ + ╵''', + lenient: true, + ); }); test('name cannot be empty', () { - expect(() => parse({}, lenient: true), throwsException); + expectParseThrows( + {}, + r''' +line 1, column 1: "name" cannot be empty. + ╷ +1 │ {} + │ ^^ + ╵''', + lenient: true, + ); }); test('bad repository url', () {
diff --git a/pkgs/pubspec_parse/test/test_utils.dart b/pkgs/pubspec_parse/test/test_utils.dart index 7f8ae87..b61cdbd 100644 --- a/pkgs/pubspec_parse/test/test_utils.dart +++ b/pkgs/pubspec_parse/test/test_utils.dart
@@ -5,6 +5,7 @@ import 'dart:cli'; import 'dart:convert'; +import 'package:checked_yaml/checked_yaml.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:stack_trace/stack_trace.dart'; @@ -25,10 +26,11 @@ void _printDebugParsedYamlException(ParsedYamlException e) { var innerError = e.innerError; - var innerStack = e.innerStack; + StackTrace innerStack; - if (e.innerError is CheckedFromJsonException) { - final cfje = e.innerError as CheckedFromJsonException; + if (innerError is CheckedFromJsonException) { + final cfje = innerError as CheckedFromJsonException; + if (cfje.innerError != null) { innerError = cfje.innerError; innerStack = cfje.innerStack; @@ -98,6 +100,14 @@ Object content, String expectedError, { bool skipTryPub = false, + bool lenient = false, }) => - expect(() => parse(content, quietOnError: true, skipTryPub: skipTryPub), - _throwsParsedYamlException(expectedError)); + expect( + () => parse( + content, + lenient: lenient, + quietOnError: true, + skipTryPub: skipTryPub, + ), + _throwsParsedYamlException(expectedError), + );