From c507e3857b2e1f660e9027cad1dfbdf2785237b3 Mon Sep 17 00:00:00 2001
From: "rnystrom@google.com" <rnystrom@google.com>
Date: Thu, 22 Aug 2013 18:09:25 +0000
Subject: [PATCH] Don't allow packages to depend on themselves.

BUG=https://code.google.com/p/dart/issues/detail?id=12300
R=nweiz@google.com

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

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@26543 260f80e4-7a28-3924-810f-c04153c831b5
---
 lib/src/pubspec.dart                  | 12 ++++++++----
 lib/src/validator/dependency.dart     |  8 --------
 test/pub_install_and_update_test.dart | 25 +++++++++++++++++++++++++
 test/pubspec_test.dart                | 18 ++++++++++++++++++
 test/validator/dependency_test.dart   | 10 ----------
 5 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart
index df7ebd93..56b012b8 100644
--- a/lib/src/pubspec.dart
+++ b/lib/src/pubspec.dart
@@ -134,10 +134,10 @@ Pubspec _parseMap(String filePath, Map map, SourceRegistry sources) {
       'The pubspec "version" field should be a semantic version number, '
       'but was "$v".');
 
-  var dependencies = _parseDependencies(filePath, sources,
+  var dependencies = _parseDependencies(name, filePath, sources,
       map['dependencies']);
 
-  var devDependencies = _parseDependencies(filePath, sources,
+  var devDependencies = _parseDependencies(name, filePath, sources,
       map['dev_dependencies']);
 
   // Make sure the same package doesn't appear as both a regular and dev
@@ -265,8 +265,8 @@ VersionConstraint _parseVersionConstraint(yaml, String getMessage(yaml)) {
   }
 }
 
-List<PackageDep> _parseDependencies(String pubspecPath, SourceRegistry sources,
-    yaml) {
+List<PackageDep> _parseDependencies(String packageName, String pubspecPath,
+    SourceRegistry sources, yaml) {
   var dependencies = <PackageDep>[];
 
   // Allow an empty dependencies key.
@@ -279,6 +279,10 @@ List<PackageDep> _parseDependencies(String pubspecPath, SourceRegistry sources,
   }
 
   yaml.forEach((name, spec) {
+    if (name == packageName) {
+      throw new FormatException("Package '$name' cannot depend on itself.");
+    }
+
     var description;
     var sourceName;
 
diff --git a/lib/src/validator/dependency.dart b/lib/src/validator/dependency.dart
index 46e00157..1715240c 100644
--- a/lib/src/validator/dependency.dart
+++ b/lib/src/validator/dependency.dart
@@ -22,14 +22,6 @@ class DependencyValidator extends Validator {
         return _warnAboutSource(dependency);
       }
 
-      if (dependency.name == entrypoint.root.name) {
-        warnings.add('You don\'t need to explicitly depend on your own '
-                'package.\n'
-            'Pub enables "package:${entrypoint.root.name}" imports '
-                'implicitly.');
-        return new Future.value();
-      }
-
       if (dependency.constraint.isAny) _warnAboutConstraint(dependency);
 
       return new Future.value();
diff --git a/test/pub_install_and_update_test.dart b/test/pub_install_and_update_test.dart
index f269126d..06da50a8 100644
--- a/test/pub_install_and_update_test.dart
+++ b/test/pub_install_and_update_test.dart
@@ -113,5 +113,30 @@ main() {
       pubCommand(command,
           error: new RegExp("^Incompatible dependencies on 'baz':\n"));
     });
+
+    integration('does not allow a dependency on itself', () {
+      d.dir(appPath, [
+        d.appPubspec({
+          "myapp": {"path": "."}
+        })
+      ]).create();
+
+      pubCommand(command,
+          error: new RegExp("Package 'myapp' cannot depend on itself."));
+    });
+
+    integration('does not allow a dev dependency on itself', () {
+      d.dir(appPath, [
+        d.pubspec({
+          "name": "myapp",
+          "dev_dependencies": {
+            "myapp": {"path": "."}
+          }
+        })
+      ]).create();
+
+      pubCommand(command,
+          error: new RegExp("Package 'myapp' cannot depend on itself."));
+    });
   });
 }
diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart
index e7608b4c..092611d3 100644
--- a/test/pubspec_test.dart
+++ b/test/pubspec_test.dart
@@ -103,6 +103,24 @@ dev_dependencies:
 ''');
     });
 
+    test("throws if it dependes on itself", () {
+      expectFormatError('''
+name: myapp
+dependencies:
+  myapp:
+    mock: ok
+''');
+    });
+
+    test("throws if it has a dev dependency on itself", () {
+      expectFormatError('''
+name: myapp
+dev_dependencies:
+  myapp:
+    mock: ok
+''');
+    });
+
     test("throws if the description isn't valid", () {
       expectFormatError('''
 dependencies:
diff --git a/test/validator/dependency_test.dart b/test/validator/dependency_test.dart
index ee38ee8b..ba29373a 100644
--- a/test/validator/dependency_test.dart
+++ b/test/validator/dependency_test.dart
@@ -237,15 +237,5 @@ main() {
         });
       });
     });
-
-    integration('has a hosted dependency on itself', () {
-      d.dir(appPath, [
-        d.libPubspec("test_pkg", "1.0.0", deps: {
-          "test_pkg": ">=1.0.0"
-        })
-      ]).create();
-
-      expectValidationWarning(dependency);
-    });
   });
 }
-- 
GitLab