change default sdk constraint to <2.0.0 (#1698)

This only applies to package dependencies, not the root package. This is to make it so the sdk constraint doesn't become a required field for all pubspecs in 2.x sdks.

A future update will make it so publishing a package requires an upper bound sdk constraint.

Author: jakemac53

lib/src/entrypoint.dart

@@ -144,7 +144,8 @@ class Entrypoint {
 
   /// Loads the entrypoint from a package at [rootDir].
   Entrypoint(String rootDir, SystemCache cache, {this.isGlobal: false})
-      : root = new Package.load(null, rootDir, cache.sources),
+      : root =
+            new Package.load(null, rootDir, cache.sources, isRootPackage: true),
         cache = cache,
         _inMemory = false;
 

lib/src/package.dart

@@ -133,9 +133,11 @@ class Package {
   /// [name] is the expected name of that package (e.g. the name given in the
   /// dependency), or `null` if the package being loaded is the entrypoint
   /// package.
-  Package.load(String name, String packageDir, SourceRegistry sources)
+  Package.load(String name, String packageDir, SourceRegistry sources,
+      {bool isRootPackage: false})
       : dir = packageDir,
-        pubspec = new Pubspec.load(packageDir, sources, expectedName: name);
+        pubspec = new Pubspec.load(packageDir, sources,
+            expectedName: name, includeDefaultSdkConstraint: !isRootPackage);
 
   /// Constructs a package with the given pubspec.
   ///

lib/src/pubspec.dart

@@ -26,11 +26,10 @@ import 'utils.dart';
 final _packageName = new RegExp(
     "^${identifierRegExp.pattern}(\\.${identifierRegExp.pattern})*\$");
 
-/// The default SDK constraint for packages that don't declare one.
+/// The default SDK upper bound constraint for packages that don't declare one.
 ///
-/// This allows 2.0.0 dev versions to make the migration proecss smoother.
-final _defaultSdkConstraint =
-    new VersionConstraint.parse("<2.0.0-dev.infinity");
+/// This provides a sane default for packages that don't have an upper bound.
+final _defaultUpperBoundSdkConstraint = new VersionConstraint.parse("<2.0.0");
 
 /// Whether or not `features` are enabled.
 ///
@@ -67,6 +66,10 @@ class Pubspec {
   /// This includes the fields from which other properties are derived.
   final YamlMap fields;
 
+  /// Whether or not to apply the [_defaultUpperBoundsSdkConstraint] to this
+  /// pubspec.
+  bool _includeDefaultSdkConstraint;
+
   /// The package's name.
   String get name {
     if (_name != null) return _name;
@@ -295,8 +298,8 @@ class Pubspec {
     });
   }
 
-  /// The constraint on the Dart SDK, or [_defaultSdkConstraint] if none is
-  /// specified.
+  /// The constraint on the Dart SDK, with [_defaultUpperBoundSdkConstraint] if
+  /// none is specified.
   VersionConstraint get dartSdkConstraint {
     _ensureEnvironment();
     return _dartSdkConstraint;
@@ -327,7 +330,11 @@ class Pubspec {
   Pair<VersionConstraint, VersionConstraint> _parseEnvironment(YamlMap parent) {
     var yaml = parent['environment'];
     if (yaml == null) {
-      return new Pair(_defaultSdkConstraint, null);
+      return new Pair(
+          _includeDefaultSdkConstraint
+              ? _defaultUpperBoundSdkConstraint
+              : VersionConstraint.any,
+          null);
     }
 
     if (yaml is! Map) {
@@ -337,7 +344,9 @@ class Pubspec {
 
     return new Pair(
         _parseVersionConstraint(yaml.nodes['sdk'],
-            defaultConstraint: _defaultSdkConstraint),
+            defaultUpperBoundConstraint: _includeDefaultSdkConstraint
+                ? _defaultUpperBoundSdkConstraint
+                : null),
         yaml.containsKey('flutter')
             ? _parseVersionConstraint(yaml.nodes['flutter'])
             : null);
@@ -485,7 +494,7 @@ class Pubspec {
   /// If [expectedName] is passed and the pubspec doesn't have a matching name
   /// field, this will throw a [PubspecError].
   factory Pubspec.load(String packageDir, SourceRegistry sources,
-      {String expectedName}) {
+      {String expectedName, bool includeDefaultSdkConstraint}) {
     var pubspecPath = path.join(packageDir, 'pubspec.yaml');
     var pubspecUri = path.toUri(pubspecPath);
     if (!fileExists(pubspecPath)) {
@@ -498,7 +507,9 @@ class Pubspec {
     }
 
     return new Pubspec.parse(readTextFile(pubspecPath), sources,
-        expectedName: expectedName, location: pubspecUri);
+        expectedName: expectedName,
+        includeDefaultSdkConstraint: includeDefaultSdkConstraint,
+        location: pubspecUri);
   }
 
   Pubspec(this._name,
@@ -507,6 +518,7 @@ class Pubspec {
       Iterable<PackageRange> devDependencies,
       Iterable<PackageRange> dependencyOverrides,
       VersionConstraint dartSdkConstraint,
+      bool includeDefaultSdkConstraint,
       VersionConstraint flutterSdkConstraint,
       Iterable<Iterable<TransformerConfig>> transformers,
       Map fields,
@@ -517,8 +529,12 @@ class Pubspec {
             devDependencies == null ? null : devDependencies.toList(),
         _dependencyOverrides =
             dependencyOverrides == null ? null : dependencyOverrides.toList(),
-        _dartSdkConstraint = dartSdkConstraint ?? _defaultSdkConstraint,
+        _dartSdkConstraint =
+            dartSdkConstraint ?? includeDefaultSdkConstraint == true
+                ? _defaultUpperBoundSdkConstraint
+                : VersionConstraint.any,
         _flutterSdkConstraint = flutterSdkConstraint,
+        _includeDefaultSdkConstraint = includeDefaultSdkConstraint,
         _transformers = transformers == null
             ? []
             : transformers.map((phase) => phase.toSet()).toList(),
@@ -531,8 +547,9 @@ class Pubspec {
         _version = Version.none,
         _dependencies = <PackageRange>[],
         _devDependencies = <PackageRange>[],
-        _dartSdkConstraint = _defaultSdkConstraint,
+        _dartSdkConstraint = VersionConstraint.any,
         _flutterSdkConstraint = null,
+        _includeDefaultSdkConstraint = false,
         _transformers = <Set<TransformerConfig>>[],
         fields = new YamlMap();
 
@@ -544,10 +561,11 @@ class Pubspec {
   ///
   /// [location] is the location from which this pubspec was loaded.
   Pubspec.fromMap(Map fields, this._sources,
-      {String expectedName, Uri location})
+      {String expectedName, bool includeDefaultSdkConstraint, Uri location})
       : fields = fields is YamlMap
             ? fields
-            : new YamlMap.wrap(fields, sourceUrl: location) {
+            : new YamlMap.wrap(fields, sourceUrl: location),
+        _includeDefaultSdkConstraint = includeDefaultSdkConstraint ?? true {
     // If [expectedName] is passed, ensure that the actual 'name' field exists
     // and matches the expectation.
     if (expectedName == null) return;
@@ -564,7 +582,7 @@ class Pubspec {
   /// If the pubspec doesn't define a version for itself, it defaults to
   /// [Version.none].
   factory Pubspec.parse(String contents, SourceRegistry sources,
-      {String expectedName, Uri location}) {
+      {String expectedName, bool includeDefaultSdkConstraint, Uri location}) {
     YamlNode pubspecNode;
     try {
       pubspecNode = loadYamlNode(contents, sourceUrl: location);
@@ -583,7 +601,9 @@ class Pubspec {
     }
 
     return new Pubspec.fromMap(pubspecMap, sources,
-        expectedName: expectedName, location: location);
+        expectedName: expectedName,
+        includeDefaultSdkConstraint: includeDefaultSdkConstraint,
+        location: location);
   }
 
   /// Returns a list of most errors in this pubspec.
@@ -702,17 +722,31 @@ class Pubspec {
     return dependencies;
   }
 
-  /// Parses [node] to a [VersionConstraint], or [defaultConstraint] if no
-  /// constraint is specified..
+  /// Parses [node] to a [VersionConstraint].
+  ///
+  /// If or [defaultUpperBoundConstraint] is specified then it will be set as
+  /// the max constraint if the original constraint doesn't have an upper
+  /// bound and it is compatible with [defaultUpperBoundConstraint].
   VersionConstraint _parseVersionConstraint(YamlNode node,
-      {VersionConstraint defaultConstraint}) {
-    if (node?.value == null) return defaultConstraint;
+      {VersionConstraint defaultUpperBoundConstraint}) {
+    if (node?.value == null) {
+      return defaultUpperBoundConstraint ?? VersionConstraint.any;
+    }
     if (node.value is! String) {
       _error('A version constraint must be a string.', node.span);
     }
 
-    return _wrapFormatException('version constraint', node.span,
-        () => new VersionConstraint.parse(node.value));
+    return _wrapFormatException('version constraint', node.span, () {
+      var constraint = new VersionConstraint.parse(node.value);
+      if (defaultUpperBoundConstraint != null &&
+          constraint is VersionRange &&
+          constraint.max == null &&
+          defaultUpperBoundConstraint.allowsAny(constraint)) {
+        constraint = new VersionConstraint.intersection(
+            [constraint, defaultUpperBoundConstraint]);
+      }
+      return constraint;
+    });
   }
 
   /// Parses [node] to a map from feature names to whether those features are

test/pubspec_test.dart

@@ -446,7 +446,36 @@ dependencies:
       test("allows an omitted environment", () {
         var pubspec = new Pubspec.parse('', sources);
         expect(pubspec.dartSdkConstraint,
-            equals(new VersionConstraint.parse("<2.0.0-dev.infinity")));
+            equals(new VersionConstraint.parse("<2.0.0")));
+        expect(pubspec.flutterSdkConstraint, isNull);
+      });
+
+      test("default sdk constraint can be ommited with empty environment", () {
+        var pubspec =
+            new Pubspec.parse('', sources, includeDefaultSdkConstraint: false);
+        expect(pubspec.dartSdkConstraint, equals(VersionConstraint.any));
+        expect(pubspec.flutterSdkConstraint, isNull);
+      });
+
+      test("defaults the upper constraint for the sdk", () {
+        var pubspec = new Pubspec.parse('''
+  environment:
+    sdk: ">1.0.0"
+  ''', sources);
+        expect(pubspec.dartSdkConstraint,
+            equals(new VersionConstraint.parse(">1.0.0 <2.0.0")));
+        expect(pubspec.flutterSdkConstraint, isNull);
+      });
+
+      test(
+          "default upper constraint for the sdk applies only if compatibile "
+          "with the lower bound", () {
+        var pubspec = new Pubspec.parse('''
+  environment:
+    sdk: ">3.0.0"
+  ''', sources);
+        expect(pubspec.dartSdkConstraint,
+            equals(new VersionConstraint.parse(">3.0.0")));
         expect(pubspec.flutterSdkConstraint, isNull);
       });
 

test/validator/dependency_test.dart

@@ -488,7 +488,7 @@ main() {
           d.libPubspec("integration_pkg", "1.0.0", deps: {"foo": "^1.2.3"})
         ]).create();
 
-        expectDependencyValidationError('  sdk: ">=1.8.0 <2.0.0-dev.infinity"');
+        expectDependencyValidationError('  sdk: ">=1.8.0 <2.0.0"');
       });
 
       test("with a too-broad SDK constraint", () async {

test/validator/sdk_constraint_test.dart

@@ -78,9 +78,8 @@ main() {
       ]).create();
       expect(
           validatePackage(sdkConstraint),
-          completion(pairOf(
-              anyElement(contains('">=1.19.0 <2.0.0-dev.infinity"')),
-              isEmpty)));
+          completion(
+              pairOf(anyElement(contains('">=1.19.0 <2.0.0"')), isEmpty)));
     });
   });
 }

test/version_solver_test.dart

@@ -1023,23 +1023,60 @@ void dartSdkConstraint() {
     await expectResolves(result: {'foo': '2.0.0', 'bar': '2.0.0'}, tries: 3);
   });
 
-  test('allows 2.0.0-dev by default', () async {
+  test('root package allows 2.0.0-dev by default', () async {
     await d.dir(appPath, [
       await d.pubspec({'name': 'myapp'})
     ]).create();
 
     await expectResolves(
-        environment: {'_PUB_TEST_SDK_VERSION': '2.0.0-dev.99'}, result: {});
+        environment: {'_PUB_TEST_SDK_VERSION': '2.0.0-dev.99'});
   });
 
-  test('disallows 2.0.0 by default', () async {
+  test('root package allows 2.0.0 by default', () async {
     await d.dir(appPath, [
       await d.pubspec({'name': 'myapp'})
     ]).create();
 
+    await expectResolves(environment: {'_PUB_TEST_SDK_VERSION': '2.0.0'});
+  });
+
+  test('package deps disallow 2.0.0-dev by default', () async {
+    await d.dir('foo', [
+      await d.pubspec({'name': 'foo'})
+    ]).create();
+
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'dependencies': {
+          'foo': {'path': '../foo'}
+        }
+      })
+    ]).create();
+
+    await expectResolves(
+        environment: {'_PUB_TEST_SDK_VERSION': '2.0.0-dev.99'},
+        error: 'Package foo requires SDK version <2.0.0 but the '
+            'current SDK is 2.0.0-dev.99.');
+  });
+
+  test('package deps disallow 2.0.0 by default', () async {
+    await d.dir('foo', [
+      await d.pubspec({'name': 'foo'})
+    ]).create();
+
+    await d.dir(appPath, [
+      await d.pubspec({
+        'name': 'myapp',
+        'dependencies': {
+          'foo': {'path': '../foo'}
+        }
+      })
+    ]).create();
+
     await expectResolves(
         environment: {'_PUB_TEST_SDK_VERSION': '2.0.0'},
-        error: 'Package myapp requires SDK version <2.0.0-dev.infinity but the '
+        error: 'Package foo requires SDK version <2.0.0 but the '
             'current SDK is 2.0.0.');
   });
 }
@@ -1168,8 +1205,8 @@ void flutterSdkConstraint() {
 
       await expectResolves(
           environment: {'FLUTTER_ROOT': p.join(d.sandbox, 'flutter')},
-          error: 'Package myapp requires SDK version >0.1.2+3 but the current '
-              'SDK is 0.1.2+3.');
+          error: 'Package myapp requires SDK version >0.1.2+3 but the '
+              'current SDK is 0.1.2+3.');
     });
 
     test('selects the latest dependency with a matching constraint', () async {