search

dart-lang / dart_style

An opinionated formatter/linter for Dart code
https://pub.dev/packages/dart_style
BSD 3-Clause "New" or "Revised" License
640 stars 117 forks source link

Support for macro and augmentation features #1297

Open jakemac53 opened 8 months ago

jakemac53 commented 8 months ago

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 commented 6 months ago

@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 commented 6 months ago

Yeah, I'll get on it today.

munificent commented 6 months ago

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 commented 6 months ago

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

This is using analyzer 6.3.0.

I have a PR out for the macro modifier: https://github.com/dart-lang/dart_style/pull/1357

scheglov commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 2 months ago

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 commented 1 month ago

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).