Support package-scoped experiments (<package>.<experiment>)
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index a196be2..348e47b 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -18,6 +18,7 @@ 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'; @@ -500,20 +501,40 @@ 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 = await cache.describe(id); + final pubspec = lockedPubspecs[name]!; entries.add( PackageConfigEntry( name: name, rootUri: p.toUri(rootPath), packageUri: p.toUri('lib/'), languageVersion: pubspec.languageVersion, - experiments: _experimentsNeedingOptIn(pubspec), + experiments: _experimentsNeedingOptIn(pubspec, resolvedExperiments), ), ); } @@ -534,7 +555,10 @@ ), packageUri: p.toUri('lib/'), languageVersion: package.pubspec.languageVersion, - experiments: _experimentsNeedingOptIn(package.pubspec), + experiments: _experimentsNeedingOptIn( + package.pubspec, + resolvedExperiments, + ), ), ); } @@ -565,9 +589,13 @@ /// /// Experiments that don't require opting in (or can't be disabled) are left /// out. - static List<String> _experimentsNeedingOptIn(Pubspec pubspec) => [ + static List<String> _experimentsNeedingOptIn( + Pubspec pubspec, + Map<String, Experiment> resolvedExperiments, + ) => [ for (final name in pubspec.experiments) - if (isEffectiveExperimentFlag(name)) name, + if (isEffectiveExperimentFlag(name, experiments: resolvedExperiments)) + name, ]; /// Gets all dependencies of the [workspaceRoot] package. @@ -647,6 +675,8 @@ ); } + 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( @@ -667,6 +697,7 @@ 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 32c2e42..8ea5c19 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart
@@ -8,6 +8,7 @@ import 'package:yaml/yaml.dart'; import 'exceptions.dart'; +import 'experiment.dart'; import 'io.dart'; import 'language_version.dart'; import 'package.dart'; @@ -19,9 +20,13 @@ 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. @@ -145,26 +150,72 @@ ); 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 => _experiments ??= parseExperiments(); + List<String> get experiments { + if (_experiments == null) _parseExperimentsField(); + return _experiments!; + } - List<String> 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() { final experimentsNode = fields.nodes['experiments']; if (experimentsNode == null || experimentsNode.value == null) { - return []; + _experiments = const []; + _declaredExperiments = const {}; + return; } - if (experimentsNode is! YamlList) { - _error('`experiments` must be a list of strings', experimentsNode.span); + if (experimentsNode is! YamlMap) { + _error( + '`experiments` must be a mapping with `enable` and/or `declare` keys.', + 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 experimentsNode.nodes) { + for (final e in listNode.nodes) { final value = e.value; if (value is! String) { - _error('`experiments` must be a list of strings', e.span); + _error('`experiments.enable` must be a list of strings', e.span); } if (result.contains(value)) { _error('The experiment `$value` is listed more than once.', e.span); @@ -179,11 +230,24 @@ } // For root packages, validate that all experiments are known by at - // least one of the current SDKs and available on this channel. + // least one of the current SDKs and available on this channel, or are + // package-scoped experiments (`<package>.<experiment>`). // // Dependencies will only be chosen by the solver if their experiments // are allowed by the workspace, so we don't validate them here. - if (_containingDescription is ResolvedRootDescription) { + 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) { final experiment = availableExperiments[name]; if (experiment == null) { final availableExperimentsDescription = @@ -214,6 +278,104 @@ 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. @@ -599,6 +761,7 @@ () => dependencies, () => executables, () => ignoredAdvisories, + () => declaredExperiments, ]); /// Returns a list of most errors in this pubspec. @@ -615,6 +778,7 @@ () => 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 a8fedcc..0fc024d 100644 --- a/lib/src/sdk.dart +++ b/lib/src/sdk.dart
@@ -137,12 +137,16 @@ /// Whether [flag] (an `<experiment>` or `no-<experiment>` entry from /// `experiments`) has an effect and should be passed on to tools. -bool isEffectiveExperimentFlag(String flag) { +bool isEffectiveExperimentFlag( + String flag, { + Map<String, Experiment>? experiments, +}) { + final knownExperiments = experiments ?? availableExperiments; if (flag.startsWith('no-')) { - final experiment = availableExperiments[flag.substring(3)]; + final experiment = knownExperiments[flag.substring(3)]; return experiment == null || experiment.canBeDisabled; } - final experiment = availableExperiments[flag]; + final experiment = knownExperiments[flag]; return experiment == null || experiment.requiresOptIn; }
diff --git a/lib/src/solver/incompatibility.dart b/lib/src/solver/incompatibility.dart index 372ddd3..3bdff03 100644 --- a/lib/src/solver/incompatibility.dart +++ b/lib/src/solver/incompatibility.dart
@@ -110,6 +110,11 @@ 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 e59cc53..9631b45 100644 --- a/lib/src/solver/incompatibility_cause.dart +++ b/lib/src/solver/incompatibility_cause.dart
@@ -90,7 +90,9 @@ String? get hint { final unknownSdkExperiments = [ for (final experiment in experiments) - if (!availableExperiments.containsKey(experiment)) experiment, + if (!experiment.contains('.') && + !availableExperiments.containsKey(experiment)) + experiment, ]; if (unknownSdkExperiments.isNotEmpty) { final first = unknownSdkExperiments.first; @@ -101,6 +103,7 @@ } for (final experiment in experiments) { + if (experiment.contains('.')) continue; final sdkExperiment = availableExperiments[experiment]; if (sdkExperiment != null && !sdkExperiment.isAvailableOnChannel(DartSdk.channel)) { @@ -130,7 +133,7 @@ ...optedIn, for (final e in experiments) if (!optedIn.contains(e)) e, - ].map((e) => ' - $e').join('\n'); + ].map((e) => ' - $e').join('\n'); return ''' $notEnabledDescription @@ -140,6 +143,7 @@ ``` experiments: + enable: $yamlEntries ```
diff --git a/lib/src/solver/package_lister.dart b/lib/src/solver/package_lister.dart index 4f85599..8b7d972 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 (!allowedExperiments.contains(experiment)) experiment, + if (!await _isExperimentAllowed(pubspec, experiment)) experiment, ]; if (disallowed.isNotEmpty) { return [ @@ -304,7 +304,14 @@ ...pubspec.dependencyOverrides.values, ], ]; - return entries.map((range) => _dependency(depender, range)).toList(); + return [ + ...entries.map((range) => _dependency(depender, range)), + if (!id.isRoot) + ...await _ungraduatedTargetIncompatibilities( + pubspec, + lockedDepender: depender, + ), + ]; } final versions = await _versions; @@ -342,23 +349,26 @@ 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]!, - ); - }).toList(); + return _dependency( + _ref.withConstraint(constraint), + dependencies[package]!, + ); + }), + ...await _ungraduatedTargetIncompatibilities(pubspec, index: index), + ]; } /// Returns an [Incompatibility] that represents a dependency from [depender] @@ -370,6 +380,122 @@ ], 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. /// @@ -379,7 +505,7 @@ final pubspec = await _describeSafe(versions[index]); final disallowedExperiments = <String>[ for (final e in pubspec.experiments) - if (!allowedExperiments.contains(e)) e, + if (!await _isExperimentAllowed(pubspec, e)) e, ]; if (disallowedExperiments.isEmpty) return null;
diff --git a/lib/src/solver/report.dart b/lib/src/solver/report.dart index cd8a9d0..2e3e4d8 100644 --- a/lib/src/solver/report.dart +++ b/lib/src/solver/report.dart
@@ -6,6 +6,7 @@ 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'; @@ -70,6 +71,9 @@ /// 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, @@ -84,9 +88,11 @@ required bool enforceLockfile, required SolveReportMode reportMode, this.expiredExperimentUsers = const {}, + Map<String, Experiment>? resolvedExperiments, }) : _dryRun = dryRun, _reportMode = reportMode, - _enforceLockfile = enforceLockfile; + _enforceLockfile = enforceLockfile, + resolvedExperiments = resolvedExperiments ?? availableExperiments; /// Displays a report of the results of the version resolution in /// [_newLockFile] relative to the [_previousLockFile] file. @@ -353,14 +359,15 @@ in expiredExperimentUsers.entries) { final isOptOut = rawName.startsWith('no-'); final name = isOptOut ? rawName.substring(3) : rawName; - final enabledIn = availableExperiments[name]?.enabledIn; + final enabledIn = resolvedExperiments[name]?.enabledIn; + final owner = name.contains('.') ? name.split('.').first : 'Dart'; final status = enabledIn == null ? 'has been retired and no longer has any effect' : isOptOut - ? 'has been enabled by default since Dart $enabledIn and can ' + ? 'has been enabled by default since $owner $enabledIn and can ' 'no longer be disabled' - : 'has been enabled by default since Dart $enabledIn'; + : 'has been enabled by default since $owner $enabledIn'; final target = isOptOut ? '`$rawName`' : 'it'; warning( 'The experiment `$name` $status. Remove $target from `experiments` in ' @@ -372,7 +379,7 @@ for (final MapEntry(key: rawName, value: packages) in experimentUsers.entries) { final name = rawName.startsWith('no-') ? rawName.substring(3) : rawName; - final description = availableExperiments[name]?.description; + final description = resolvedExperiments[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 ba8f093..c343cf5 100644 --- a/lib/src/solver/result.dart +++ b/lib/src/solver/result.dart
@@ -7,6 +7,7 @@ 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'; @@ -15,6 +16,7 @@ import '../sdk.dart'; import '../source/cached.dart'; import '../system_cache.dart'; +import '../utils.dart'; /// The result of a successful version resolution. class SolveResult { @@ -55,6 +57,16 @@ /// 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. /// @@ -63,7 +75,10 @@ final result = SplayTreeMap<String, List<String>>(); for (final name in pubspecs.keys.sorted()) { for (final experiment in pubspecs[name]!.experiments) { - if (!isEffectiveExperimentFlag(experiment)) { + if (!isEffectiveExperimentFlag( + experiment, + experiments: resolvedExperiments, + )) { continue; } (result[experiment] ??= []).add(name); @@ -80,7 +95,7 @@ for (final experiment in package.pubspec.experiments) { final experimentName = experiment.startsWith('no-') ? experiment.substring(3) : experiment; - if (availableExperiments[experimentName]?.expired == true) { + if (resolvedExperiments[experimentName]?.expired == true) { (result[experiment] ??= []).add(package.name); } } @@ -88,6 +103,46 @@ 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 ba6fbd3..bfa4736 100644 --- a/lib/src/solver/version_solver.dart +++ b/lib/src/solver/version_solver.dart
@@ -112,6 +112,10 @@ _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 a2a2c08..7060e43 100644 --- a/lib/src/validator/experiments.dart +++ b/lib/src/validator/experiments.dart
@@ -9,7 +9,8 @@ import '../validator.dart'; /// A validator that checks that packages enabling experiments are published as -/// pre-release versions and have a pre-release SDK constraint. +/// pre-release versions (and, when enabling SDK experiments, have a pre-release +/// SDK constraint). class ExperimentsValidator extends Validator { @override Future<void> validate() async { @@ -25,21 +26,26 @@ ); } - 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 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); - 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 00ddb10..be1a0fd 100644 --- a/test/experiments_test.dart +++ b/test/experiments_test.dart
@@ -25,14 +25,18 @@ 'foo', '1.0.0', pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ) .create(); @@ -61,7 +65,9 @@ 'foo', '1.0.0', pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ); await d.dir(appPath, [ @@ -81,7 +87,9 @@ deps: {'foo': '^1.0.0'}, resolutionWorkspace: true, extras: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ), ]), @@ -108,21 +116,27 @@ 'foo', '1.0.0-dev', pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ); server.serve( 'foo', '1.0.1-dev', // This version is newer, but uses a disabled experiment. pubspec: { - 'experiments': ['abcd'], + 'experiments': { + 'enable': ['abcd'], + }, }, ); await d .appDir( dependencies: {'foo': '^1.0.0-dev'}, pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ) .create(); @@ -140,7 +154,9 @@ 'foo', '1.1.0-dev', pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ); await d.appDir(dependencies: {'foo': '^1.0.0-dev'}).create(); @@ -157,7 +173,8 @@ ``` experiments: - - abc + enable: + - abc ``` Read more about experiments at https://dart.dev/go/experiments.''', @@ -174,7 +191,9 @@ 'foo', '1.0.0', extras: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ), ]).create(); @@ -200,7 +219,9 @@ 'foo', '1.0.0', pubspec: { - 'experiments': ['shipped'], + 'experiments': { + 'enable': ['shipped'], + }, }, ); await d.appDir(dependencies: {'foo': '^1.0.0'}).create(); @@ -221,7 +242,9 @@ await d .appDir( pubspec: { - 'experiments': <String>['abcd'], + 'experiments': { + 'enable': <String>['abcd'], + }, }, ) .create(); @@ -247,7 +270,9 @@ await d .appDir( pubspec: { - 'experiments': ['abc', 'abc'], + 'experiments': { + 'enable': ['abc', 'abc'], + }, }, ) .create(); @@ -266,7 +291,9 @@ await d .appDir( pubspec: { - 'experiments': ['main-only'], + 'experiments': { + 'enable': ['main-only'], + }, }, ) .create(); @@ -298,7 +325,9 @@ await d .appDir( pubspec: { - 'experiments': ['dart-feature'], + 'experiments': { + 'enable': ['dart-feature'], + }, }, ) .create(); @@ -350,7 +379,9 @@ await d .appDir( pubspec: { - 'experiments': ['data-assets', 'variance'], + 'experiments': { + 'enable': ['data-assets', 'variance'], + }, }, ) .create(); @@ -389,7 +420,9 @@ await d .appDir( pubspec: { - 'experiments': ['dart-feature'], + 'experiments': { + 'enable': ['dart-feature'], + }, }, ) .create(); @@ -423,14 +456,18 @@ 'foo', '1.0.0', pubspec: { - 'experiments': ['abandoned'], + 'experiments': { + 'enable': ['abandoned'], + }, }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': ['shipped-and-expired'], + 'experiments': { + 'enable': ['shipped-and-expired'], + }, }, ) .create(); @@ -452,7 +489,9 @@ .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': ['abandoned'], + 'experiments': { + 'enable': ['abandoned'], + }, }, ) .create(); @@ -467,7 +506,9 @@ .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': ['no-shipped-and-expired'], + 'experiments': { + 'enable': ['no-shipped-and-expired'], + }, }, ) .create(); @@ -492,14 +533,18 @@ 'foo', '1.0.0', pubspec: { - 'experiments': ['no-shipped'], + 'experiments': { + 'enable': ['no-shipped'], + }, }, ); await d .appDir( dependencies: {'foo': '^1.0.0'}, pubspec: { - 'experiments': ['no-shipped', 'no-abc'], + 'experiments': { + 'enable': ['no-shipped', 'no-abc'], + }, }, ) .create(); @@ -522,7 +567,9 @@ await d .appDir( pubspec: { - 'experiments': ['shipped', 'no-shipped'], + 'experiments': { + 'enable': ['shipped', 'no-shipped'], + }, }, ) .create(); @@ -536,28 +583,15 @@ ); }); - 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': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ); await _setupSdks(); @@ -570,6 +604,457 @@ }); 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 { @@ -587,14 +1072,18 @@ 'foo', '1.0.0-dev', pubspec: { - 'experiments': ['def', 'ghi'], + 'experiments': { + 'enable': ['def', 'ghi'], + }, }, ); await d .appDir( dependencies: {'foo': '^1.0.0-dev'}, pubspec: { - 'experiments': ['abc'], + 'experiments': { + 'enable': ['abc'], + }, }, ) .create(); @@ -610,9 +1099,10 @@ ``` experiments: - - abc - - def - - ghi + enable: + - abc + - def + - ghi ``` Read more about experiments at https://dart.dev/go/experiments.''', @@ -625,7 +1115,9 @@ 'bar', '1.0.0-dev', pubspec: { - 'experiments': ['main-only'], + 'experiments': { + 'enable': ['main-only'], + }, }, ); await d.appDir(dependencies: {'bar': '^1.0.0-dev'}).create(); @@ -645,7 +1137,7 @@ Future<void> createPublishablePackage({ required String version, required String sdkConstraint, - required List<String> experiments, + required Map<String, Object?> experiments, Map<String, Object?> dependencies = const {}, }) async { await d.validPackage(version: version).create(); @@ -670,7 +1162,9 @@ await createPublishablePackage( version: '1.0.0', sdkConstraint: '>=3.1.0-0 <4.0.0', - experiments: ['abc'], + experiments: { + 'enable': ['abc'], + }, ); await runPub( @@ -694,7 +1188,9 @@ await createPublishablePackage( version: '1.0.0-0', sdkConstraint: '^3.1.0', - experiments: ['abc'], + experiments: { + 'enable': ['abc'], + }, ); await runPub( @@ -717,7 +1213,9 @@ await createPublishablePackage( version: '1.0.0-0', sdkConstraint: '>=3.1.0-0 <4.0.0', - experiments: ['abc'], + experiments: { + 'enable': ['abc'], + }, ); await runPub( @@ -726,6 +1224,28 @@ 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, + ); + }, + ); }); }