From 76566af93c084b3dc28b0739a3e701b4025004b0 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Tue, 17 Aug 2021 23:23:04 +0200 Subject: [PATCH] Allow configurable outputs for combining builder --- source_gen/CHANGELOG.md | 5 + source_gen/README.md | 36 +++++++ source_gen/lib/builder.dart | 62 ++++++++++-- source_gen/lib/src/builder.dart | 32 +++--- source_gen/lib/src/utils.dart | 14 ++- source_gen/pubspec.yaml | 4 +- source_gen/test/builder_test.dart | 157 ++++++++++++++++++++++++++---- 7 files changed, 265 insertions(+), 45 deletions(-) diff --git a/source_gen/CHANGELOG.md b/source_gen/CHANGELOG.md index d197fc6f..006752f4 100644 --- a/source_gen/CHANGELOG.md +++ b/source_gen/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.1.0-dev + +* Add the `build_extensions` option to `combining_builder`, allowing output + files to be generated into a different directory. + ## 1.0.5 * Fix a bug with reviving constant expressions which are fields defined on a diff --git a/source_gen/README.md b/source_gen/README.md index b4893694..4fa375e3 100644 --- a/source_gen/README.md +++ b/source_gen/README.md @@ -117,6 +117,41 @@ targets: If you provide a builder that uses `SharedPartBuilder` and `combining_builder`, you should document this feature for your users. +### Generating files in different directories + +When using shared-part builders which apply the `combining_builder` as part of +the build, the output location for an input file can be changed. +By default, a `.g.dart` file next to the input is generated. + +To change this, set the `build_extensions` option on the combining builder. In +the options, `build_extensions` is a map from `String` to `String`, where the +key is matches inputs and the value is a single build output. +For more details on build extensions, see [the docs in the build package][outputs]. + +For example, you can use these options to generate files under `lib/generated` +with the following build configuration: + +```yaml +targets: + $default: + builders: + source_gen|combining_builder: + options: + build_extensions: + '^lib/{{}}.dart': 'lib/generated/{{}}.g.dart' +``` + +Remember to change the `part` statement in the input to refer to the correct +output file in the other directory. + +Note that builder options are part of `source_gen`'s public api! When using +them in a build configuration, always add a dependency on `source_gen` as well: + +```yaml +dev_dependencies: + source_gen: ^1.1.0 +``` + ## FAQ ### What is the difference between `source_gen` and [build][]? @@ -148,3 +183,4 @@ wraps a single Generator to make a `Builder` which creates Dart library files. [Trivial example]: https://github.com/dart-lang/source_gen/blob/master/source_gen/test/src/comment_generator.dart [Full example package]: https://github.com/dart-lang/source_gen/tree/master/example [example usage]: https://github.com/dart-lang/source_gen/tree/master/example_usage +[outputs]: https://github.com/dart-lang/build/blob/master/docs/writing_a_builder.md#configuring-outputs \ No newline at end of file diff --git a/source_gen/lib/builder.dart b/source_gen/lib/builder.dart index 66d35aeb..54044b86 100644 --- a/source_gen/lib/builder.dart +++ b/source_gen/lib/builder.dart @@ -21,6 +21,9 @@ import 'src/utils.dart'; const _outputExtensions = '.g.dart'; const _partFiles = '.g.part'; +const _defaultExtensions = { + '.dart': [_outputExtensions] +}; Builder combiningBuilder([BuilderOptions options = BuilderOptions.empty]) { final optionsMap = Map.from(options.config); @@ -29,10 +32,12 @@ Builder combiningBuilder([BuilderOptions options = BuilderOptions.empty]) { final ignoreForFile = Set.from( optionsMap.remove('ignore_for_file') as List? ?? [], ); + final buildExtensions = _validatedBuildExtensionsFrom(optionsMap); final builder = CombiningBuilder( includePartName: includePartName, ignoreForFile: ignoreForFile, + buildExtensions: buildExtensions, ); if (optionsMap.isNotEmpty) { @@ -41,6 +46,41 @@ Builder combiningBuilder([BuilderOptions options = BuilderOptions.empty]) { return builder; } +Map> _validatedBuildExtensionsFrom( + Map optionsMap) { + final extensionsOption = optionsMap.remove('build_extensions'); + if (extensionsOption == null) return _defaultExtensions; + + if (extensionsOption is! Map) { + throw ArgumentError( + 'Configured build_extensions should be a map from inputs to outputs.'); + } + + final result = >{}; + + for (final entry in extensionsOption.entries) { + final input = entry.key; + if (input is! String || !input.endsWith('.dart')) { + throw ArgumentError('Invalid key in build_extensions option: `$input` ' + 'should be a string ending with `.dart`'); + } + + final output = entry.value; + if (output is! String || !output.endsWith('.dart')) { + throw ArgumentError('Invalid output extension `$output`. It should be a ' + 'string ending with `.dart`'); + } + + result[input] = [output]; + } + + if (result.isEmpty) { + throw ArgumentError('Configured build_extensions must not be empty.'); + } + + return result; +} + PostProcessBuilder partCleanup(BuilderOptions options) => const FileDeletingBuilder(['.g.part']); @@ -53,9 +93,7 @@ class CombiningBuilder implements Builder { final Set _ignoreForFile; @override - Map> get buildExtensions => const { - '.dart': [_outputExtensions] - }; + final Map> buildExtensions; /// Returns a new [CombiningBuilder]. /// @@ -65,6 +103,7 @@ class CombiningBuilder implements Builder { const CombiningBuilder({ bool? includePartName, Set? ignoreForFile, + this.buildExtensions = _defaultExtensions, }) : _includePartName = includePartName ?? false, _ignoreForFile = ignoreForFile ?? const {}; @@ -104,8 +143,20 @@ class CombiningBuilder implements Builder { .where((s) => s.isNotEmpty) .join('\n\n'); if (assets.isEmpty) return; + final inputLibrary = await buildStep.inputLibrary; - final partOf = nameOfPartial(inputLibrary, buildStep.inputId); + final outputId = buildStep.allowedOutputs.single; + final partOf = nameOfPartial(inputLibrary, buildStep.inputId, outputId); + + // Ensure that the input has a correct `part` statement. + final libraryUnit = + await buildStep.resolver.compilationUnitFor(buildStep.inputId); + final part = computePartUrl(buildStep.inputId, outputId); + if (!hasExpectedPartDirective(libraryUnit, part)) { + log.warning('$part must be included as a part directive in ' + 'the input library with:\n part \'$part\';'); + return; + } final ignoreForFile = _ignoreForFile.isEmpty ? '' @@ -117,7 +168,6 @@ part of $partOf; $assets '''; - await buildStep.writeAsString( - buildStep.inputId.changeExtension(_outputExtensions), output); + await buildStep.writeAsString(outputId, output); } } diff --git a/source_gen/lib/src/builder.dart b/source_gen/lib/src/builder.dart index d902f82d..cf285254 100644 --- a/source_gen/lib/src/builder.dart +++ b/source_gen/lib/src/builder.dart @@ -105,31 +105,29 @@ class _Builder extends Builder { if (!_isLibraryBuilder) { final asset = buildStep.inputId; - final name = nameOfPartial(library, asset); + final name = nameOfPartial(library, asset, outputId); contentBuffer.writeln(); - String part; if (this is PartBuilder) { contentBuffer ..write(languageOverrideForLibrary(library)) ..writeln('part of $name;'); - part = computePartUrl(buildStep.inputId, outputId); + final part = computePartUrl(buildStep.inputId, outputId); + + final libraryUnit = + await buildStep.resolver.compilationUnitFor(buildStep.inputId); + final hasLibraryPartDirectiveWithOutputUri = + hasExpectedPartDirective(libraryUnit, part); + if (!hasLibraryPartDirectiveWithOutputUri) { + // TODO: Upgrade to error in a future breaking change? + log.warning('$part must be included as a part directive in ' + 'the input library with:\n part \'$part\';'); + return; + } } else { assert(this is SharedPartBuilder); - final finalPartId = buildStep.inputId.changeExtension('.g.dart'); - part = computePartUrl(buildStep.inputId, finalPartId); - } - - final libraryUnit = - await buildStep.resolver.compilationUnitFor(buildStep.inputId); - final hasLibraryPartDirectiveWithOutputUri = libraryUnit.directives - .whereType() - .any((e) => e.uri.stringValue == part); - if (!hasLibraryPartDirectiveWithOutputUri) { - // TODO: Upgrade to error in a future breaking change? - log.warning('$part must be included as a part directive in ' - 'the input library with:\n part \'$part\';'); - return; + // For shared-part builders, `part` statements will be checked by the + // combining build step. } } diff --git a/source_gen/lib/src/utils.dart b/source_gen/lib/src/utils.dart index d9064d34..9555ed28 100644 --- a/source_gen/lib/src/utils.dart +++ b/source_gen/lib/src/utils.dart @@ -4,6 +4,7 @@ import 'dart:io'; +import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:build/build.dart'; @@ -37,14 +38,21 @@ String typeNameOf(DartType type) { throw UnimplementedError('(${type.runtimeType}) $type'); } +bool hasExpectedPartDirective(CompilationUnit unit, String part) => + unit.directives + .whereType() + .any((e) => e.uri.stringValue == part); + /// Returns a name suitable for `part of "..."` when pointing to [element]. -String nameOfPartial(LibraryElement element, AssetId source) { +String nameOfPartial(LibraryElement element, AssetId source, AssetId output) { if (element.name.isNotEmpty) { return element.name; } - final sourceUrl = p.basename(source.uri.toString()); - return '\'$sourceUrl\''; + assert(source.package == output.package); + final relativeSourceUri = + p.url.relative(source.path, from: p.dirname(output.path)); + return '\'$relativeSourceUri\''; } /// Returns a suggested library identifier based on [source] path and package. diff --git a/source_gen/pubspec.yaml b/source_gen/pubspec.yaml index 3eb5672d..82836e39 100644 --- a/source_gen/pubspec.yaml +++ b/source_gen/pubspec.yaml @@ -1,5 +1,5 @@ name: source_gen -version: 1.0.5 +version: 1.1.0-dev description: >- Source code generation builders and utilities for the Dart build system repository: https://github.com/dart-lang/source_gen @@ -10,7 +10,7 @@ environment: dependencies: analyzer: ^2.0.0 async: ^2.5.0 - build: ^2.0.0 + build: ^2.1.0 dart_style: ^2.0.0 glob: ^2.0.0 meta: ^1.3.0 diff --git a/source_gen/test/builder_test.dart b/source_gen/test/builder_test.dart index 46df52c2..e98b7135 100644 --- a/source_gen/test/builder_test.dart +++ b/source_gen/test/builder_test.dart @@ -275,24 +275,24 @@ part "a.foo.dart";''' }); }); - group('SharedPartBuilder', () { - test('warns about missing part', () async { - final srcs = _createPackageStub(testLibContent: _testLibContentNoPart); - final builder = SharedPartBuilder([const CommentGenerator()], 'comment'); - final logs = []; - await testBuilder( - builder, - srcs, - onLog: (log) { - logs.add(log.message); - }, - ); - expect(logs, [ - 'test_lib.g.dart must be included as a part directive in the input ' - 'library with:\n part \'test_lib.g.dart\';' - ]); - }); + test('PartBuilder warns about missing part', () async { + final srcs = _createPackageStub(testLibContent: _testLibContentNoPart); + final builder = PartBuilder([const CommentGenerator()], '.g.dart'); + final logs = []; + await testBuilder( + builder, + srcs, + onLog: (log) { + logs.add(log.message); + }, + ); + expect(logs, [ + 'test_lib.g.dart must be included as a part directive in the input ' + 'library with:\n part \'test_lib.g.dart\';' + ]); + }); + group('SharedPartBuilder', () { test('outputs .g.part files', () async { await testBuilder( SharedPartBuilder( @@ -504,6 +504,129 @@ foo generated content }, ); }); + + test('warns about missing part statement', () async { + final logs = []; + await testBuilder( + combiningBuilder(), + { + '$_pkgName|lib/a.dart': '', + '$_pkgName|lib/a.foo.g.part': '// generated', + }, + generateFor: {'$_pkgName|lib/a.dart'}, + onLog: (msg) => logs.add(msg.message), + outputs: {}, + ); + + expect( + logs, + contains( + 'a.g.dart must be included as a part directive in the input ' + 'library with:\n part \'a.g.dart\';', + ), + ); + }); + + group('with custom extensions', () { + test("disallows options that aren't a map", () { + expect( + () => combiningBuilder( + const BuilderOptions({'build_extensions': 'foo'})), + throwsArgumentError, + ); + }); + + test('disallows empty options', () { + expect( + () => + combiningBuilder(const BuilderOptions({'build_extensions': {}})), + throwsArgumentError, + ); + }); + + test('disallows inputs not ending with .dart', () { + expect( + () => combiningBuilder(const BuilderOptions({ + 'build_extensions': { + '.txt': ['.dart'] + } + })), + throwsA( + isArgumentError.having( + (e) => e.message, + 'message', + 'Invalid key in build_extensions option: `.txt` should be a ' + 'string ending with `.dart`', + ), + ), + ); + }); + + test('disallows outputs not ending with .dart', () { + expect( + () => combiningBuilder(const BuilderOptions({ + 'build_extensions': {'.dart': '.out'} + })), + throwsA( + isArgumentError.having( + (e) => e.message, + 'message', + 'Invalid output extension `.out`. It should be a string ending ' + 'with `.dart`', + ), + ), + ); + }); + + test('generates relative `path of` for output in different directory', + () async { + await testBuilder( + combiningBuilder(const BuilderOptions({ + 'build_extensions': {'^lib/{{}}.dart': 'lib/generated/{{}}.g.dart'} + })), + { + '$_pkgName|lib/a.dart': 'part "generated/a.g.dart";', + '$_pkgName|lib/a.foo.g.part': '\n\nfoo generated content\n', + '$_pkgName|lib/a.only_whitespace.g.part': '\n\n\t \n \n', + '$_pkgName|lib/a.bar.g.part': '\nbar generated content', + }, + generateFor: {'$_pkgName|lib/a.dart'}, + outputs: { + '$_pkgName|lib/generated/a.g.dart': decodedMatches(endsWith(r''' +part of '../a.dart'; + +bar generated content + +foo generated content +''')), + }, + ); + }); + + test('warns about missing part statement', () async { + final logs = []; + await testBuilder( + combiningBuilder(const BuilderOptions({ + 'build_extensions': {'^lib/{{}}.dart': 'lib/generated/{{}}.g.dart'} + })), + { + '$_pkgName|lib/a.dart': '', + '$_pkgName|lib/a.foo.g.part': '// generated', + }, + generateFor: {'$_pkgName|lib/a.dart'}, + onLog: (msg) => logs.add(msg.message), + outputs: {}, + ); + + expect( + logs, + contains( + 'generated/a.g.dart must be included as a part directive in the ' + 'input library with:\n part \'generated/a.g.dart\';', + ), + ); + }); + }); }); test('can skip formatting with a trivial lambda', () async {