Skip to content

Commit

Permalink
Deprecate the unnamed AnalyzerResolvers constructor, replace it with …
Browse files Browse the repository at this point in the history
…a static instance getter and custom factory constructor
  • Loading branch information
jakemac53 committed Sep 20, 2023
1 parent a161297 commit 2acca4c
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 38 deletions.
2 changes: 2 additions & 0 deletions build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## 2.4.2-wip

## 2.4.1

- Allow the latest analyzer (6.x.x).
Expand Down
4 changes: 2 additions & 2 deletions build/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build
version: 2.4.1
version: 2.4.2-wip
description: A package for authoring build_runner compatible code generators.
repository: https://github.com/dart-lang/build/tree/master/build

Expand All @@ -18,7 +18,7 @@ dependencies:
path: ^1.8.0

dev_dependencies:
build_resolvers: ^2.0.0
build_resolvers: ^2.3.3
build_test: ^2.0.0
dart_flutter_team_lints: ^1.0.0
test: ^1.16.0
20 changes: 13 additions & 7 deletions build/test/builder/build_step_impl_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void main() {
primary = makeAssetId();
outputs = List.generate(5, (index) => makeAssetId());
buildStep = BuildStepImpl(primary, outputs, reader, writer,
AnalyzerResolvers(), resourceManager, _unsupported);
AnalyzerResolvers.custom(), resourceManager, _unsupported);
});

