Skip to content
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.

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

Scoping augmentations down for code generators #4256

Open
munificent opened this issue Feb 6, 2025 · 13 comments
Open

Scoping augmentations down for code generators #4256

munificent opened this issue Feb 6, 2025 · 13 comments
Labels
augmentations Issues related to the augmentations proposal.

Comments

@munificent
Copy link
Member

The augmentations proposal is very comprehensive. It basically lets you modify existing declarations in all possible ways. This was necessary because we wanted macros to be quite powerful and the intent was to have macro applications compile to a generated augmentation. With macros out of the picture now, we have the opportunity to simplify augmentations. We'd still like them to be powerful, but there are some corners of the proposal that always felt pretty subtle and tricky and if those corners aren't ultimately that useful, it's probably worth removing them.

Without macros, code generation will continue to be critical for the ecosystem. I think if code generators start outputting augmentations instead of just libraries or part files, the overall user experience can be significantly improved. For example, here's what a user of built_value has to write today:

abstract class SimpleValue implements Built<SimpleValue, SimpleValueBuilder> {
  static Serializer<SimpleValue> get serializer => _$simpleValueSerializer;

  int get anInt;
  String? get aString;

  factory SimpleValue([void Function(SimpleValueBuilder) updates]) =
      _$SimpleValue;
  SimpleValue._();
}

Here's what it could look like if built_value generated an augmentation instead of a part file with a subclass that the user has to explicitly forward to:

@serialized
class SimpleValue implements Built<SimpleValue, SimpleValueBuilder> {
  final int anInt;
  final String? aString;
}

To start to get a sense of what requirements code generators would place on augmentations, I looked at the built_value and json_serializable packages. (I know that @davidmorgan and @jakemac53 could have just told me the answer to these, but it was useful for me to load those packages into my head by doing the exercise myself.)

For both of those, I hand-migrated their examples to show what the user-authored and generated code might look like with augmentations:

Overall, the experience was a positive one. I think the hand-authored code gets smaller and simpler without the need to forward to the generated code. The generated code often gets simpler too. The set of augmentation features used was quite small. For the most part, it's just adding new top-level declarations and new members in classes.

I found a few interesting cases:

built_value field validation

The built_value package lets a user validate fields inside the value type's constructor, like:

abstract class ValidatedValue
    implements Built<ValidatedValue, ValidatedValueBuilder> {
  int get anInt;

  factory ValidatedValue([void Function(ValidatedValueBuilder) updates]) =
      _$ValidatedValue;

  ValidatedValue._() {
    if (anInt == 7) throw StateError('anInt may not be 7');
  }
}

This works because the code generator creates a subclass whose constructor ends up calling the superclass's constructor. I'd like it if the user didn't have to hand-write the forwarding constructor and let the generated augmentation define that, like so:

// User-authored:
abstract class ValidatedValue
    implements Built<ValidatedValue, ValidatedValueBuilder> {
  int get anInt;
}

// Generated:
augment class ValidatedValue {
  factory ValidatedValue([void Function(ValidatedValueBuilder)? updates]) =>
      (new ValidatedValueBuilder()..update(updates))._build();

  ValidatedValue._({required this.anInt});

  // ...
}

But then where does the validation code go? If we want to support constructor augmentations, then the user could define a constructor in the hand-authored class and then the generated constructor would call augmented to reach that constructor and run the validation in its body. But that requires the user to write the full parameter list for the constructor, which is fairly verbose. Instead, I think a cleaner approach is:

// User-authored:
abstract class ValidatedValue
    implements Built<ValidatedValue, ValidatedValueBuilder> {
  int get anInt;

  _validate() {
    if (anInt == 7) throw StateError('anInt may not be 7');
  }
}

// Generated:
augment class ValidatedValue {
  factory ValidatedValue([void Function(ValidatedValueBuilder)? updates]) =>
      (new ValidatedValueBuilder()..update(updates))._build();

  ValidatedValue._({required this.anInt}) {
    _validate();
  }

  // ...
}

The user writes a _validate() instance method. If the code generator sees that such a method exists, it inserts a call to it in the generated constructor in the augmentation. Since this method is called after the instance is created, it can be an instance method, so this works fine. This would also mean we don't need support for augmenting constructors.

built_value field wrappers in custom builders

The built_value package lets a user write their own builder:

 /// Builder class for [ValueWithInt].
