Apply patch from other client.

BUG=

Review URL: https://codereview.chromium.org//11235067

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@13984 260f80e4-7a28-3924-810f-c04153c831b5

lib/src/package.dart

@@ -27,12 +27,17 @@ class Package {
       if (!exists) throw new PubspecNotFoundException(name);
       return readTextFile(pubspecPath);
     }).transform((contents) {
-      var pubspec = new Pubspec.parse(contents, sources);
-      if (pubspec.name == null) throw new PubspecHasNoNameException(name);
-      if (name != null && pubspec.name != name) {
-        throw new PubspecNameMismatchException(name, pubspec.name);
+      try {
+        var pubspec = new Pubspec.parse(contents, sources);
+
+        if (pubspec.name == null) throw new PubspecHasNoNameException(name);
+        if (name != null && pubspec.name != name) {
+          throw new PubspecNameMismatchException(name, pubspec.name);
+        }
+        return new Package._(packageDir, pubspec);
+      } on FormatException catch (ex) {
+        throw 'Could not parse $pubspecPath:\n${ex.message}';
       }
-      return new Package._(packageDir, pubspec);
     });
   }
 

lib/src/pubspec.dart

@@ -59,7 +59,13 @@ class Pubspec {
       throw new FormatException('The pubspec must be a YAML mapping.');
     }
 
-    if (parsedPubspec.containsKey('name')) name = parsedPubspec['name'];
+    if (parsedPubspec.containsKey('name')) {
+      name = parsedPubspec['name'];
+      if (name is! String) {
+        throw new FormatException(
+            'The pubspec "name" field should be a string, but was "$name".');
+      }
+    }
 
     if (parsedPubspec.containsKey('version')) {
       version = new Version.parse(parsedPubspec['version']);
@@ -70,7 +76,8 @@ class Pubspec {
       if (dependencyEntries is! Map ||
           dependencyEntries.getKeys().some((e) => e is! String)) {
         throw new FormatException(
-            'The pubspec dependencies must be a map of package names.');
+            'The pubspec dependencies should be a map of package names, but '
+            'was ${dependencyEntries}.');
       }
 
       dependencyEntries.forEach((name, spec) {
@@ -98,14 +105,15 @@ class Pubspec {
           var sourceName = only(sourceNames);
           if (sourceName is! String) {
             throw new FormatException(
-                'Source name $sourceName must be a string.');
+                'Source name $sourceName should be a string.');
           }
 
           source = sources[sourceName];
           description = spec[sourceName];
         } else {
           throw new FormatException(
-              'Dependency specification $spec must be a string or a mapping.');
+              'Dependency specification $spec should be a string or a '
+              'mapping.');
         }
 
         source.validateDescription(description, fromLockFile: false);
@@ -115,6 +123,43 @@ class Pubspec {
       });
     }
 
+    // Even though the pub app itself doesn't use these fields, we validate
+    // them here so that users find errors early before they try to upload to
+    // the server:
+
+    if (parsedPubspec.containsKey('homepage') &&
+        parsedPubspec['homepage'] is! String) {
+      throw new FormatException(
+          'The "homepage" field should be a string, but was '
+          '${parsedPubspec["homepage"]}.');
+    }
+
+    if (parsedPubspec.containsKey('author') &&
+        parsedPubspec['author'] is! String) {
+      throw new FormatException(
+          'The "author" field should be a string, but was '
+          '${parsedPubspec["author"]}.');
+    }
+
+    if (parsedPubspec.containsKey('authors')) {
+      var authors = parsedPubspec['authors'];
+      if (authors is List) {
+        // All of the elements must be strings.
+        if (!authors.every((author) => author is String)) {
+          throw new FormatException('The "authors" field should be a string '
+              'or a list of strings, but was "$authors".');
+        }
+      } else if (authors is! String) {
+        throw new FormatException('The pubspec "authors" field should be a '
+            'string or a list of strings, but was "$authors".');
+      }
+
+      if (parsedPubspec.containsKey('author')) {
+        throw new FormatException('A pubspec should not have both an "author" '
+            'and an "authors" field.');
+      }
+    }
+
     return new Pubspec(name, version, dependencies);
   }
 }

test/pubspec_test.dart

@@ -23,10 +23,10 @@ class MockSource extends Source {
 main() {
   group('Pubspec', () {
     group('parse()', () {
-      test("allows a version constraint for dependencies", () {
-        var sources = new SourceRegistry();
-        sources.register(new MockSource());
+      var sources = new SourceRegistry();
+      sources.register(new MockSource());
 
+      test("allows a version constraint for dependencies", () {
         var pubspec = new Pubspec.parse('''
 dependencies:
   foo:
@@ -42,11 +42,8 @@ dependencies:
       });
 
       test("throws if the description isn't valid", () {
-        var sources = new SourceRegistry();
-        sources.register(new MockSource());
-
         expect(() {
-        new Pubspec.parse('''
+          new Pubspec.parse('''
 dependencies:
   foo:
     mock: bad
@@ -54,10 +51,49 @@ dependencies:
         }, throwsFormatException);
       });
 
-      test("allows comment-only files", () {
-        var sources = new SourceRegistry();
-        sources.register(new MockSource());
+      test("throws if 'name' is not a string", () {
+        expect(() => new Pubspec.parse('name: [not, a, string]', sources),
+            throwsFormatException);
+      });
+
+      test("throws if 'homepage' is not a string", () {
+        expect(() => new Pubspec.parse('homepage: [not, a, string]', sources),
+            throwsFormatException);
+      });
+
+      test("throws if 'authors' is not a string or a list of strings", () {
+        new Pubspec.parse('authors: ok fine', sources);
+        new Pubspec.parse('authors: [also, ok, fine]', sources);
+
+        expect(() => new Pubspec.parse('authors: 123', sources),
+            throwsFormatException);
+
+        expect(() => new Pubspec.parse('authors: {not: {a: string}}', sources),
+            throwsFormatException);
 
+        expect(() => new Pubspec.parse('authors: [ok, {not: ok}]', sources),
+            throwsFormatException);
+      });
+
+      test("throws if 'author' is not a string", () {
+        new Pubspec.parse('author: ok fine', sources);
+
+        expect(() => new Pubspec.parse('author: 123', sources),
+            throwsFormatException);
+
+        expect(() => new Pubspec.parse('author: {not: {a: string}}', sources),
+            throwsFormatException);
+
+        expect(() => new Pubspec.parse('author: [not, ok]', sources),
+            throwsFormatException);
+      });
+
+      test("throws if both 'author' and 'authors' are present", () {
+        expect(() => new Pubspec.parse('{author: abe, authors: ted}', sources),
+            throwsFormatException);
+      });
+
+      test("allows comment-only files", () {
         var pubspec = new Pubspec.parse('''
 # No external dependencies yet
 # Including for completeness