test('doesnt allow non-expected outputs', () {
Expand Down Expand Up @@ -93,7 +93,7 @@ void main() {
[outputId],
reader,
writer,
AnalyzerResolvers(),
AnalyzerResolvers.custom(),
resourceManager,
_unsupported,
);
Expand Down Expand Up @@ -121,7 +121,7 @@ void main() {

var primary = makeAssetId('a|web/a.dart');
var buildStep = BuildStepImpl(primary, [], reader, writer,
AnalyzerResolvers(), resourceManager, _unsupported);
AnalyzerResolvers.custom(), resourceManager, _unsupported);
var resolver = buildStep.resolver;

var aLib = await resolver.libraryFor(primary);
Expand Down Expand Up @@ -150,8 +150,14 @@ void main() {
assetWriter = SlowAssetWriter();
outputId = makeAssetId('a|test.txt');
outputContent = '$outputId';
buildStep = BuildStepImpl(primary, [outputId], StubAssetReader(),
assetWriter, AnalyzerResolvers(), resourceManager, _unsupported);
buildStep = BuildStepImpl(
primary,
[outputId],
StubAssetReader(),
assetWriter,
AnalyzerResolvers.custom(),
resourceManager,
_unsupported);
});

test('Completes only after writes finish', () async {
Expand Down Expand Up @@ -199,7 +205,7 @@ void main() {
primary = makeAssetId();
output = makeAssetId();
buildStep = BuildStepImpl(primary, [output], reader, writer,
AnalyzerResolvers(), resourceManager, _unsupported,
AnalyzerResolvers.custom(), resourceManager, _unsupported,
stageTracker: NoOpStageTracker.instance);
});

Expand All @@ -214,7 +220,7 @@ void main() {
var writer = StubAssetWriter();
var unused = <AssetId>{};
var buildStep = BuildStepImpl(makeAssetId(), [], reader, writer,
AnalyzerResolvers(), resourceManager, _unsupported,
AnalyzerResolvers.custom(), resourceManager, _unsupported,
reportUnusedAssets: unused.addAll);
var reported = [
makeAssetId(),
Expand Down
8 changes: 8 additions & 0 deletions build_resolvers/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 2.3.3-wip

- Deprecate the unnamed `AnalyzerResolvers` constructor, an replace it with the
`AnalyzerResolvers.sharedInstance` getter and `AnalyzerResolvers.custom`
constructor. These new apis enforce a 1:1 relationship between the
`BuildAssetUriResolver` instances and `AnalyzerResolvers` instances, which is
necessary to ensure correct builds in the presence of multiple resolvers.

## 2.3.2

- Skip file delete for SDK summary and deps file. This will only impact behavior
Expand Down
14 changes: 10 additions & 4 deletions build_resolvers/lib/src/build_asset_uri_resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,16 @@ class BuildAssetUriResolver extends UriResolver {
/// input, subsequent calls to a resolver, or a transitive import thereof.
final _buildStepTransitivelyResolvedAssets = <BuildStep, HashSet<AssetId>>{};

/// Use the [instance] getter to get an instance.
BuildAssetUriResolver._();
/// Use the [sharedInstance] getter to get the shared instance, which is what
/// real builds should do. There should always be a 1:1 relationship between
/// [BuildAssetUriResolver]s and `AnalyzerResolvers`.
BuildAssetUriResolver();

static final BuildAssetUriResolver instance = BuildAssetUriResolver._();
/// The instance used by the shared `AnalyzerResolvers` instance, which is
/// what should be used by normal builds.
///
/// This is not used within testing contexts or similar custom contexts.
static final BuildAssetUriResolver sharedInstance = BuildAssetUriResolver();

/// Updates [resourceProvider] and the analysis driver given by
/// `withDriverResource` with updated versions of [entryPoints].
Expand Down Expand Up @@ -261,7 +267,7 @@ Set<AssetId> _parseDependencies(String content, AssetId from) => HashSet.of(
/// directives, and cache the results if they weren't already cached.
Future<Iterable<AssetId>?> dependenciesOf(
AssetId id, BuildStep buildStep) async =>
(await BuildAssetUriResolver.instance
(await BuildAssetUriResolver.sharedInstance
._updateCachedAssetState(id, buildStep))
?.dependencies;

Expand Down
60 changes: 48 additions & 12 deletions build_resolvers/lib/src/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ class AnalyzerResolvers implements Resolvers {

// Lazy, all access must be preceded by a call to `_ensureInitialized`.
late final AnalyzerResolver _resolver;
BuildAssetUriResolver? _uriResolver;

final BuildAssetUriResolver _uriResolver;

/// Nullable, should not be accessed outside of [_ensureInitialized].
Future<Result<void>>? _initialized;
Expand All @@ -402,30 +403,65 @@ class AnalyzerResolvers implements Resolvers {
/// **NOTE**: The [_packageConfig] is not used for path resolution, it is
/// primarily used to get the language versions. Any other data (including
/// extra data), may be passed to the analyzer on an as needed basis.
AnalyzerResolvers(
[AnalysisOptions? analysisOptions,
Future<String> Function()? sdkSummaryGenerator,
this._packageConfig])
: _analysisOptions = analysisOptions ??
factory AnalyzerResolvers.custom({
AnalysisOptions? analysisOptions,
Future<String> Function()? sdkSummaryGenerator,
PackageConfig? packageConfig,
}) =>
AnalyzerResolvers._(
analysisOptions: analysisOptions,
sdkSummaryGenerator: sdkSummaryGenerator,
packageConfig: packageConfig,
// Custom resolvers get their own asset uri resolver, as there should
// always be a 1:1 relationship between them.
uriResolver: BuildAssetUriResolver());

/// See [AnalyzerResolvers.custom] for docs.
@Deprecated('Use either the AnalyzerResolvers.custom constructor or the '
'AnalyzerResolvers.sharedInstance static getter to get an instance.')
factory AnalyzerResolvers([
AnalysisOptions? analysisOptions,
Future<String> Function()? sdkSummaryGenerator,
PackageConfig? packageConfig,
]) =>
AnalyzerResolvers._(
analysisOptions: analysisOptions,
sdkSummaryGenerator: sdkSummaryGenerator,
packageConfig: packageConfig,
// For backwards compatibility we use the shared instance here.
uriResolver: BuildAssetUriResolver.sharedInstance);

/// See [AnalyzerResolvers.custom] for docs.
AnalyzerResolvers._({
AnalysisOptions? analysisOptions,
PackageConfig? packageConfig,
Future<String> Function()? sdkSummaryGenerator,
required BuildAssetUriResolver uriResolver,
}) : _analysisOptions = analysisOptions ??
(AnalysisOptionsImpl()
..contextFeatures =
_featureSet(enableExperiments: enabledExperiments)),
_packageConfig = packageConfig,
_sdkSummaryGenerator =
sdkSummaryGenerator ?? defaultSdkSummaryGenerator;
sdkSummaryGenerator ?? defaultSdkSummaryGenerator,
_uriResolver = uriResolver;

/// The instance that most real build systems should use.
static final AnalyzerResolvers sharedInstance =
AnalyzerResolvers._(uriResolver: BuildAssetUriResolver.sharedInstance);

/// Create a Resolvers backed by an `AnalysisContext` using options
/// [_analysisOptions].
Future<void> _ensureInitialized() {
return Result.release(_initialized ??= Result.capture(() async {
_warnOnLanguageVersionMismatch();
final uriResolver = _uriResolver = BuildAssetUriResolver.instance;
final loadedConfig = _packageConfig ??=
await loadPackageConfigUri((await Isolate.packageConfig)!);
var driver = await analysisDriver(uriResolver, _analysisOptions,
var driver = await analysisDriver(_uriResolver, _analysisOptions,
await _sdkSummaryGenerator(), loadedConfig);

_resolver =
AnalyzerResolver(driver, _driverPool, _readAndWritePool, uriResolver);
_resolver = AnalyzerResolver(
driver, _driverPool, _readAndWritePool, _uriResolver);
}()));
}

Expand All @@ -439,7 +475,7 @@ class AnalyzerResolvers implements Resolvers {
/// Must be called between each build.
@override
void reset() {
_uriResolver?.reset();
_uriResolver.reset();
}
}

Expand Down
2 changes: 1 addition & 1 deletion build_resolvers/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build_resolvers
version: 2.3.2
version: 2.3.3-wip
description: Resolve Dart code in a Builder
repository: https://github.com/dart-lang/build/tree/master/build_resolvers

Expand Down
4 changes: 3 additions & 1 deletion build_runner_core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
## 7.2.11-dev
## 7.2.11-wip

- Add better error messages for incomplete package configs when creating a
package graph.
- Update to build_resolvers 2.3.3 and use the shared analyzer resolvers
instance.

## 7.2.10

Expand Down
2 changes: 1 addition & 1 deletion build_runner_core/lib/src/generate/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ feature, you may need to run `dart run build_runner clean` and then rebuild.
}
trackPerformance = true;
}
resolvers ??= AnalyzerResolvers();
resolvers ??= AnalyzerResolvers.sharedInstance;

return BuildOptions._(
debounceDelay: debounceDelay,
Expand Down
4 changes: 2 additions & 2 deletions build_runner_core/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build_runner_core
version: 7.2.11-dev
version: 7.2.11-wip
description: Core tools to organize the structure of a build and run Builders.
repository: https://github.com/dart-lang/build/tree/master/build_runner_core

Expand All @@ -15,7 +15,7 @@ dependencies:
async: ^2.5.0
build: ^2.4.0
build_config: ^1.0.0
build_resolvers: ^2.0.0
build_resolvers: ^2.3.3
collection: ^1.15.0
convert: ^3.0.0
crypto: ^3.0.0
Expand Down
2 changes: 2 additions & 0 deletions build_test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 2.2.1-dev

- Avoid passing a nullable value to `Future.value`.
- Update to build_resolvers 2.3.3, which resolves some race conditions when
using multiple resolvers instances.

## 2.2.0

Expand Down
6 changes: 2 additions & 4 deletions build_test/lib/src/resolve_source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import 'in_memory_writer.dart';
import 'multi_asset_reader.dart';
import 'package_reader.dart';

final defaultResolvers = AnalyzerResolvers();

/// Marker constant that may be used in combination with [resolveSources].
///
/// Use of this string means instead of using the contents of the string as the
Expand Down Expand Up @@ -199,8 +197,8 @@ Future<T> _resolveAssets<T>(
// Use the default resolver if no experiments are enabled. This is much
// faster.
resolvers ??= packageConfig == null && enabledExperiments.isEmpty
? defaultResolvers
: AnalyzerResolvers(null, null, resolvedConfig);
? AnalyzerResolvers.sharedInstance
: AnalyzerResolvers.custom(packageConfig: resolvedConfig);

// We don't care about the results of this build, but we also can't await
// it because that would block on the `tearDown` of the `resolveBuilder`.
Expand Down
5 changes: 2 additions & 3 deletions build_test/lib/src/test_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import 'assets.dart';
import 'in_memory_reader.dart';
import 'in_memory_writer.dart';
import 'multi_asset_reader.dart';
import 'resolve_source.dart';
import 'written_asset_reader.dart';

AssetId _passThrough(AssetId id) => id;
Expand Down Expand Up @@ -171,8 +170,8 @@ Future testBuilder(
var logger = Logger('testBuilder');
var logSubscription = logger.onRecord.listen(onLog);
var resolvers = packageConfig == null && enabledExperiments.isEmpty
? defaultResolvers
: AnalyzerResolvers(null, null, packageConfig);
? AnalyzerResolvers.sharedInstance
: AnalyzerResolvers.custom(packageConfig: packageConfig);

for (var input in inputIds) {
// create another writer spy and reader for each input. This prevents writes
Expand Down
2 changes: 1 addition & 1 deletion build_test/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ dependencies:
async: ^2.5.0
build: ^2.1.0
build_config: ^1.0.0
build_resolvers: ^2.0.0
build_resolvers: ^2.3.3
crypto: ^3.0.0
glob: ^2.0.0
html: ^0.15.0
Expand Down

0 comments on commit 2acca4c

Please sign in to comment.