Migration: set SDK min constraint appropriately.
As discussed in
https://github.com/dart-lang/sdk/issues/44031#issuecomment-720967259,
we want to encourage users to publish null safe packages with an SDK
min constraint matching the SDK version that was used for the
migration, e.g. `2.12.0-18.0.beta`. However, we also want to make
sure that users doing migration via internal, dev, or bleeding edge
builds publish their null safe packages don't wind up inadvertently
publishing packages that are un-resolvable with the latest beta SDK,
so if the user is on one of those versions, we'll set their SDK min
constraint to `2.12.0-0`.
And of course, once the feature ships to stable, we'll want users to
publish their null safe packages with an SDK min constraint matching
the version in which null safety shipped to stable.
This CL implements all of that functionality, and tests it by
overriding the `version` file in the analyzer's mock SDK.
Change-Id: Ib9ed97e691271da0ed391a6c1a5fe209aa959dce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170380
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
index 9c5ebc0..cd29202 100644
--- a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
+++ b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
@@ -1105,18 +1105,21 @@
///
/// [nullSafePackages], if supplied, is a list of packages names that should
/// be included in the null safety allow list.
+ ///
+ /// [sdkVersion], if supplied will override the version stored in the mock
+ /// SDK's `version` file.
MockSdk({
@required this.resourceProvider,
List<MockSdkLibrary> additionalLibraries = const [],
List<String> nullSafePackages = const [],
+ String sdkVersion,
}) {
+ sdkVersion ??= '${ExperimentStatus.currentVersion.major}.'
+ '${ExperimentStatus.currentVersion.minor}.0';
_versionFile = resourceProvider
.getFolder(resourceProvider.convertPath(sdkRoot))
.getChildAssumingFile('version');
- _versionFile.writeAsStringSync(
- '${ExperimentStatus.currentVersion.major}.'
- '${ExperimentStatus.currentVersion.minor}.0',
- );
+ _versionFile.writeAsStringSync(sdkVersion);
for (MockSdkLibrary library in _LIBRARIES) {
var convertedLibrary = library._toProvider(resourceProvider);
diff --git a/pkg/nnbd_migration/lib/migration_cli.dart b/pkg/nnbd_migration/lib/migration_cli.dart
index f6d45aa..2252d76 100644
--- a/pkg/nnbd_migration/lib/migration_cli.dart
+++ b/pkg/nnbd_migration/lib/migration_cli.dart
@@ -692,11 +692,13 @@
Object bindAddress,
{List<String> included = const <String>[],
int preferredPort,
- String summaryPath}) {
+ String summaryPath,
+ @required String sdkPath}) {
return NonNullableFix(listener, resourceProvider, getLineInfo, bindAddress,
included: included,
preferredPort: preferredPort,
- summaryPath: summaryPath);
+ summaryPath: summaryPath,
+ sdkPath: sdkPath);
}
/// Runs the full migration process.
@@ -732,7 +734,8 @@
_fixCodeProcessor.getLineInfo, computeBindAddress(),
included: [options.directory],
preferredPort: options.previewPort,
- summaryPath: options.summary);
+ summaryPath: options.summary,
+ sdkPath: options.sdkPath);
nonNullableFix.rerunFunction = _rerunFunction;
_fixCodeProcessor.registerCodeTask(nonNullableFix);
diff --git a/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart b/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
index 792eeba..6148a95 100644
--- a/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
+++ b/pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart
@@ -4,6 +4,7 @@
import 'dart:convert' show jsonDecode, JsonEncoder;
+import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/file_system/file_system.dart';
@@ -26,20 +27,10 @@
/// and determines whether the associated variable or parameter can be null
/// then adds or removes a '?' trailing the named type as appropriate.
class NonNullableFix {
- // TODO(srawlins): Refactor to use
- // `Feature.non_nullable.releaseVersion` when this becomes non-null (perhaps
- // after "Beta").
- static final Version _intendedMinimumSdkVersion = Version.parse('2.12.0-0');
-
- // In the package_config.json file, the patch number is omitted.
- static final String _intendedLanguageVersion =
- '${_intendedMinimumSdkVersion.major}.${_intendedMinimumSdkVersion.minor}';
-
- static final String _intendedSdkVersionConstraint =
- '>=$_intendedMinimumSdkVersion <3.0.0';
-
static final List<HttpPreviewServer> _allServers = [];
+ final Version _intendedMinimumSdkVersion;
+
/// The internet address the server should bind to. Should be suitable for
/// passing to HttpServer.bind, i.e. either a [String] or an
/// [InternetAddress].
@@ -87,14 +78,26 @@
NonNullableFix(
this.listener, this.resourceProvider, this._getLineInfo, this.bindAddress,
- {List<String> included = const [], this.preferredPort, this.summaryPath})
+ {List<String> included = const [],
+ this.preferredPort,
+ this.summaryPath,
+ @required String sdkPath})
: includedRoot =
- _getIncludedRoot(included, listener.server.resourceProvider) {
+ _getIncludedRoot(included, listener.server.resourceProvider),
+ _intendedMinimumSdkVersion =
+ _computeIntendedMinimumSdkVersion(resourceProvider, sdkPath) {
reset();
}
bool get isPreviewServerRunning => _server != null;
+ /// In the package_config.json file, the patch number is omitted.
+ String get _intendedLanguageVersion =>
+ '${_intendedMinimumSdkVersion.major}.${_intendedMinimumSdkVersion.minor}';
+
+ String get _intendedSdkVersionConstraint =>
+ '>=$_intendedMinimumSdkVersion <3.0.0';
+
InstrumentationListener createInstrumentationListener(
{MigrationSummary migrationSummary}) =>
InstrumentationListener(migrationSummary: migrationSummary);
@@ -371,6 +374,52 @@
_allServers.clear();
}
+ static Version _computeIntendedMinimumSdkVersion(
+ ResourceProvider resourceProvider, String sdkPath) {
+ var versionFile = resourceProvider
+ .getFile(resourceProvider.pathContext.join(sdkPath, 'version'));
+ if (!versionFile.exists) {
+ throw StateError(
+ 'Could not find SDK version file at ${versionFile.path}');
+ }
+ var sdkVersionString = versionFile.readAsStringSync().trim();
+ var sdkVersion = Version.parse(sdkVersionString);
+ // Ideally, we would like to set the user's minimum SDK constraint to the
+ // version in which null safety was released to stable. But we only want to
+ // do so if we are sure that stable release exists. An easy way to check
+ // that is to see if the current SDK version is greater than or equal to the
+ // stable release of null safety.
+ var nullSafetyStableReleaseVersion = Feature.non_nullable.releaseVersion;
+ if (sdkVersion >= nullSafetyStableReleaseVersion) {
+ // It is, so we can use it as the minimum SDK constraint.
+ return nullSafetyStableReleaseVersion;
+ } else {
+ // It isn't. This either means that null safety hasn't been released to
+ // stable yet (in which case it's definitely not safe to use
+ // `nullSafetyStableReleaseVersion` as a minimum SDK constraint), or it
+ // has been released but the user hasn't upgraded to it (in which case we
+ // don't want to use it as a minimum SDK constraint anyway, because we
+ // don't want to force the user to upgrade their SDK in order to be able
+ // to use their own package). Our next best option is to use the user's
+ // current SDK version as a minimum SDK constraint, assuming it's a proper
+ // beta release version.
+ if (sdkVersionString.contains('beta')) {
+ // It is, so we can use it.
+ return sdkVersion;
+ } else {
+ // It isn't. The user is probably either on a bleeding edge version of
+ // the SDK (e.g. `2.12.0-edge.<SHA>`), a dev version
+ // (e.g. `2.12.0-X.Y.dev`), or an internally built version
+ // (e.g. `2.12.0-<large number>`). All of these version numbers are
+ // unsafe for the user to use as their minimum SDK constraint, because
+ // if they published their package, it wouldn't be usable with the
+ // latest beta release. So just fall back on using a version of
+ // `<stable release>-0`.
+ return Version.parse('$nullSafetyStableReleaseVersion-0');
+ }
+ }
+ }
+
/// Get the "root" of all [included] paths. See [includedRoot] for its
/// definition.
static String _getIncludedRoot(
diff --git a/pkg/nnbd_migration/test/migration_cli_test.dart b/pkg/nnbd_migration/test/migration_cli_test.dart
index 31d3cec..72acffb 100644
--- a/pkg/nnbd_migration/test/migration_cli_test.dart
+++ b/pkg/nnbd_migration/test/migration_cli_test.dart
@@ -67,11 +67,13 @@
Object bindAddress,
{List<String> included = const <String>[],
int preferredPort,
- String summaryPath})
+ String summaryPath,
+ @required String sdkPath})
: super(listener, resourceProvider, getLineInfo, bindAddress,
included: included,
preferredPort: preferredPort,
- summaryPath: summaryPath);
+ summaryPath: summaryPath,
+ sdkPath: sdkPath);
@override
InstrumentationListener createInstrumentationListener(
@@ -150,19 +152,22 @@
Object bindAddress,
{List<String> included = const <String>[],
int preferredPort,
- String summaryPath}) {
+ String summaryPath,
+ @required String sdkPath}) {
if (cli._test.injectArtificialException) {
return _ExceptionGeneratingNonNullableFix(
listener, resourceProvider, getLineInfo, bindAddress,
included: included,
preferredPort: preferredPort,
- summaryPath: summaryPath);
+ summaryPath: summaryPath,
+ sdkPath: sdkPath);
} else {
return super.createNonNullableFix(
listener, resourceProvider, getLineInfo, bindAddress,
included: included,
preferredPort: preferredPort,
- summaryPath: summaryPath);
+ summaryPath: summaryPath,
+ sdkPath: sdkPath);
}
}
@@ -432,7 +437,7 @@
'''
name: test
environment:
- sdk: '${migrated ? '>=2.12.0-0 <3.0.0' : '>=2.6.0 <3.0.0'}'
+ sdk: '${migrated ? '>=2.12.0 <3.0.0' : '>=2.6.0 <3.0.0'}'
''',
'.dart_tool/package_config.json':
packageConfigText ?? _getPackageConfigText(migrated: migrated),
@@ -1794,7 +1799,7 @@
name: test
environment:
foo: 1
- sdk: '>=2.12.0-0 <3.0.0'
+ sdk: '>=2.12.0 <3.0.0'
'''));
}
@@ -1851,7 +1856,7 @@
projectDir, simpleProject(migrated: true, pubspecText: '''
name: test
environment:
- sdk: '>=2.12.0-0 <3.0.0'
+ sdk: '>=2.12.0 <3.0.0'
'''));
}
@@ -1868,7 +1873,7 @@
// This is strange-looking, but valid.
'''
environment:
- sdk: '>=2.12.0-0 <3.0.0'
+ sdk: '>=2.12.0 <3.0.0'
name: test
'''));
@@ -1882,6 +1887,58 @@
expect(() async => await cliRunner.run(), throwsUnsupportedError);
}
+ test_pubspec_with_sdk_version_beta() async {
+ var projectDir = createProjectDir(simpleProject());
+ var cliRunner = _createCli(sdkVersion: '2.12.0-1.2.beta')
+ .decodeCommandLineArgs(_parseArgs(['--apply-changes', projectDir]));
+ await cliRunner.run();
+ assertProjectContents(
+ projectDir, simpleProject(migrated: true, pubspecText: '''
+name: test
+environment:
+ sdk: '>=2.12.0-1.2.beta <3.0.0'
+'''));
+ }
+
+ test_pubspec_with_sdk_version_dev() async {
+ var projectDir = createProjectDir(simpleProject());
+ var cliRunner = _createCli(sdkVersion: '2.12.0-1.2.dev')
+ .decodeCommandLineArgs(_parseArgs(['--apply-changes', projectDir]));
+ await cliRunner.run();
+ assertProjectContents(
+ projectDir, simpleProject(migrated: true, pubspecText: '''
+name: test
+environment:
+ sdk: '>=2.12.0-0 <3.0.0'
+'''));
+ }
+
+ test_pubspec_with_sdk_version_edge() async {
+ var projectDir = createProjectDir(simpleProject());
+ var cliRunner = _createCli(sdkVersion: '2.12.0-edge.1234567')
+ .decodeCommandLineArgs(_parseArgs(['--apply-changes', projectDir]));
+ await cliRunner.run();
+ assertProjectContents(
+ projectDir, simpleProject(migrated: true, pubspecText: '''
+name: test
+environment:
+ sdk: '>=2.12.0-0 <3.0.0'
+'''));
+ }
+
+ test_pubspec_with_sdk_version_internal() async {
+ var projectDir = createProjectDir(simpleProject());
+ var cliRunner = _createCli(sdkVersion: '2.12.0-1234567')
+ .decodeCommandLineArgs(_parseArgs(['--apply-changes', projectDir]));
+ await cliRunner.run();
+ assertProjectContents(
+ projectDir, simpleProject(migrated: true, pubspecText: '''
+name: test
+environment:
+ sdk: '>=2.12.0-0 <3.0.0'
+'''));
+ }
+
test_uses_physical_resource_provider_by_default() {
var cli = MigrationCli(binaryName: 'nnbd_migration');
expect(cli.resourceProvider, same(PhysicalResourceProvider.INSTANCE));
@@ -1896,9 +1953,12 @@
headers: {'Content-Type': 'application/json; charset=UTF-8'});
}
- _MigrationCli _createCli({List<String> nullSafePackages = const []}) {
+ _MigrationCli _createCli(
+ {List<String> nullSafePackages = const [], String sdkVersion}) {
mock_sdk.MockSdk(
- resourceProvider: resourceProvider, nullSafePackages: nullSafePackages);
+ resourceProvider: resourceProvider,
+ nullSafePackages: nullSafePackages,
+ sdkVersion: sdkVersion);
return _MigrationCli(this);
}
