Skip to content

Commit

Permalink
Migrate user-facing apis to null-safety (#2996)
Browse files Browse the repository at this point in the history
Migrate packages typically imported by build authors to null-safety (part 1 /3). Closes #2920

In this PR, I migrate

- the `build` package
- the `build_test` package (or rather, the parts of that package that don't import `build_resolvers`)
- the `scratch_space` package

After the `graphs` package is migrated, I'll open another PR to migrate the `build_resolvers` package. When that one is done too, we can start to opt the tests of `build` and `build_test` into null-safety and publish, thus enabling build authors to write and test their builders with strong null-safety. 

Migrating the rest of the build system is not a goal for the near future, so we won't run builds with strong null-safety.
  • Loading branch information
simolus3 authored Feb 19, 2021
1 parent 8d9d91d commit cb3fb86
Show file tree
Hide file tree
Showing 61 changed files with 232 additions and 133 deletions.
5 changes: 5 additions & 0 deletions build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 2.0.0-dev

- Migrate to null-safety
- __Breaking__: Remove the deprecated `rootPackage` argument to `runBuilder`

## 1.6.3

- Use latest analyzer version `1.x`.
Expand Down
4 changes: 2 additions & 2 deletions build/lib/src/analyzer/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ abstract class Resolver {
/// **NOTE**: In general, its recommended to use [libraryFor] with an absolute
/// asset id instead of a named identifier that has the possibility of not
/// being unique.
Future<LibraryElement> findLibraryByName(String libraryName);
Future<LibraryElement?> findLibraryByName(String libraryName);

/// Returns the [AssetId] of the Dart library or part declaring [element].
///
Expand Down Expand Up @@ -168,7 +168,7 @@ class SyntaxErrorInAssetException implements Exception {
final error = errorAndResult.key;
// Use a short name: We present the full context by including the asset id
// and this is easier to skim through
final sourceName = error.source?.shortName ?? '<unknown>';
final sourceName = error.source.shortName;

final lineInfo = errorAndResult.value.lineInfo;
final position = lineInfo.getLocation(error.offset);
Expand Down
5 changes: 2 additions & 3 deletions build/lib/src/asset/id.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class AssetId implements Comparable<AssetId> {
///
/// `asset:` uris have the format '$package/$path', including the top level
/// directory.
factory AssetId.resolve(String uri, {AssetId from}) {
factory AssetId.resolve(String uri, {AssetId? from}) {
final parsedUri = Uri.parse(uri);
if (parsedUri.hasScheme) {
if (parsedUri.scheme == 'package') {
Expand Down Expand Up @@ -108,8 +108,7 @@ class AssetId implements Comparable<AssetId> {
/// A `package:` URI suitable for use directly with other systems if this
/// asset is under it's package's `lib/` directory, else an `asset:` URI
/// suitable for use within build tools.
Uri get uri => _uri ??= _constructUri(this);
Uri _uri;
late final Uri uri = _constructUri(this);

/// Deserializes an [AssetId] from [data], which must be the result of
/// calling [serialize] on an existing [AssetId].
Expand Down
4 changes: 2 additions & 2 deletions build/lib/src/asset/reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ abstract class AssetReader {
/// * Throws a `PackageNotFoundException` if `id.package` is not found.
/// * Throws a `AssetNotFoundException` if `id.path` is not found.
/// * Throws an `InvalidInputException` if [id] is an invalid input.
Future<String> readAsString(AssetId id, {Encoding encoding});
Future<String> readAsString(AssetId id, {Encoding encoding = utf8});

/// Indicates whether asset at [id] is readable.
Future<bool> canRead(AssetId id);
Expand Down Expand Up @@ -68,5 +68,5 @@ abstract class MultiPackageAssetReader extends AssetReader {
/// Some implementations may require the [package] argument, while others
/// may have a sane default.
@override
Stream<AssetId> findAssets(Glob glob, {String package});
Stream<AssetId> findAssets(Glob glob, {String? package});
}
23 changes: 14 additions & 9 deletions build/lib/src/builder/build_step_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import 'exceptions.dart';
/// This represents a single input and its expected and real outputs. It also
/// handles tracking of dependencies.
class BuildStepImpl implements BuildStep {
final Resolvers _resolvers;
final Resolvers? _resolvers;
final StageTracker _stageTracker;

/// The primary input id for this build step.
Expand Down Expand Up @@ -55,23 +55,28 @@ class BuildStepImpl implements BuildStep {

bool _isComplete = false;

final void Function(Iterable<AssetId>) _reportUnusedAssets;
final void Function(Iterable<AssetId>)? _reportUnusedAssets;

BuildStepImpl(this.inputId, Iterable<AssetId> expectedOutputs, this._reader,
this._writer, this._resolvers, this._resourceManager,
{StageTracker stageTracker,
void Function(Iterable<AssetId>) reportUnusedAssets})
{StageTracker? stageTracker,
void Function(Iterable<AssetId>)? reportUnusedAssets})
: _expectedOutputs = expectedOutputs.toSet(),
_stageTracker = stageTracker ?? NoOpStageTracker.instance,
_reportUnusedAssets = reportUnusedAssets;

@override
Resolver get resolver {
if (_isComplete) throw BuildStepCompletedException();
return _DelayedResolver(_resolver ??= _resolvers.get(this));
final resolvers = _resolvers;
if (resolvers == null) {
throw UnsupportedError('Resolvers are not available in this build.');
}

return _DelayedResolver(_resolver ??= resolvers.get(this));
}

Future<ReleasableResolver> _resolver;
Future<ReleasableResolver>? _resolver;

@override
Future<bool> canRead(AssetId id) {
Expand Down Expand Up @@ -142,7 +147,7 @@ class BuildStepImpl implements BuildStep {

Future<void> _futureOrWrite<T>(
FutureOr<T> content, Future<void> Function(T content) write) =>
(content is Future<T>) ? content.then(write) : write(content as T);
(content is Future<T>) ? content.then(write) : write(content);

/// Waits for work to finish and cleans up resources.
///
Expand All @@ -165,7 +170,7 @@ class BuildStepImpl implements BuildStep {

@override
void reportUnusedAssets(Iterable<AssetId> assets) {
if (_reportUnusedAssets != null) _reportUnusedAssets(assets);
_reportUnusedAssets?.call(assets);
}
}

Expand Down Expand Up @@ -202,7 +207,7 @@ class _DelayedResolver implements Resolver {
.libraryFor(assetId, allowSyntaxErrors: allowSyntaxErrors);

@override
Future<LibraryElement> findLibraryByName(String libraryName) async =>
Future<LibraryElement?> findLibraryByName(String libraryName) async =>
(await _delegate).findLibraryByName(libraryName);

@override
Expand Down
4 changes: 2 additions & 2 deletions build/lib/src/builder/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class BuilderOptions {
/// Whether or not this builder is running on the root package.
final bool isRoot;

const BuilderOptions(this.config, {bool isRoot}) : isRoot = isRoot ?? false;
const BuilderOptions(this.config, {this.isRoot = false});

/// Returns a new set of options with keys from [other] overriding options in
/// this instance.
Expand All @@ -50,7 +50,7 @@ class BuilderOptions {
/// returned directly.
///
/// The `isRoot` value will also be overridden to value from [other].
BuilderOptions overrideWith(BuilderOptions other) {
BuilderOptions overrideWith(BuilderOptions? other) {
if (other == null) return this;
return BuilderOptions({}..addAll(config)..addAll(other.config),
isRoot: other.isRoot);
Expand Down
2 changes: 1 addition & 1 deletion build/lib/src/builder/exceptions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import '../asset/id.dart';

class UnexpectedOutputException implements Exception {
final AssetId assetId;
final Iterable<AssetId> expected;
final Iterable<AssetId>? expected;

UnexpectedOutputException(this.assetId, {this.expected});

Expand Down
2 changes: 1 addition & 1 deletion build/lib/src/builder/file_deleting_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class FileDeletingBuilder implements PostProcessBuilder {
FileDeletingBuilder.withExcludes(
this.inputExtensions, Iterable<String> exclude,
{this.isEnabled = true})
: exclude = exclude?.map((s) => Glob(s))?.toList() ?? const [];
: exclude = exclude.map((s) => Glob(s)).toList();

@override
FutureOr<Null> build(PostProcessBuildStep buildStep) {
Expand Down
4 changes: 2 additions & 2 deletions build/lib/src/builder/logging.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ final _default = Logger('build.fallback');
/// The log instance for the currently running BuildStep.
///
/// Will be `null` when not running within a build.
Logger get log => Zone.current[logKey] as Logger ?? _default;
Logger get log => Zone.current[logKey] as Logger? ?? _default;

/// Runs [fn] in an error handling [Zone].
///
Expand All @@ -29,7 +29,7 @@ Future<T> scopeLogAsync<T>(Future<T> Function() fn, Logger log) {
done.completeError(e, st);
}, zoneSpecification: ZoneSpecification(print: (self, parent, zone, message) {
log.warning(message);
}), zoneValues: {logKey: log}).then((result) {
}), zoneValues: {logKey: log})?.then((result) {
if (done.isCompleted) return;
done.complete(result);
});
Expand Down
3 changes: 1 addition & 2 deletions build/lib/src/builder/multiplexing_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ Map<String, List<String>> _mergeMaps(Iterable<Map<String, List<String>>> maps) {
var result = <String, List<String>>{};
for (var map in maps) {
for (var key in map.keys) {
result.putIfAbsent(key, () => []);
result[key].addAll(map[key]);
result.putIfAbsent(key, () => []).addAll(map[key]!);
}
}
return result;
Expand Down
2 changes: 1 addition & 1 deletion build/lib/src/builder/post_process_build_step.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,4 @@ class PostProcessBuildStep {

Future<void> _futureOrWrite<T>(
FutureOr<T> content, Future<void> Function(T content) write) =>
(content is Future<T>) ? content.then(write) : write(content as T);
(content is Future<T>) ? content.then(write) : write(content);
2 changes: 1 addition & 1 deletion build/lib/src/experiments.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const Symbol _enabledExperimentsKey = #dartLanguageEnabledExperiments;
///
/// This can be overridden for a new [Zone] by using [withEnabledExperiments].
List<String> get enabledExperiments =>
Zone.current[_enabledExperimentsKey] as List<String> ?? const [];
Zone.current[_enabledExperimentsKey] as List<String>? ?? const [];

/// Runs [fn] in a [Zone], setting [enabledExperiments] for all code running
/// in that [Zone].
Expand Down
2 changes: 1 addition & 1 deletion build/lib/src/generate/expected_outputs.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Iterable<AssetId> expectedOutputs(Builder builder, AssetId input) {
var matchingExtensions =
builder.buildExtensions.keys.where((e) => input.path.endsWith(e));
return matchingExtensions
.expand((e) => _replaceExtensions(input, e, builder.buildExtensions[e]));
.expand((e) => _replaceExtensions(input, e, builder.buildExtensions[e]!));
}

Iterable<AssetId> _replaceExtensions(
Expand Down
17 changes: 8 additions & 9 deletions build/lib/src/generate/run_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,21 @@ import 'expected_outputs.dart';
/// `BuildStep.reportUnusedAssets` in [builder] will be forwarded to this
/// function with the associated primary input.
Future<void> runBuilder(Builder builder, Iterable<AssetId> inputs,
AssetReader reader, AssetWriter writer, Resolvers resolvers,
{Logger logger,
ResourceManager resourceManager,
@Deprecated('The rootPackage argument is unused') String rootPackage,
AssetReader reader, AssetWriter writer, Resolvers? resolvers,
{Logger? logger,
ResourceManager? resourceManager,
StageTracker stageTracker = NoOpStageTracker.instance,
void Function(AssetId input, Iterable<AssetId> assets)
void Function(AssetId input, Iterable<AssetId> assets)?
reportUnusedAssetsForInput}) async {
var shouldDisposeResourceManager = resourceManager == null;
resourceManager ??= ResourceManager();
final resources = resourceManager ?? ResourceManager();
logger ??= Logger('runBuilder');
//TODO(nbosch) check overlapping outputs?
Future<void> buildForInput(AssetId input) async {
var outputs = expectedOutputs(builder, input);
if (outputs.isEmpty) return;
var buildStep = BuildStepImpl(
input, outputs, reader, writer, resolvers, resourceManager,
input, outputs, reader, writer, resolvers, resources,
stageTracker: stageTracker,
reportUnusedAssets: reportUnusedAssetsForInput == null
? null
Expand All @@ -59,7 +58,7 @@ Future<void> runBuilder(Builder builder, Iterable<AssetId> inputs,
await scopeLogAsync(() => Future.wait(inputs.map(buildForInput)), logger);

if (shouldDisposeResourceManager) {
await resourceManager.disposeAll();
await resourceManager.beforeExit();
await resources.disposeAll();
await resources.beforeExit();
}
}
5 changes: 2 additions & 3 deletions build/lib/src/generate/run_post_process_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:logging/logging.dart';
import 'package:meta/meta.dart';

import '../asset/id.dart';
import '../asset/reader.dart';
Expand All @@ -20,8 +19,8 @@ import '../builder/post_process_builder.dart';
/// deleted on disk since the `writer` has no mechanism for delete.
Future<void> runPostProcessBuilder(PostProcessBuilder builder, AssetId inputId,
AssetReader reader, AssetWriter writer, Logger logger,
{@required void Function(AssetId) addAsset,
@required void Function(AssetId) deleteAsset}) async {
{required void Function(AssetId) addAsset,
required void Function(AssetId) deleteAsset}) async {
await scopeLogAsync(() async {
var buildStep =
postProcessBuildStep(inputId, reader, writer, addAsset, deleteAsset);
Expand Down
10 changes: 5 additions & 5 deletions build/lib/src/resource/resource.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ class Resource<T> {

/// Optional method which is given an existing instance that is ready to be
/// disposed.
final DisposeInstance<T> _userDispose;
final DisposeInstance<T>? _userDispose;

/// Optional method which is called before the process is going to exit.
///
/// This allows resources to do any final cleanup, and is not given an
/// instance.
final BeforeExit _userBeforeExit;
final BeforeExit? _userBeforeExit;

/// A Future instance of this resource if one has ever been requested.
final _instanceByManager = <ResourceManager, Future<T>>{};

Resource(this._create, {DisposeInstance<T> dispose, BeforeExit beforeExit})
Resource(this._create, {DisposeInstance<T>? dispose, BeforeExit? beforeExit})
: _userDispose = dispose,
_userBeforeExit = beforeExit;

Expand All @@ -47,7 +47,7 @@ class Resource<T> {
var oldInstance = _fetch(manager);
_instanceByManager.remove(manager);
if (_userDispose != null) {
return oldInstance.then(_userDispose);
return oldInstance.then(_userDispose!);
} else {
return Future.value(null);
}
Expand Down Expand Up @@ -88,7 +88,7 @@ class ResourceManager {
/// Invokes the `beforeExit` callbacks of all [Resource]s that had one.
Future<Null> beforeExit() async {
await Future.wait(_resourcesWithBeforeExit.map((r) async {
if (r._userBeforeExit != null) return r._userBeforeExit();
return r._userBeforeExit?.call();
}));
_resourcesWithBeforeExit.clear();
}
Expand Down
27 changes: 17 additions & 10 deletions build/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
name: build
version: 1.6.3
version: 2.0.0-dev
description: A build system for Dart.
homepage: https://github.com/dart-lang/build/tree/master/build

environment:
sdk: ">=2.11.99 <3.0.0"
sdk: ">=2.12.0-0 <3.0.0"

dependencies:
analyzer: ^1.0.0
async: ">=1.13.3 <3.0.0"
convert: ">=2.0.0 <4.0.0"
crypto: ">=0.9.2 <4.0.0"
async: ^2.0.0
convert: ^3.0.0
crypto: ^3.0.0
glob: ^2.0.0
logging: ">=0.11.2 <2.0.0"
meta: ^1.1.0
path: ^1.1.0
logging: ^1.0.0
meta: ^1.3.0
path: ^1.8.0

dev_dependencies:
build_resolvers: ^1.0.0
build_test: ^1.0.0
build_test: ^1.4.0
pedantic: ^1.0.0
test: ^1.2.0
test: ^1.16.0

dependency_overrides:
build_resolvers:
path: ../build_resolvers
build_test:
path: ../build_test
## Remove each of the above as they are published
1 change: 1 addition & 0 deletions build/test/builder/build_step_impl_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
//@dart=2.9
@TestOn('vm')
import 'dart:async';
import 'dart:convert';
Expand Down
1 change: 1 addition & 0 deletions build/test/builder/multiplexing_builder_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'package:build/build.dart';
import 'package:build_test/build_test.dart';
Expand Down
1 change: 1 addition & 0 deletions build/test/generate/run_builder_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9
@TestOn('vm')
import 'dart:async';

Expand Down
1 change: 1 addition & 0 deletions build/test/generate/run_post_process_builder_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'package:build/build.dart';
import 'package:build_test/build_test.dart';
Expand Down
2 changes: 1 addition & 1 deletion build/test/resource/resource_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:test/test.dart';
import 'package:build/build.dart';

void main() {
ResourceManager resourceManager;
late ResourceManager resourceManager;
setUp(() {
resourceManager = ResourceManager();
});
Expand Down
Loading

0 comments on commit cb3fb86

Please sign in to comment.