abstract class ValueWithIntBuilder
    implements Builder<ValueWithInt, ValueWithIntBuilder> {
  int? anInt;

  factory ValueWithIntBuilder() = _$ValueWithIntBuilder;
  ValueWithIntBuilder._();
}

When they do, the code generator produces a subclass:

class _$ValueWithIntBuilder extends ValueWithIntBuilder {
  ValueWithInt? _$v;

  @override
  int? get anInt {
    _$this;
    return super.anInt;
  }

  @override
  set anInt(int? anInt) {
    _$this;
    super.anInt = anInt;
  }

  _$ValueWithIntBuilder() : super._();

  @override
  void replace(ValueWithInt other) {
    _$v = other;
  }

  ValueWithIntBuilder get _$this {
    final $v = _$v;
    if ($v != null) {
      super.anInt = $v.anInt;
      _$v = null;
    }
    return this;
  }

  ...
}

Note how anInt is overridden by a getter/setter pair. Those internally call super.anInt. If we support augmented on getters/setters, those super calls can become augmented. But note also the super.anInt call in _$this. If _$ValueWithIntBuilder is turned into an augmentation on ValueWithIntBuilder, then there's no way to express that.

You can't use augmented because you aren't in the member being augmented. This doesn't work even with the current full-featured augmentation spec.

In this case, I think the generator could instead produce:

augment class ValueWithIntBuilder {
  ValueWithInt? _$v;

  int? get anInt {
    _$this;
    return augmented;
  }

  set anInt(int? anInt) {
    _$this;
    augmented = anInt;
  }

  @override
  void replace(ValueWithInt other) {
    _$v = other;
  }

  ValueWithIntBuilder get _$this {
    if (_$v case final $v?) {
      // Clear it eagerly so that calling the setter doesn't infinite loop.
      _$v = null;
      // Call the setter, which will go through the augmentation.
      anInt = $v.anInt;
    }
    return this;
  }

  ...
}

Here, _$this no longer calls super to route around the setter. It just invokes the setter directly, which will go through the augmented setter which in turn calls augmented. To avoid an infinite loop, we clear _$v first.

This was the only place in the two packages where I found the need to use augmented.

JSON literals in json_serializable

The json_serializable package supports reading in a separate data file of JSON and inserted it into the generated code as static data. If a user writes:

@JsonLiteral('data.json')
Map get glossaryData => _$glossaryDataJsonLiteral;

Then the code generator reads that file and generates code in a part file like:

final _$glossaryDataJsonLiteral = {
  // Big blob of JSON...
};

With augmentations, it would be nice if the user didn't have to write an explicit forwarder like _$glossaryDataJsonLiteral. But they need to write something to associate a name with a literal and hang the @JsonLiteral metadata annotation of it.

It can't be an abstract getter because it's a top-level declaration. It could be an external getter, though that feels a little misleading to me since it's not really external to the program.

Instead, I suggest that it be a variable declaration with no initializer:

@JsonLiteral('data.json')
final Map glossaryData;

Then the generated augmentation augments that variable by providing an initializer:

augment final Map glossaryData = {
  // Big blob of JSON...
};

This requires support for augmenting a variable declaration with an initializer. It doesn't need to be able to call the original declaration's initializer (since there is none).

Summary

