Skip to content
This repository has been archived by the owner on Feb 4, 2025. It is now read-only.

Add LiteralParams macro. #128

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Oct 31, 2024

Fix converter for lists of union types.

This macro shows the use case where the macro metadata is written entirely in the annotation.

Mostly works already, just missing constructor invocation names.

@davidmorgan davidmorgan changed the title Add PrimitiveParams macro. Add LiteralParams macro. Oct 31, 2024
Fix converter for lists of union types.
@davidmorgan davidmorgan marked this pull request as ready for review October 31, 2024 10:12
"expression": {
"type": "IntegerLiteral",
"value": {
"text": "23"

Choose a reason for hiding this comment

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

I've update the BooleanLiteral, IntegerLiteral and DoubleLiteral to hold their primitive value where possible (ints are a bit weird). It might be good to pass that along - in particular since evaluation (coming shortly) will only provide the value and not the text.

// TODO(davidmorgan): common code for this in `dart_model` so macros don't
// all have to write expression evaluation code.
extension ExpressionExtension on Expression {
Object get evaluate => switch (type) {

Choose a reason for hiding this comment

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

This is done more extensively in https://dart-review.googlesource.com/c/sdk/+/392440

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I expect we need a higher level discussion soon to figure out where all this is headed. Or one of us could throw some thoughts on an issue/doc.

No rush though, going to push on the other tracks (performance, data model) a bit next. Thanks.

Future<void> buildDeclarationsForClass(
ClassDeclarationsBuilder builder) async {
final annotation = builder
.target.metadataAnnotations.first.expression.asConstructorInvocation;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably add a TODO for checking the type of the annotation (in practice you will want to search for a specific one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks! Done.

Copy link
Contributor Author

@davidmorgan davidmorgan left a comment

Choose a reason for hiding this comment

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

Thanks!

Future<void> buildDeclarationsForClass(
ClassDeclarationsBuilder builder) async {
final annotation = builder
.target.metadataAnnotations.first.expression.asConstructorInvocation;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks! Done.

// TODO(davidmorgan): common code for this in `dart_model` so macros don't
// all have to write expression evaluation code.
extension ExpressionExtension on Expression {
Object get evaluate => switch (type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I expect we need a higher level discussion soon to figure out where all this is headed. Or one of us could throw some thoughts on an issue/doc.

No rush though, going to push on the other tracks (performance, data model) a bit next. Thanks.

@davidmorgan davidmorgan merged commit 178c59f into dart-archive:main Oct 31, 2024
5 checks passed
@davidmorgan davidmorgan deleted the metadata-macro branch October 31, 2024 15:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants