Skip to content

GeneratorBuilder flag for "part of" without "library". #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 22, 2017
Merged

GeneratorBuilder flag for "part of" without "library". #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
```

In Dart SDK `>=1.25.0` this can be relaxed as `part of` can refer to a path.
To opt-in, `GeneratorBuilder` now has a new flag, `requireLibraryDirective`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea, but you can check the SDK version using Platform.version from dart:io, and we could make the default be based on that even? That would also likely require a dependency on pub_semver but that should be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw I actually like this idea, but it hasn't technically landed yet even at -dev. Maybe we can set the default based on that once 1.25.0 lands officially?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - if it hasn't landed officially in any release there is no sdk constraint we can set either right.... so we can't really release this feature at all until then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured adding this flags gives users the chance to make that choice for themselves versus waiting for us to do the right thing ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I forgot about the fact that you need to update your sdk constraint if you are publishing these files as part of a package, so using it automatically is a no-go.

What we could do though is just add the sdk constraint in this package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But ya in any case adding the flag seems like an easy workaround to sidestep the issue

Set it to `false`, and also set your `sdk` constraint appropriately:

```yaml
sdk: '>=1.25.0 <2.0.0'
```

* Added `findType`, an utility method for `LibraryElement#getType` that also
traverses `export` directives for publicly exported types. For example, to
Expand Down
22 changes: 19 additions & 3 deletions lib/src/builder.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,24 @@ class GeneratorBuilder extends Builder {
/// Whether to emit a standalone (non-`part`) file in this builder.
final bool isStandalone;

final bool _requireLibraryDirective;

/// Wrap [generators] to form a [Builder]-compatible API.
///
/// May set [requireLibraryDirective] to `false` in order to opt-in to
/// supporting a `1.25.0` feature of `part of` being usable without an
/// explicit `library` directive. Developers should restrict their `pubspec`
/// accordingly:
/// ```yaml
/// sdk: '>=1.25.0 <2.0.0'
/// ```
GeneratorBuilder(this.generators,
{OutputFormatter formatOutput,
this.generatedExtension: '.g.dart',
this.isStandalone: false})
: formatOutput = formatOutput ?? _formatter.format {
this.isStandalone: false,
bool requireLibraryDirective: true})
: formatOutput = formatOutput ?? _formatter.format,
_requireLibraryDirective = requireLibraryDirective {
if (generatedExtension == null) {
throw new ArgumentError.notNull('generatedExtension');
}
Expand Down Expand Up @@ -86,7 +98,11 @@ class GeneratorBuilder extends Builder {

if (!isStandalone) {
var asset = buildStep.inputId;
var name = nameOfPartial(library, asset);
var name = nameOfPartial(
library,
asset,
allowUnnamedPartials: !_requireLibraryDirective,
);
if (name == null) {
var suggest = suggestLibraryName(asset);
throw new InvalidGenerationSourceError(
Expand Down
4 changes: 3 additions & 1 deletion lib/src/utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/standard_resolution_map.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:build/build.dart';
import 'package:path/path.dart' as p;

String friendlyNameForElement(Element element) {
var friendlyName = element.displayName;
Expand Down Expand Up @@ -56,7 +57,8 @@ String nameOfPartial(
return element.name;
}
if (allowUnnamedPartials) {
return '\'package:${source.package}/${source.path}\'';
var sourceUrl = p.basename(source.uri.toString());
return '\'$sourceUrl\'';
}
return null;
}
Expand Down
20 changes: 20 additions & 0 deletions test/builder_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ void main() {
throwsA(const isInstanceOf<InvalidGenerationSourceError>()));
});

test('Allow no "library" when requireLibraryDirective=false', () async {
var sources = _createPackageStub(pkgName, testLibContent: 'class A {}');
var builder = new GeneratorBuilder([const CommentGenerator()],
requireLibraryDirective: false);
await testBuilder(builder, sources,
outputs: {'$pkgName|lib/test_lib.g.dart': _testGenNoLibrary});
});

test(
'Simple Generator test for library',
() => _generateTest(
Expand Down Expand Up @@ -306,3 +314,15 @@ part of test_lib;

// Code for "class Customer"
''';

const _testGenNoLibrary = r'''// GENERATED CODE - DO NOT MODIFY BY HAND

part of 'test_lib.dart';

// **************************************************************************
// Generator: CommentGenerator
// Target: class A
// **************************************************************************

// Code for "class A"
''';