Include file path in JSON error when possible.

sdk/lib/_internal/pub/lib/src/lock_file.dart

Index: sdk/lib/_internal/pub/lib/src/lock_file.dart
diff --git a/sdk/lib/_internal/pub/lib/src/lock_file.dart b/sdk/lib/_internal/pub/lib/src/lock_file.dart
index 95421a604fc372af3f170a0fc6c2f9142b51db7f..e659657e10605aaf90a2914ff7da8525b49cb092 100644
--- a/sdk/lib/_internal/pub/lib/src/lock_file.dart
+++ b/sdk/lib/_internal/pub/lib/src/lock_file.dart
@@ -4,6 +4,7 @@
 
 library pub.lock_file;
 
+import 'package:source_maps/source_maps.dart';
 import 'package:yaml/yaml.dart';
 
 import 'io.dart';
@@ -48,26 +49,25 @@ class LockFile {
     var packages = <String, PackageId>{};
 
     if (contents.trim() == '') return new LockFile.empty();
-    var parsed = loadYaml(contents);
+    var parsed = loadYamlNode(contents, sourceName: filePath);
+
+    _validate(parsed is Map, 'The lockfile must be a YAML mapping.', parsed);
 
     var packageEntries = parsed['packages'];
     if (packageEntries != null) {

[nweiz, 2014/07/16 18:01:42]

Validate that `packageEntries` is a map.

[Bob Nystrom, 2014/07/16 18:34:44]

Done.

       packageEntries.forEach((name, spec) {
         // Parse the version.
-        if (!spec.containsKey('version')) {
-          throw new FormatException('Package $name is missing a version.');
-        }
+        _validate(spec.containsKey('version'),
+            'Package $name is missing a version.', spec);

[nweiz, 2014/07/16 18:01:42]

These errors originally included the package name as a means of helping the user
figure out where the error was. Since SpanFormatException does that better,
including the name in the error message seems redundant.

[Bob Nystrom, 2014/07/16 18:34:44]

I kind of like leaving in there because it reads well and means the user doesn't
have to hunt the package name down in the path, which may be unfamiliar to them
if it points into the cache.

[nweiz, 2014/07/17 07:11:19]

The span will include the package name. I feel like it will read redundantly,
but DWYW.

[Bob Nystrom, 2014/07/17 22:19:19]

Acknowledged.

         var version = new Version.parse(spec['version']);
 
         // Parse the source.
-        if (!spec.containsKey('source')) {
-          throw new FormatException('Package $name is missing a source.');
-        }
+        _validate(spec.containsKey('source'),
+            'Package $name is missing a source.', spec);
         var sourceName = spec['source'];
 
-        if (!spec.containsKey('description')) {
-          throw new FormatException('Package $name is missing a description.');
-        }
+        _validate(spec.containsKey('description'),
+            'Package $name is missing a description.', spec);
         var description = spec['description'];
 
         // Let the source parse the description.
@@ -78,10 +78,8 @@ class LockFile {
         var id = new PackageId(name, sourceName, version, description);
 
         // Validate the name.
-        if (name != id.name) {
-          throw new FormatException(
-            "Package name $name doesn't match ${id.name}.");
-        }
+        _validate(name == id.name,
+            "Package name $name doesn't match ${id.name}.", parsed);

[nweiz, 2014/07/16 18:01:42]

`parsed` is the entire lockfile; that's not the span we want here. I think this
should be either `spec` or the span for `name`.

[Bob Nystrom, 2014/07/16 18:34:45]

Done.

 
         packages[name] = id;
       });
@@ -90,6 +88,12 @@ class LockFile {
     return new LockFile._(packages);
   }
 
+  /// If [condition] is `false` throws a format error with [message] for [node].
+  static void _validate(bool condition, String message, YamlNode node) {
+    if (condition) return;
+    throw new SpanFormatException(message, node.span);
+  }
+
   /// Returns the serialized YAML text of the lock file.
   ///
   /// [packageDir] is the containing directory of the root package, used to