Scope PR to SDK experiments (move <package>.<experiment> to stacked PR)
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 348e47b..a196be2 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -18,7 +18,6 @@ import 'dart.dart' as dart; import 'exceptions.dart'; import 'executable.dart'; -import 'experiment.dart'; import 'io.dart'; import 'language_version.dart'; import 'lock_file.dart'; @@ -501,40 +500,20 @@ SystemCache cache, { VersionConstraint? entrypointSdkConstraint, }) async { - final lockedPubspecs = <String, Pubspec>{}; - if (lockFile.packages.isNotEmpty) { - for (final name in lockFile.packages.keys.sorted()) { - final id = lockFile.packages[name]!; - lockedPubspecs[name] = await cache.describe(id); - } - } - final resolvedExperiments = <String, Experiment>{ - ...availableExperiments, - for (final pubspec in lockedPubspecs.values) - for (final MapEntry(:key, :value) - in pubspec.declaredExperiments.entries) - '${pubspec.name}.$key': value, - if (!isCachedGlobal) - for (final package in workspaceRoot.transitiveWorkspace) - for (final MapEntry(:key, :value) - in package.pubspec.declaredExperiments.entries) - '${package.name}.$key': value, - }; - final entries = <PackageConfigEntry>[]; if (lockFile.packages.isNotEmpty) { final relativeFromPath = p.join(workspaceRoot.dir, '.dart_tool'); for (final name in lockFile.packages.keys.sorted()) { final id = lockFile.packages[name]!; final rootPath = cache.getDirectory(id, relativeFrom: relativeFromPath); - final pubspec = lockedPubspecs[name]!; + final pubspec = await cache.describe(id); entries.add( PackageConfigEntry( name: name, rootUri: p.toUri(rootPath), packageUri: p.toUri('lib/'), languageVersion: pubspec.languageVersion, - experiments: _experimentsNeedingOptIn(pubspec, resolvedExperiments), + experiments: _experimentsNeedingOptIn(pubspec), ), ); } @@ -555,10 +534,7 @@ ), packageUri: p.toUri('lib/'), languageVersion: package.pubspec.languageVersion, - experiments: _experimentsNeedingOptIn( - package.pubspec, - resolvedExperiments, - ), + experiments: _experimentsNeedingOptIn(package.pubspec), ), ); } @@ -589,13 +565,9 @@ /// /// Experiments that don't require opting in (or can't be disabled) are left /// out. - static List<String> _experimentsNeedingOptIn( - Pubspec pubspec, - Map<String, Experiment> resolvedExperiments, - ) => [ + static List<String> _experimentsNeedingOptIn(Pubspec pubspec) => [ for (final name in pubspec.experiments) - if (isEffectiveExperimentFlag(name, experiments: resolvedExperiments)) - name, + if (isEffectiveExperimentFlag(name)) name, ]; /// Gets all dependencies of the [workspaceRoot] package. @@ -675,8 +647,6 @@ ); } - result.validatePackageExperiments(); - // We have to download files also with --dry-run to ensure we know the // archive hashes for downloaded files. final newLockFile = await result.downloadCachedPackages( @@ -697,7 +667,6 @@ enforceLockfile: enforceLockfile, reportMode: reportMode, expiredExperimentUsers: result.expiredExperimentUsers, - resolvedExperiments: result.resolvedExperiments, ); await report.show(summary: true);
diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index 8ea5c19..32c2e42 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart
@@ -8,7 +8,6 @@ import 'package:yaml/yaml.dart'; import 'exceptions.dart'; -import 'experiment.dart'; import 'io.dart'; import 'language_version.dart'; import 'package.dart'; @@ -20,13 +19,9 @@ import 'source.dart'; import 'source/root.dart'; import 'system_cache.dart'; -import 'utils.dart' show identifierRegExp; export 'pubspec_parse.dart' hide PubspecBase; -/// Matches the `<experiment>` part of a `<package>.<experiment>` flag. -final _packageExperimentNameRegExp = RegExp(r'^[a-zA-Z0-9_-]+$'); - /// The default SDK upper bound constraint for packages that don't declare one. /// /// This provides a sane default for packages that don't have an upper bound. @@ -150,72 +145,26 @@ ); List<String>? _experiments; - Map<String, Experiment>? _declaredExperiments; /// The experiments this package opts in to via the `experiments` field. /// /// Each package gets its own list in `.dart_tool/package_config.json`, and /// tools enable the experiments only for libraries in that package. - List<String> get experiments { - if (_experiments == null) _parseExperimentsField(); - return _experiments!; - } + List<String> get experiments => _experiments ??= parseExperiments(); - /// The experiments this package declares via `experiments: declare:` in its - /// `pubspec.yaml`, keyed by unqualified experiment name. - Map<String, Experiment> get declaredExperiments { - if (_declaredExperiments == null) _parseExperimentsField(); - return _declaredExperiments!; - } - - void _parseExperimentsField() { + List<String> parseExperiments() { final experimentsNode = fields.nodes['experiments']; if (experimentsNode == null || experimentsNode.value == null) { - _experiments = const []; - _declaredExperiments = const {}; - return; + return []; } - if (experimentsNode is! YamlMap) { - _error( - '`experiments` must be a mapping with `enable` and/or `declare` keys.', - experimentsNode.span, - ); + if (experimentsNode is! YamlList) { + _error('`experiments` must be a list of strings', experimentsNode.span); } - for (final keyNode in experimentsNode.nodes.keys) { - final key = (keyNode as YamlNode).value; - if (key != 'enable' && key != 'declare') { - _error( - '`experiments` mapping may only contain `enable` and `declare` ' - 'keys.', - keyNode.span, - ); - } - } - final enableNode = experimentsNode.nodes['enable']; - if (enableNode == null || enableNode.value == null) { - _experiments = const []; - } else if (enableNode is YamlList) { - _experiments = _parseEnabledExperimentsList(enableNode); - } else { - _error('`experiments.enable` must be a list of strings', enableNode.span); - } - - final declareNode = experimentsNode.nodes['declare']; - if (declareNode == null || declareNode.value == null) { - _declaredExperiments = const {}; - } else if (declareNode is YamlMap) { - _declaredExperiments = _parseDeclaredExperimentsMap(declareNode); - } else { - _error('`experiments.declare` must be a mapping', declareNode.span); - } - } - - List<String> _parseEnabledExperimentsList(YamlList listNode) { final result = <String>[]; - for (final e in listNode.nodes) { + for (final e in experimentsNode.nodes) { final value = e.value; if (value is! String) { - _error('`experiments.enable` must be a list of strings', e.span); + _error('`experiments` must be a list of strings', e.span); } if (result.contains(value)) { _error('The experiment `$value` is listed more than once.', e.span); @@ -230,24 +179,11 @@ } // For root packages, validate that all experiments are known by at - // least one of the current SDKs and available on this channel, or are - // package-scoped experiments (`<package>.<experiment>`). + // least one of the current SDKs and available on this channel. // // Dependencies will only be chosen by the solver if their experiments // are allowed by the workspace, so we don't validate them here. - if (value.contains('.')) { - final parts = value.split('.'); - if (isOptOut || - parts.length != 2 || - !identifierRegExp.hasMatch(parts[0]) || - !_packageExperimentNameRegExp.hasMatch(parts[1])) { - _error( - 'Package experiment `$value` must have the form ' - '`<package>.<experiment>`.', - e.span, - ); - } - } else if (_containingDescription is ResolvedRootDescription) { + if (_containingDescription is ResolvedRootDescription) { final experiment = availableExperiments[name]; if (experiment == null) { final availableExperimentsDescription = @@ -278,104 +214,6 @@ return result; } - Map<String, Experiment> _parseDeclaredExperimentsMap(YamlMap declareNode) { - final result = <String, Experiment>{}; - final pkgName = _packageName; - declareNode.nodes.forEach((keyNode, specNode) { - final expNameNode = keyNode as YamlNode; - final expName = expNameNode.value; - if (expName is! String || - expName.startsWith('no-') || - !_packageExperimentNameRegExp.hasMatch(expName)) { - _error( - 'Declared experiment name must be a valid identifier ' - '(matching `^[a-zA-Z0-9_-]+\$` and not starting with `no-`).', - expNameNode.span, - ); - } - if (specNode is! YamlMap) { - _error( - 'Declared experiment `$expName` must be a mapping.', - specNode.span, - ); - } - for (final fieldKeyNode in specNode.nodes.keys) { - final fieldKey = (fieldKeyNode as YamlNode).value; - if (!const { - 'description', - 'docUrl', - 'enabledIn', - 'expired', - }.contains(fieldKey)) { - _error( - 'Unknown field `$fieldKey` in declared experiment `$expName`.', - fieldKeyNode.span, - ); - } - } - final descriptionNode = specNode.nodes['description']; - final description = descriptionNode?.value; - if (description is! String) { - _error( - 'Declared experiment `$expName` must have a string "description".', - descriptionNode?.span ?? specNode.span, - ); - } - final docUrlNode = specNode.nodes['docUrl']; - String? docUrl; - if (docUrlNode != null && docUrlNode.value != null) { - if (docUrlNode.value is! String) { - _error( - '"docUrl" of declared experiment `$expName` must be a string.', - docUrlNode.span, - ); - } - docUrl = docUrlNode.value as String; - } - final enabledInNode = specNode.nodes['enabledIn']; - Version? enabledIn; - if (enabledInNode != null && enabledInNode.value != null) { - final rawVersion = enabledInNode.value; - if (rawVersion is! String) { - _error( - '"enabledIn" of declared experiment `$expName` must be a version ' - 'string.', - enabledInNode.span, - ); - } - try { - enabledIn = Version.parse(rawVersion); - } on FormatException catch (e) { - _error( - 'Invalid "enabledIn" version in declared experiment `$expName`: ' - '${e.message}', - enabledInNode.span, - ); - } - } - final expiredNode = specNode.nodes['expired']; - var expired = enabledIn != null; - if (expiredNode != null && expiredNode.value != null) { - if (expiredNode.value is! bool) { - _error( - '"expired" of declared experiment `$expName` must be a boolean.', - expiredNode.span, - ); - } - expired = expiredNode.value as bool; - } - final qualifiedName = pkgName == null ? expName : '$pkgName.$expName'; - result[expName] = Experiment( - qualifiedName, - description, - docUrl: docUrl, - enabledIn: enabledIn, - expired: expired, - ); - }); - return UnmodifiableMapView(result); - } - Map<String, PackageRange>? _dependencies; /// The packages this package depends on when it is the root package. @@ -761,7 +599,6 @@ () => dependencies, () => executables, () => ignoredAdvisories, - () => declaredExperiments, ]); /// Returns a list of most errors in this pubspec. @@ -778,7 +615,6 @@ () => sdkConstraints, () => ignoredAdvisories, () => experiments, - () => declaredExperiments, ]); /// Returns the type of dependency from this package onto [name].
diff --git a/lib/src/sdk.dart b/lib/src/sdk.dart index 0fc024d..a8fedcc 100644 --- a/lib/src/sdk.dart +++ b/lib/src/sdk.dart
@@ -137,16 +137,12 @@ /// Whether [flag] (an `<experiment>` or `no-<experiment>` entry from /// `experiments`) has an effect and should be passed on to tools. -bool isEffectiveExperimentFlag( - String flag, { - Map<String, Experiment>? experiments, -}) { - final knownExperiments = experiments ?? availableExperiments; +bool isEffectiveExperimentFlag(String flag) { if (flag.startsWith('no-')) { - final experiment = knownExperiments[flag.substring(3)]; + final experiment = availableExperiments[flag.substring(3)]; return experiment == null || experiment.canBeDisabled; } - final experiment = knownExperiments[flag]; + final experiment = availableExperiments[flag]; return experiment == null || experiment.requiresOptIn; }
diff --git a/lib/src/solver/incompatibility.dart b/lib/src/solver/incompatibility.dart index 3bdff03..372ddd3 100644 --- a/lib/src/solver/incompatibility.dart +++ b/lib/src/solver/incompatibility.dart
@@ -110,11 +110,6 @@ return '${_terse(depender, details, allowEvery: true)} depends on ' '${_terse(dependee, details)}'; } else if (cause is ExperimentIncompatibilityCause) { - if (terms.length == 2) { - return '${_terse(terms.first, details, allowEvery: true)} with ' - '${_terse(terms.last, details)} requires enabling ' - '${cause.description}'; - } assert(terms.length == 1); final dependee = terms.first; return '${_terse(dependee, details, allowEvery: true)} requires '
diff --git a/lib/src/solver/incompatibility_cause.dart b/lib/src/solver/incompatibility_cause.dart index 9631b45..e59cc53 100644 --- a/lib/src/solver/incompatibility_cause.dart +++ b/lib/src/solver/incompatibility_cause.dart
@@ -90,9 +90,7 @@ String? get hint { final unknownSdkExperiments = [ for (final experiment in experiments) - if (!experiment.contains('.') && - !availableExperiments.containsKey(experiment)) - experiment, + if (!availableExperiments.containsKey(experiment)) experiment, ]; if (unknownSdkExperiments.isNotEmpty) { final first = unknownSdkExperiments.first; @@ -103,7 +101,6 @@ } for (final experiment in experiments) { - if (experiment.contains('.')) continue; final sdkExperiment = availableExperiments[experiment]; if (sdkExperiment != null && !sdkExperiment.isAvailableOnChannel(DartSdk.channel)) { @@ -133,7 +130,7 @@ ...optedIn, for (final e in experiments) if (!optedIn.contains(e)) e, - ].map((e) => ' - $e').join('\n'); + ].map((e) => ' - $e').join('\n'); return ''' $notEnabledDescription @@ -143,7 +140,6 @@ ``` experiments: - enable: $yamlEntries ```
diff --git a/lib/src/solver/package_lister.dart b/lib/src/solver/package_lister.dart index 8b7d972..4f85599 100644 --- a/lib/src/solver/package_lister.dart +++ b/lib/src/solver/package_lister.dart
@@ -260,7 +260,7 @@ if (!id.isRoot) { final disallowed = <String>[ for (final experiment in pubspec.experiments) - if (!await _isExperimentAllowed(pubspec, experiment)) experiment, + if (!allowedExperiments.contains(experiment)) experiment, ]; if (disallowed.isNotEmpty) { return [ @@ -304,14 +304,7 @@ ...pubspec.dependencyOverrides.values, ], ]; - return [ - ...entries.map((range) => _dependency(depender, range)), - if (!id.isRoot) - ...await _ungraduatedTargetIncompatibilities( - pubspec, - lockedDepender: depender, - ), - ]; + return entries.map((range) => _dependency(depender, range)).toList(); } final versions = await _versions; @@ -349,26 +342,23 @@ final lower = await _dependencyBounds(dependencies, index, upper: false); final upper = await _dependencyBounds(dependencies, index); - return [ - ...dependencies.keys.sorted().map((package) { - final constraint = VersionRange( - min: lower[package], - includeMin: true, - max: upper[package], - alwaysIncludeMaxPreRelease: true, - ); + return dependencies.keys.sorted().map((package) { + final constraint = VersionRange( + min: lower[package], + includeMin: true, + max: upper[package], + alwaysIncludeMaxPreRelease: true, + ); - _alreadyListedDependencies[package] = constraint.union( - _alreadyListedDependencies[package] ?? VersionConstraint.empty, - ); + _alreadyListedDependencies[package] = constraint.union( + _alreadyListedDependencies[package] ?? VersionConstraint.empty, + ); - return _dependency( - _ref.withConstraint(constraint), - dependencies[package]!, - ); - }), - ...await _ungraduatedTargetIncompatibilities(pubspec, index: index), - ]; + return _dependency( + _ref.withConstraint(constraint), + dependencies[package]!, + ); + }).toList(); } /// Returns an [Incompatibility] that represents a dependency from [depender] @@ -380,122 +370,6 @@ ], DependencyIncompatibilityCause(depender, target)); } - /// For an experiment `<targetPackage>.<expName>`, returns the version ranges - /// of `<targetPackage>` where `<expName>` still requires opt-in, or `null` if - /// the experiment is disallowed across all versions of `<targetPackage>` - /// allowed by [pubspec]. - /// - /// Returns an empty list if [allowedExperiments] already contains - /// [experiment] or if every version of `<targetPackage>` has - /// graduated/expired `<expName>`. - Future<List<PackageRange>?> _incompatibleTargetRangesForExperiment( - Pubspec pubspec, - String experiment, - ) async { - if (allowedExperiments.contains(experiment)) return const []; - final dotIndex = experiment.indexOf('.'); - if (dotIndex == -1) return null; - final targetPackage = experiment.substring(0, dotIndex); - final expName = experiment.substring(dotIndex + 1); - final depRange = pubspec.dependencies[targetPackage]; - if (depRange == null) return null; - final targetRef = depRange.toRef(); - try { - final versions = - (await _systemCache.getVersions(targetRef)).toList() - ..sort((a, b) => a.version.compareTo(b.version)); - var anyGraduatedInDepRange = false; - final isUngraduated = <bool>[]; - for (final id in versions) { - final targetPubspec = await _systemCache.describe(id); - final declared = targetPubspec.declaredExperiments[expName]; - final graduated = declared != null && !declared.requiresOptIn; - if (graduated && depRange.allows(id)) { - anyGraduatedInDepRange = true; - } - isUngraduated.add(!graduated); - } - if (!anyGraduatedInDepRange) return null; - - final ranges = <PackageRange>[]; - var i = 0; - while (i < versions.length) { - if (!isUngraduated[i]) { - i++; - continue; - } - final first = i; - while (i + 1 < versions.length && isUngraduated[i + 1]) { - i++; - } - final last = i; - final range = VersionRange( - min: first == 0 ? null : versions[first].version, - includeMin: true, - max: last == versions.length - 1 ? null : versions[last + 1].version, - alwaysIncludeMaxPreRelease: true, - ); - ranges.add(targetRef.withConstraint(range)); - i++; - } - return ranges; - } on Exception { - return null; - } - } - - Future<bool> _isExperimentAllowed(Pubspec pubspec, String experiment) async => - await _incompatibleTargetRangesForExperiment(pubspec, experiment) != null; - - Future<List<Incompatibility>> _ungraduatedTargetIncompatibilities( - Pubspec pubspec, { - PackageRange? lockedDepender, - int? index, - }) async { - final result = <Incompatibility>[]; - for (final experiment in pubspec.experiments) { - if (allowedExperiments.contains(experiment)) continue; - final ungraduatedRanges = await _incompatibleTargetRangesForExperiment( - pubspec, - experiment, - ); - if (ungraduatedRanges != null && ungraduatedRanges.isNotEmpty) { - final PackageRange dependerRange; - if (lockedDepender != null) { - dependerRange = lockedDepender; - } else { - final versions = await _versions; - final (boundsFirstIndex, boundsLastIndex) = await _findBounds( - index!, - (p) => p.experiments.contains(experiment), - ); - final dependerVersions = VersionRange( - min: - boundsFirstIndex == 0 - ? null - : versions[boundsFirstIndex].version, - includeMin: true, - max: - boundsLastIndex == versions.length - 1 - ? null - : versions[boundsLastIndex + 1].version, - alwaysIncludeMaxPreRelease: true, - ); - dependerRange = _ref.withConstraint(dependerVersions); - } - for (final badTargetRange in ungraduatedRanges) { - result.add( - Incompatibility( - [Term(dependerRange, true), Term(badTargetRange, true)], - ExperimentIncompatibilityCause([experiment], enabledExperiments), - ), - ); - } - } - } - return result; - } - /// If the version at [index] in [_versions] isn't compatible with the allowed /// experiments, returns an [Incompatibility] indicating this fact. /// @@ -505,7 +379,7 @@ final pubspec = await _describeSafe(versions[index]); final disallowedExperiments = <String>[ for (final e in pubspec.experiments) - if (!await _isExperimentAllowed(pubspec, e)) e, + if (!allowedExperiments.contains(e)) e, ]; if (disallowedExperiments.isEmpty) return null;
diff --git a/lib/src/solver/report.dart b/lib/src/solver/report.dart index 2e3e4d8..cd8a9d0 100644 --- a/lib/src/solver/report.dart +++ b/lib/src/solver/report.dart
@@ -6,7 +6,6 @@ import 'package:pub_semver/pub_semver.dart'; import '../command_runner.dart'; -import '../experiment.dart'; import '../lock_file.dart'; import '../log.dart' as log; import '../package_name.dart'; @@ -71,9 +70,6 @@ /// of those packages. final Map<String, List<String>> expiredExperimentUsers; - /// All experiments known in this resolution (both SDK and package-declared). - final Map<String, Experiment> resolvedExperiments; - SolveReport( this._type, this._location, @@ -88,11 +84,9 @@ required bool enforceLockfile, required SolveReportMode reportMode, this.expiredExperimentUsers = const {}, - Map<String, Experiment>? resolvedExperiments, }) : _dryRun = dryRun, _reportMode = reportMode, - _enforceLockfile = enforceLockfile, - resolvedExperiments = resolvedExperiments ?? availableExperiments; + _enforceLockfile = enforceLockfile; /// Displays a report of the results of the version resolution in /// [_newLockFile] relative to the [_previousLockFile] file. @@ -359,15 +353,14 @@ in expiredExperimentUsers.entries) { final isOptOut = rawName.startsWith('no-'); final name = isOptOut ? rawName.substring(3) : rawName; - final enabledIn = resolvedExperiments[name]?.enabledIn; - final owner = name.contains('.') ? name.split('.').first : 'Dart'; + final enabledIn = availableExperiments[name]?.enabledIn; final status = enabledIn == null ? 'has been retired and no longer has any effect' : isOptOut - ? 'has been enabled by default since $owner $enabledIn and can ' + ? 'has been enabled by default since Dart $enabledIn and can ' 'no longer be disabled' - : 'has been enabled by default since $owner $enabledIn'; + : 'has been enabled by default since Dart $enabledIn'; final target = isOptOut ? '`$rawName`' : 'it'; warning( 'The experiment `$name` $status. Remove $target from `experiments` in ' @@ -379,7 +372,7 @@ for (final MapEntry(key: rawName, value: packages) in experimentUsers.entries) { final name = rawName.startsWith('no-') ? rawName.substring(3) : rawName; - final description = resolvedExperiments[name]?.description; + final description = availableExperiments[name]?.description; message( '* `$rawName` for ${packages.join(', ')}' '${description == null ? '' : ' - $description'}',
diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart index c343cf5..ba8f093 100644 --- a/lib/src/solver/result.dart +++ b/lib/src/solver/result.dart
@@ -7,7 +7,6 @@ import 'package:collection/collection.dart'; import 'package:pub_semver/pub_semver.dart'; -import '../experiment.dart'; import '../lock_file.dart'; import '../log.dart' as log; import '../package.dart'; @@ -16,7 +15,6 @@ import '../sdk.dart'; import '../source/cached.dart'; import '../system_cache.dart'; -import '../utils.dart'; /// The result of a successful version resolution. class SolveResult { @@ -57,16 +55,6 @@ /// That is the union of the experiments listed by the workspace packages. List<String> get experiments => _root.allExperimentsInWorkspace.toList(); - /// All experiments known in this resolution (both SDK experiments and - /// experiments declared by resolved packages), keyed by their qualified - /// name (`<sdk-experiment>` or `<package>.<experiment>`). - late final Map<String, Experiment> resolvedExperiments = { - ...availableExperiments, - for (final pubspec in pubspecs.values) - for (final MapEntry(:key, :value) in pubspec.declaredExperiments.entries) - '${pubspec.name}.$key': value, - }; - /// For each experiment that some package in the solution opts in to or out /// of, the sorted names of those packages. /// @@ -75,10 +63,7 @@ final result = SplayTreeMap<String, List<String>>(); for (final name in pubspecs.keys.sorted()) { for (final experiment in pubspecs[name]!.experiments) { - if (!isEffectiveExperimentFlag( - experiment, - experiments: resolvedExperiments, - )) { + if (!isEffectiveExperimentFlag(experiment)) { continue; } (result[experiment] ??= []).add(name); @@ -95,7 +80,7 @@ for (final experiment in package.pubspec.experiments) { final experimentName = experiment.startsWith('no-') ? experiment.substring(3) : experiment; - if (resolvedExperiments[experimentName]?.expired == true) { + if (availableExperiments[experimentName]?.expired == true) { (result[experiment] ??= []).add(package.name); } } @@ -103,46 +88,6 @@ return result; } - /// Validates that every package-scoped experiment (`<package>.<experiment>`) - /// listed by a workspace package refers to a package in [pubspecs] that - /// declares `<experiment>`. - void validatePackageExperiments() { - for (final package in _root.transitiveWorkspace.sortedBy((p) => p.name)) { - for (final experiment in package.pubspec.experiments) { - final dotIndex = experiment.indexOf('.'); - if (dotIndex == -1) continue; - final targetPackage = experiment.substring(0, dotIndex); - final experimentName = experiment.substring(dotIndex + 1); - final targetPubspec = pubspecs[targetPackage]; - if (targetPubspec == null) { - dataError( - 'The experiment `$experiment` in the pubspec.yaml of ' - '${package.name} refers to package `$targetPackage`, which is not ' - 'in the dependency graph.', - ); - } - final declared = targetPubspec.declaredExperiments[experimentName]; - if (declared == null) { - final available = - targetPubspec.declaredExperiments.values - .where((e) => !e.expired) - .toList(); - final availableDescription = - available.isEmpty - ? 'Package `$targetPackage` (${targetPubspec.version}) does ' - 'not declare any experiments.' - : ''' -Available experiments in `$targetPackage` (${targetPubspec.version}) are: -${available.map((e) => '* ${e.summary}').join('\n')}'''; - dataError(''' -`$experiment` is not a known experiment of package `$targetPackage` (${targetPubspec.version}). - -$availableDescription'''); - } - } - } - } - /// Downloads all the cached packages selected by this version resolution. /// /// If some already cached package differs from what is provided by the server
diff --git a/lib/src/solver/version_solver.dart b/lib/src/solver/version_solver.dart index bfa4736..ba6fbd3 100644 --- a/lib/src/solver/version_solver.dart +++ b/lib/src/solver/version_solver.dart
@@ -112,10 +112,6 @@ _allowedExperiments = { ..._root.allExperimentsInWorkspace, ...experimentsNotRequiringOptIn, - for (final package in _root.transitiveWorkspace) - for (final MapEntry(:key, :value) - in package.pubspec.declaredExperiments.entries) - if (!value.requiresOptIn) '${package.name}.$key', }; /// Prime the solver with [constraints].
diff --git a/lib/src/validator/experiments.dart b/lib/src/validator/experiments.dart index 7060e43..a2a2c08 100644 --- a/lib/src/validator/experiments.dart +++ b/lib/src/validator/experiments.dart
@@ -9,8 +9,7 @@ import '../validator.dart'; /// A validator that checks that packages enabling experiments are published as -/// pre-release versions (and, when enabling SDK experiments, have a pre-release -/// SDK constraint). +/// pre-release versions and have a pre-release SDK constraint. class ExperimentsValidator extends Validator { @override Future<void> validate() async { @@ -26,26 +25,21 @@ ); } - final hasSdkExperiments = enabledExperiments.any((e) => !e.contains('.')); - if (hasSdkExperiments) { - final dartConstraint = - package.pubspec.dartSdkConstraint.originalConstraint; - final dartMin = - dartConstraint is VersionRange ? dartConstraint.min : null; - final flutterConstraint = - package.pubspec.sdkConstraints['flutter']?.originalConstraint; - final flutterMin = - flutterConstraint is VersionRange ? flutterConstraint.min : null; - final hasPreReleaseSdkConstraint = - (dartMin != null && dartMin.isPreRelease) || - (flutterMin != null && flutterMin.isPreRelease); + final dartConstraint = package.pubspec.dartSdkConstraint.originalConstraint; + final dartMin = dartConstraint is VersionRange ? dartConstraint.min : null; + final flutterConstraint = + package.pubspec.sdkConstraints['flutter']?.originalConstraint; + final flutterMin = + flutterConstraint is VersionRange ? flutterConstraint.min : null; + final hasPreReleaseSdkConstraint = + (dartMin != null && dartMin.isPreRelease) || + (flutterMin != null && flutterMin.isPreRelease); - if (!hasPreReleaseSdkConstraint) { - errors.add( - 'Packages that enable SDK experiments must have a pre-release SDK ' - 'constraint (for example, `sdk: ^3.8.0-0`).', - ); - } + if (!hasPreReleaseSdkConstraint) { + errors.add( + 'Packages that enable SDK experiments must have a pre-release SDK ' + 'constraint (for example, `sdk: ^3.8.0-0`).', + ); } } }
diff --git a/test/experiments_test.dart b/test/experiments_test.dart index be1a0fd..00ddb10 100644 --- a/test/experiments_test.dart +++ b/test/experiments_test.dart
@@ -25,18 +25,14 @@ 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ) .create(); @@ -65,9 +61,7 @@ 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ); await d.dir(appPath, [ @@ -87,9 +81,7 @@ deps: {'foo': '^1.0.0'}, resolutionWorkspace: true, extras: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ), ]), @@ -116,27 +108,21 @@ 'foo', '1.0.0-dev', pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ); server.serve( 'foo', '1.0.1-dev', // This version is newer, but uses a disabled experiment. pubspec: { - 'experiments': { - 'enable': ['abcd'], - }, + 'experiments': ['abcd'], }, ); await d .appDir( dependencies: {'foo': '^1.0.0-dev'}, pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ) .create(); @@ -154,9 +140,7 @@ 'foo', '1.1.0-dev', pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ); await d.appDir(dependencies: {'foo': '^1.0.0-dev'}).create(); @@ -173,8 +157,7 @@ ``` experiments: - enable: - - abc + - abc ``` Read more about experiments at https://dart.dev/go/experiments.''', @@ -191,9 +174,7 @@ 'foo', '1.0.0', extras: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ), ]).create(); @@ -219,9 +200,7 @@ 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['shipped'], - }, + 'experiments': ['shipped'], }, ); await d.appDir(dependencies: {'foo': '^1.0.0'}).create(); @@ -242,9 +221,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': <String>['abcd'], - }, + 'experiments': <String>['abcd'], }, ) .create(); @@ -270,9 +247,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['abc', 'abc'], - }, + 'experiments': ['abc', 'abc'], }, ) .create(); @@ -291,9 +266,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['main-only'], - }, + 'experiments': ['main-only'], }, ) .create(); @@ -325,9 +298,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['dart-feature'], - }, + 'experiments': ['dart-feature'], }, ) .create(); @@ -379,9 +350,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['data-assets', 'variance'], - }, + 'experiments': ['data-assets', 'variance'], }, ) .create(); @@ -420,9 +389,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['dart-feature'], - }, + 'experiments': ['dart-feature'], }, ) .create(); @@ -456,18 +423,14 @@ 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['abandoned'], - }, + 'experiments': ['abandoned'], }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': { - 'enable': ['shipped-and-expired'], - }, + 'experiments': ['shipped-and-expired'], }, ) .create(); @@ -489,9 +452,7 @@ .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': { - 'enable': ['abandoned'], - }, + 'experiments': ['abandoned'], }, ) .create(); @@ -506,9 +467,7 @@ .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': { - 'enable': ['no-shipped-and-expired'], - }, + 'experiments': ['no-shipped-and-expired'], }, ) .create(); @@ -533,18 +492,14 @@ 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['no-shipped'], - }, + 'experiments': ['no-shipped'], }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': { - 'enable': ['no-shipped', 'no-abc'], - }, + 'experiments': ['no-shipped', 'no-abc'], }, ) .create(); @@ -567,9 +522,7 @@ await d .appDir( pubspec: { - 'experiments': { - 'enable': ['shipped', 'no-shipped'], - }, + 'experiments': ['shipped', 'no-shipped'], }, ) .create(); @@ -583,15 +536,28 @@ ); }); + test('rejects invalid experiments schema', () async { + await _setupSdks(); + for (final schema in <Object>[ + 123, + [123], + ]) { + await d.appDir(pubspec: {'experiments': schema}).create(); + await pubGet( + error: contains('`experiments` must be a list of strings'), + environment: _environment, + exitCode: DATA, + ); + } + }); + test('Can global activate a package using experiments', () async { final server = await servePackages(); server.serve( 'foo', '1.0.0', pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ); await _setupSdks(); @@ -604,457 +570,6 @@ }); test( - 'supports package-scoped experiments (<package>.<experiment>)', - () async { - final server = await servePackages(); - await _setupSdks(); - server.serve( - 'foo', - '1.0.0', - pubspec: { - 'experiments': { - 'declare': { - 'new_api': { - 'description': 'New experimental API', - 'docUrl': 'https://example.com/new_api', - }, - 'graduated_api': { - 'description': 'Graduated API', - 'enabledIn': '0.9.0', - }, - 'retired_api': {'description': 'Retired API', 'expired': true}, - }, - }, - }, - ); - server.serve( - 'bar', - '1.0.0-dev', - deps: {'foo': '^1.0.0'}, - pubspec: { - 'experiments': { - 'enable': ['foo.new_api', 'foo.graduated_api'], - }, - }, - ); - - // Fails when `bar` requires `foo.new_api` and root does not opt in. - await d.appDir(dependencies: {'bar': '^1.0.0-dev'}).create(); - await pubGet( - error: contains('The experiment `foo.new_api` has not been enabled.'), - environment: _environment, - ); - - // Succeeds when root opts into `foo.new_api` (using map syntax with - // both `enable` and `declare`). Graduated and retired experiments emit - // warnings and are omitted from package_config.json. - await d - .appDir( - dependencies: {'bar': '^1.0.0-dev'}, - pubspec: { - 'experiments': { - 'enable': [ - 'foo.new_api', - 'foo.graduated_api', - 'foo.retired_api', - ], - 'declare': { - 'app_exp': {'description': 'App experiment'}, - }, - }, - }, - ) - .create(); - await pubGet( - output: contains( - '* `foo.new_api` for bar, myapp - New experimental API', - ), - warning: allOf( - contains( - 'The experiment `foo.graduated_api` has been enabled by default ' - 'since foo 0.9.0. Remove it from `experiments` in the pubspec.yaml ' - 'of myapp.', - ), - contains( - 'The experiment `foo.retired_api` has been retired and no longer ' - 'has any effect. Remove it from `experiments` in the pubspec.yaml ' - 'of myapp.', - ), - ), - environment: _environment, - ); - expect(_experimentsByPackage(_readPackageConfig()), { - 'bar': ['foo.new_api'], - 'myapp': ['foo.new_api'], - }); - - // A dependency that only lists graduated/retired package experiments does - // not require the root package to opt in. - server.serve( - 'baz', - '1.0.0', - deps: {'foo': '^1.0.0'}, - pubspec: { - 'experiments': { - 'enable': ['foo.graduated_api', 'foo.retired_api'], - }, - }, - ); - await d.appDir(dependencies: {'baz': '^1.0.0'}).create(); - await pubGet( - output: allOf( - contains('+ baz 1.0.0'), - isNot(contains('Experiments enabled')), - ), - environment: _environment, - ); - expect(_experimentsByPackage(_readPackageConfig()), isEmpty); - }, - ); - - test('selects version where package experiment has graduated, or fails if ' - 'constrained to ungraduated version without opt-in', () async { - final server = await servePackages(); - await _setupSdks(); - server.serve( - 'foo', - '0.8.0', - pubspec: { - 'experiments': { - 'declare': { - 'graduated_api': {'description': 'Experimental in 0.8.0'}, - }, - }, - }, - ); - server.serve( - 'foo', - '1.0.0', - pubspec: { - 'experiments': { - 'declare': { - 'graduated_api': { - 'description': 'Graduated in 0.9.0', - 'enabledIn': '0.9.0', - }, - }, - }, - }, - ); - server.serve( - 'bar', - '1.0.0-dev', - deps: {'foo': '>=0.8.0 <2.0.0'}, - pubspec: { - 'experiments': { - 'enable': ['foo.graduated_api'], - }, - }, - ); - - // When `myapp` pins `foo: 0.8.0` without opting in to - // `foo.graduated_api`, solving fails with the experiment hint. - await d - .appDir(dependencies: {'bar': '^1.0.0-dev', 'foo': '0.8.0'}) - .create(); - await pubGet( - error: contains( - 'The experiment `foo.graduated_api` has not been enabled.', - ), - environment: _environment, - ); - - // When `myapp` allows `foo: ^1.0.0` (where `graduated_api` is graduated), - // solving succeeds without opt-in. - await d - .appDir(dependencies: {'bar': '^1.0.0-dev', 'foo': '>=0.8.0 <2.0.0'}) - .create(); - await pubGet(output: contains('+ foo 1.0.0'), environment: _environment); - expect(_experimentsByPackage(_readPackageConfig()), isEmpty); - }); - - test('supports intra-workspace declared experiments', () async { - await servePackages(); - await _setupSdks(); - await d.dir(appPath, [ - d.libPubspec( - 'myapp', - '1.0.0', - sdk: '^3.5.0', - extras: { - 'workspace': ['pkgs/a', 'pkgs/b'], - }, - ), - d.dir('pkgs', [ - d.dir('a', [ - d.libPubspec( - 'a', - '1.0.0', - resolutionWorkspace: true, - extras: { - 'experiments': { - 'declare': { - 'exp_one': {'description': 'Workspace experiment one'}, - 'exp_graduated': { - 'description': 'Graduated workspace experiment', - 'enabledIn': '1.0.0', - }, - }, - }, - }, - ), - ]), - d.dir('b', [ - d.libPubspec( - 'b', - '1.0.0', - deps: {'a': '^1.0.0'}, - resolutionWorkspace: true, - extras: { - 'experiments': { - 'enable': ['a.exp_one', 'a.exp_graduated'], - }, - }, - ), - ]), - ]), - ]).create(); - - await pubGet( - output: contains('* `a.exp_one` for b - Workspace experiment one'), - warning: contains( - 'The experiment `a.exp_graduated` has been enabled by default since a ' - '1.0.0. Remove it from `experiments` in the pubspec.yaml of b.', - ), - environment: {..._environment, '_PUB_TEST_SDK_VERSION': '3.5.0'}, - ); - expect(_experimentsByPackage(_readPackageConfig()), { - 'b': ['a.exp_one'], - }); - }); - - test( - 'errors when a package-scoped experiment references a missing package or ' - 'undeclared experiment', - () async { - final server = await servePackages(); - await _setupSdks(); - server.serve( - 'foo', - '1.0.0', - pubspec: { - 'experiments': { - 'declare': { - 'new_api': { - 'description': 'New experimental API', - 'docUrl': 'https://example.com/new_api', - }, - }, - }, - }, - ); - server.serve('plain_pkg', '1.0.0'); - - await d - .appDir( - pubspec: { - 'experiments': { - 'enable': ['missing_pkg.new_api'], - }, - }, - ) - .create(); - await pubGet( - error: contains( - 'The experiment `missing_pkg.new_api` in the pubspec.yaml of myapp ' - 'refers to package `missing_pkg`, which is not in the dependency ' - 'graph.', - ), - environment: _environment, - exitCode: DATA, - ); - - await d - .appDir( - dependencies: {'foo': '^1.0.0'}, - pubspec: { - 'experiments': { - 'enable': ['foo.unknown_api'], - }, - }, - ) - .create(); - await pubGet( - error: contains( - '`foo.unknown_api` is not a known experiment of package `foo` ' - '(1.0.0).\n\n' - 'Available experiments in `foo` (1.0.0) are:\n' - '* `foo.new_api`: New experimental API (https://example.com/new_api)', - ), - environment: _environment, - exitCode: DATA, - ); - - await d - .appDir( - dependencies: {'plain_pkg': '^1.0.0'}, - pubspec: { - 'experiments': { - 'enable': ['plain_pkg.some_api'], - }, - }, - ) - .create(); - await pubGet( - error: contains( - '`plain_pkg.some_api` is not a known experiment of package ' - '`plain_pkg` (1.0.0).\n\n' - 'Package `plain_pkg` (1.0.0) does not declare any experiments.', - ), - environment: _environment, - exitCode: DATA, - ); - }, - ); - - test( - 'rejects malformed package-scoped experiment names and declare schema', - () async { - await _setupSdks(); - for (final bad in ['no-foo.bar', 'foo.', '.bar', 'foo.bar.baz']) { - await d - .appDir( - pubspec: { - 'experiments': { - 'enable': [bad], - }, - }, - ) - .create(); - await pubGet( - error: contains( - 'Package experiment `$bad` must have the form ' - '`<package>.<experiment>`.', - ), - environment: _environment, - exitCode: DATA, - ); - } - - for (final badDeclared in ['no-exp', 'foo.bar']) { - await d - .appDir( - pubspec: { - 'experiments': { - 'declare': { - badDeclared: {'description': 'Bad name'}, - }, - }, - }, - ) - .create(); - await pubGet( - error: contains( - 'Declared experiment name must be a valid identifier ' - '(matching `^[a-zA-Z0-9_-]+\$` and not starting with `no-`).', - ), - environment: _environment, - exitCode: DATA, - ); - } - - final invalidSchemas = <(Object, String)>[ - ( - <String>['abc'], - '`experiments` must be a mapping with `enable` and/or `declare` ' - 'keys.', - ), - ( - 123, - '`experiments` must be a mapping with `enable` and/or `declare` ' - 'keys.', - ), - ( - {'unknown': <String>[]}, - '`experiments` mapping may only contain `enable` and `declare` keys.', - ), - ( - {'enable': 'not-a-list'}, - '`experiments.enable` must be a list of strings', - ), - ( - { - 'enable': [123], - }, - '`experiments.enable` must be a list of strings', - ), - ({'declare': <String>[]}, '`experiments.declare` must be a mapping'), - ( - { - 'declare': {'exp': 'not-a-map'}, - }, - 'Declared experiment `exp` must be a mapping.', - ), - ( - { - 'declare': { - 'exp': {'description': 'ok', 'bogus': 1}, - }, - }, - 'Unknown field `bogus` in declared experiment `exp`.', - ), - ( - { - 'declare': {'exp': <String, Object?>{}}, - }, - 'Declared experiment `exp` must have a string "description".', - ), - ( - { - 'declare': { - 'exp': {'description': 'ok', 'docUrl': 123}, - }, - }, - '"docUrl" of declared experiment `exp` must be a string.', - ), - ( - { - 'declare': { - 'exp': {'description': 'ok', 'enabledIn': 123}, - }, - }, - '"enabledIn" of declared experiment `exp` must be a version string.', - ), - ( - { - 'declare': { - 'exp': {'description': 'ok', 'enabledIn': 'not-a-version'}, - }, - }, - 'Invalid "enabledIn" version in declared experiment `exp`:', - ), - ( - { - 'declare': { - 'exp': {'description': 'ok', 'expired': 'not-a-bool'}, - }, - }, - '"expired" of declared experiment `exp` must be a boolean.', - ), - ]; - - for (final (schema, expectedError) in invalidSchemas) { - await d.appDir(pubspec: {'experiments': schema}).create(); - await pubGet( - error: contains(expectedError), - environment: _environment, - exitCode: DATA, - ); - } - }, - ); - - test( 'solver error reporting handles multiple disallowed experiments, ' 'channel-restricted experiments, and already-enabled experiments', () async { @@ -1072,18 +587,14 @@ 'foo', '1.0.0-dev', pubspec: { - 'experiments': { - 'enable': ['def', 'ghi'], - }, + 'experiments': ['def', 'ghi'], }, ); await d .appDir( dependencies: {'foo': '^1.0.0-dev'}, pubspec: { - 'experiments': { - 'enable': ['abc'], - }, + 'experiments': ['abc'], }, ) .create(); @@ -1099,10 +610,9 @@ ``` experiments: - enable: - - abc - - def - - ghi + - abc + - def + - ghi ``` Read more about experiments at https://dart.dev/go/experiments.''', @@ -1115,9 +625,7 @@ 'bar', '1.0.0-dev', pubspec: { - 'experiments': { - 'enable': ['main-only'], - }, + 'experiments': ['main-only'], }, ); await d.appDir(dependencies: {'bar': '^1.0.0-dev'}).create(); @@ -1137,7 +645,7 @@ Future<void> createPublishablePackage({ required String version, required String sdkConstraint, - required Map<String, Object?> experiments, + required List<String> experiments, Map<String, Object?> dependencies = const {}, }) async { await d.validPackage(version: version).create(); @@ -1162,9 +670,7 @@ await createPublishablePackage( version: '1.0.0', sdkConstraint: '>=3.1.0-0 <4.0.0', - experiments: { - 'enable': ['abc'], - }, + experiments: ['abc'], ); await runPub( @@ -1188,9 +694,7 @@ await createPublishablePackage( version: '1.0.0-0', sdkConstraint: '^3.1.0', - experiments: { - 'enable': ['abc'], - }, + experiments: ['abc'], ); await runPub( @@ -1213,9 +717,7 @@ await createPublishablePackage( version: '1.0.0-0', sdkConstraint: '>=3.1.0-0 <4.0.0', - experiments: { - 'enable': ['abc'], - }, + experiments: ['abc'], ); await runPub( @@ -1224,28 +726,6 @@ environment: _environment, ); }); - - test( - 'allows publishing a stable package that only declares experiments', - () async { - await _setupSdks(); - await createPublishablePackage( - version: '1.0.0', - sdkConstraint: '^3.1.0', - experiments: { - 'declare': { - 'new_api': {'description': 'Experimental API'}, - }, - }, - ); - - await runPub( - args: ['publish', '--dry-run'], - output: contains('Package has 0 warnings.'), - environment: _environment, - ); - }, - ); }); }