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

Support for macro and augmentation features #1297

Open
jakemac53 opened this issue Oct 26, 2023 · 9 comments
Open

Support for macro and augmentation features #1297

jakemac53 opened this issue Oct 26, 2023 · 9 comments

Comments

@jakemac53
Copy link
Contributor

This may soon become a blocker for experimenting with these features, given that formatting is required in places where we want to land macros.

@jakemac53
Copy link
Contributor Author

@munificent do you think this is something that you could work on relatively soon? As we ramp up work on macros, not having the ability to format them is becoming more annoying :).

@munificent
Copy link
Member

Yeah, I'll get on it today.

@munificent
Copy link
Member

CC @scheglov and @bwilkerson. Just to let you know, I'm implementing this now, which means that the formatter will be using all of the corners of the Analyzer AST API related to this: AugmentationImportDirective, MethodDeclaration.augmentKeyword, etc.

If those APIs change, we'll have to roll them out gradually to not break the formatter. Since those ASTs are never meaningful if you don't have the "macros" experiment enabled, I don't know if you consider changes to those APIs to be breaking or not. But soon, they'll be able to break the formatter. :)

@munificent
Copy link
Member

I got most of the way through augmentations, but it looks like the analyzer AST API isn't fully there yet. I'm missing:

  • FunctionDeclarationImpl has an augmentKeyword field, but FunctionDeclaration doesn't expose it.
  • It doesn't look like top level variables have an augmentKeyword that I could find.
  • Likewise, I couldn't find an augmentKeyword on FieldDeclaration.

This is using analyzer 6.3.0.

I have a PR out for the macro modifier: #1357

@scheglov
Copy link
Contributor

scheglov commented Jan 12, 2024

I could add missing APIs and publish the analyzer.

However some of these are missing because corresponding token sequence is not parsed.
I vaguely remember this being the case for extensions and variables, but I have to tried recently.
So, it might not work anyway.

And so, maybe we will have to live with what we have currently, and re-iterate on the analyzer and the formatter when the parser is updated. I don't have short term plan to work on the parser myself.

Yes, I guess we have to admit that the formatter will start using it, and now changing augmentKeyword will be a breaking change to the analyzer, that will have to go in the next major version, etc.

@bwilkerson
Copy link
Member

I don't have any problem with exposing those pieces of the API. As far as I'm aware this isn't really any different than any other syntax-introducing language change, other than the fact that we're updating the formatter a little earlier in the process. It does mean that there's a higher cost to making changes to the syntax for macros, but I don't think that should stop us from making progress.

@munificent
Copy link
Member

And so, maybe we will have to live with what we have currently, and re-iterate on the analyzer and the formatter when the parser is updated. I don't have short term plan to work on the parser myself.

My understanding is that @jakemac53 mostly cares about support for formatting the macro keyword (which the analyzer AST API already supports just fine) and that augment and augmented can come later. So I think it's OK to just sit on this for now.

Yes, I guess we have to admit that the formatter will start using it, and now changing augmentKeyword will be a breaking change to the analyzer, that will have to go in the next major version, etc.

I'm not planning to land support for formatting augment yet, so there is still time to change that API if you want. I'll let you know before I intend to commit anything.

@pattobrien
Copy link

I'm not planning to land support for formatting augment yet, so there is still time to change that API if you want. I'll let you know before I intend to commit anything.

Do you have any rough timeline on when you think this may be supported? (particularly import augment 'foo.dart' directives)

I'm working on a couple macro-related packages that rely on build_runner generations (for now), and I'm debating whether part files or augmentation libraries would be the better experience in the short-term, which depends on this issue which currently breaks formatting in user's code.

@jakemac53
Copy link
Contributor Author

We are currently discussing merging augmentations and parts, so instead we have just "enhanced parts" (which can have imports), and allow augmentations to exist in any file (part or main library). So, I wouldn't start implementing formatter support for (import augment yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

5 participants