These are the only packages I've looked at so far. I started poking at freezed, but it's pretty big and will take me a while to dig into. (If you have thoughts on how you'd like to use augmentations in freezed, @rrousselGit, I would definitely like to hear them.) If there are other widely used code generated packages, I'd like to hear about them so I can see what kind of requirements they would place on augmentations.

So far, from these two packages, what I see we need is:

  • Adding new top-level declarations.

  • Adding new members to existing classes. All kinds of members: instance, static, fields, methods, etc.

  • In one place, augmenting a variable with an initializer. We could take other approaches if this was problematic.

  • Calling augmented in an augmenting getter and setter to access the augmented field. If necessary, we could probably tweak the design to avoid this as well, though it would be trickier.

  • Potentially calling an augmented constructor body from an augmenting constructor. I worked around it by using _validate() instead which I think is also a better user experience.

Thoughts? Any other packages I should look at?

@rrousselGit
Copy link

Hello there!

Overall, as long as augmentations can apply extends, with and implements to a class then add constructors/static members, then all is good.
Emphasis on being able to add custom extends. In riverpod_generator, when users do:

@riverpod
class MyNotifier {}

I'd like to generate:

augment class MyNotifier extends Notifier<...> {
  MyNotifier(super.ref, this.args);

  final SomeType args;
}

Currently, due to the lack of a way to add such constructor, I rely on a late final for all fields in the constructor. I'd like to remove those late keywords.


It's probably a tad off-topic, but I think there's one key feature that macro promised that we're loosing by "just" using augmentations:
Macro gave annotations some super-power.

This is technically not an augmentation, but feels closely related.
Annotations define how to generate the augmentations after-all ; and both require Dart changes.

Specifically, with macros we could do:

// We can pass functions in the annotation, and the generated code can use them
// We get breakpoints and this doesn't require a const function
@Annotation(someCallback: (someArg) => print('hello'))

Currently this is a huge hassle with generators. The syntax is often instead:

ReturnValue someFunction(Param someArg) {
  print('hello');
}

@Annotation(someCallback: someFunction)

This causes generators to pretty much give up on taking callbacks as parameters through the annotations and instead find different ways of achieving the same thing.

@davidmorgan
Copy link
Contributor

Thanks Bob!

Just one note to add re: built_value, rather than

@serialized
class SimpleValue implements Built<SimpleValue, SimpleValueBuilder> {
  final int anInt;
  final String? aString;
}

I would prefer to drop implements Built:

@BuiltValue(serialized: true)
class SimpleValue {
  final int anInt;
  final String? aString;
}

--it was added to reduce the boilerplate you have to write, and arguably it makes sense with how codegen works today, but the Built interface is almost never actually used as an interface and the generics are very awkward indeed. The same goes for Builder.

Just having those methods be normal methods on the post-generation type, with no interfaces, would be great :)

@munificent
Copy link
Member Author

Overall, as long as augmentations can apply extends, with and implements to a class then add constructors/static members, then all is good.

Thanks, this is really helpful.

Specifically, with macros we could do:

// We can pass functions in the annotation, and the generated code can use them
// We get breakpoints and this doesn't require a const function
@annotation(someCallback: (someArg) => print('hello'))

Hmm, that may have worked with the in-progress macro implementation, but I don't think it was ever fully intended to work. One of the reasons we canceled the feature is that we struggled for a very long time to pin down exactly what was allowed inside an annotation used for a macro application and what the semantics of those argument lists were.

As far as I know, we plan to allow non-const expressions in there.

Currently this is a huge hassle with generators. The syntax is often instead:

ReturnValue someFunction(Param someArg) {
print('hello');
}

@annotation(someCallback: someFunction)
This causes generators to pretty much give up on taking callbacks as parameters through the annotations and instead find different ways of achieving the same thing.

Yeah, in general passing a "chunk of code" to either a macro or a code generator is a hard problem. It immediately raises all of the hard questions around scoping/hygiene/variable capture that have plagued macros for decades. Those questions aren't unsolvable, but the solutions have a lot of tricky trade-offs.

@rrousselGit
Copy link

I think a lot of the challenges can be offloaded to the analyzer.

One solution could be that given:

@Annotation(cb: (arg) => print(arg));

Generators generate:

void someRandomFunction() {
  ((arg) => print(arg))(value);
}

With possibly some comments around it to tell Analyzer that this snippet was defined in the @Annotation; so that we redirect breakpoints and stuff.

I've raised a few issues with a similar pattern yesterday for related topics:

Those could be ways to implement proposed Macro features without modifying the language.

@bivens-dev
Copy link

@munificent I’m not sure if you had a chance to already see the very interesting new project called Codable and its related RFC: New Serialization Protocol for Dart but I thought that you might also be interested in seeing how they were thinking about using Augmentations as outlined here: schultek/codable#10

@Guldem
Copy link

Guldem commented Feb 14, 2025

Like in the example with JsonLiteral I faced something similar when I experimented with macros for Chopper.

When defining a 'service' the abstract class with abstract method are annotated and the implementation is generated. But when I tried to augment the class and its method I also had to make them external which felt a bit strange.

Currently it looks like this:

@ChopperApi(baseUrl: '/resources')
abstract class MyService extends ChopperService {
  static MyService create([ChopperClient? client]) => _$MyService(client);

  @GET(path: '/{id}/')
  Future<Response> getResource(@Path() String id);

  @GET(path: '/list')
  Future<Response<BuiltList<Resource>>> getBuiltListResources();

  @GET(path: '/', headers: {'foo': 'bar'})
  Future<Response<Resource>> getTypedResource();

  @POST()
  Future<Response<Resource>> newResource(
    @Body() Resource resource, {
    @Header() String? name,
  });
}

But if something like this could be possible with augmentations that would be really nice:

@ChopperApi(baseUrl: '/resources')
class MyService {
  @GET(path: '/{id}/')
  Future<Response> getResource(@Path() String id);

  @GET(path: '/list')
  Future<Response<BuiltList<Resource>>> getBuiltListResources();

  @GET(path: '/', headers: {'foo': 'bar'})
  Future<Response<Resource>> getTypedResource();

  @POST()
  Future<Response<Resource>> newResource(
    @Body() Resource resource, {
    @Header() String? name,
  });
}

Without the need to add external to each method.

@rrousselGit
Copy link

rrousselGit commented Feb 14, 2025

Going back to this:

One unsolved problem by build_runner is how analysis contains InvalidTypes when User code contains to-be-generated types.
That's one of the reasons Macros had the whole "phase" ordeal

One way generators can solve it is by relying on multiple part.
For example Freezed uses:

part 'myfile.freezed.dart';
part 'myfile.g.dart';

This enables JSON-serializable to interact with types generated by Freezed.

The problem with this solution is: It's super painful for users to have to add multiple parts.
So that makes such solution not workable at scale, as the more generators use this solution, the more parts users have to define.

In that case:

Could augmentation libraries import more augmentation libraries?

Say we have:

// file.dart
augment import 'file.freezed.dart';

Could we maybe support:

// file.freezed.dart
augment import 'file.g.dart';

Then, the number of "phase" becomes an implementation detail rather than a requirement placed on users.

@davidmorgan
Copy link
Contributor

Could augmentation libraries import more augmentation libraries?

Augmentation libraries are so 2024 ;) ... the design now is that augmentations can go anywhere.

When you want to put them in a separate file, you'll use part files; and to make part files a better fit, there will be enhanced parts, which does indeed allow the nesting that you're asking for.

Pretty much, enhanced parts solves the "generate to library or part?" question with a definite answer for all cases: "enhanced part" :)

@rrousselGit
Copy link

Cool! Then we'll have to make sure that it combines nicely with build_runner such that a generator can generate a part ; which is the asked to generate even more parts.

Preferably, all that "runs_before" stuff in build.yaml shouldn't be necessary anymore

@davidmorgan
Copy link
Contributor

Cool! Then we'll have to make sure that it combines nicely with build_runner such that a generator can generate a part ; which is the asked to generate even more parts.

Preferably, all that "runs_before" stuff in build.yaml shouldn't be necessary anymore

I'm optimistic ;)

There is also some discussion about whether a generator could output a part for API and a nested part for implementation; then analysis can complete after the API is output without waiting for implementation. This gets interesting if a generator can output API more cheaply, for example if it can output API before resolution.

@rrousselGit
Copy link

rrousselGit commented Feb 26, 2025

By the way, did we settle on whether augmentations could add an extends?
I remember some discussions about not allowing that.

I think that'd be quite useful.

I can already see generators for sealed classes to add an automatic extends Base for subclasses if we ever get nested classes:

@freezed
sealed class Foo {
  class A {}
  class B{}
}
augment class Foo {
  augment class A extends Foo {}
  augment class B extends Foo {}
}

(obviously this is an example about a future feature, but I think that's a good one)

And of course, there's the myriad of existing code-generators using extends _$Foo, like Riverpod:

@riverpod
class Counter extends _$Counter {...}

@mateusfccp
Copy link
Contributor

By the way, did we settle on whether augmentations could add an extends? I remember some discussions about not allowing that.

I was not following these discussions, but this is a must IMO. Your last example is enough for me. I am also using a similar approach locally, where I have something similar.

@tatumizer
Copy link

Consider a zig-style alternative:

// the prototype
class _Counter {
  // your class defs here
}
class Counter = @riverpod(_Counter);
class WithExtras = @extras(Counter);
// or directly:
class RiverpodWithExtras = @extras(@riverpod(_Counter));

(this doesn't require augmentations - every macro transforms its input into output, with no artificial restrictions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
augmentations Issues related to the augmentations proposal.
Projects
Development

No branches or pull requests

7 participants