Consider disabling formatting in generated files by default

[rrousselGit]

Formatting is fairly expensive for code-generation. Although it's useful to make badly generated code more readable, it has a big cost.

Consider

• have dartfmt not check generated files by default (by ignoring .g.dart?)
• have build not format those
• suggest builder authors to have their generated code be humanly readable.

[davidmorgan]

Thanks! Yes, we've noticed formatting can be a nontrivial cost.

There is a trick here: by far the most important case is if the generated code hits a bad performance case in "short style" dartfmt. You can tweak the generated code to make it much, much faster. Usually this means adding some trailing commas.

Here is that fix on build_value. I'd like to check if common generators have the same problem, but haven't gotten around to it. In any case it stops being a problem with the Dart 3.7 "tall style" which just adds and removes commas as it likes, so it will become much less important.

Once that's taken care of, formatting is a very small part of build_runner cost, so it doesn't need optimizing yet. But when build_runner is fast it might be interesting. Formatting parallelizes well, so I think we can reach a state where we do format everything and it's fast.

[jakemac53]

As far as not formatting generated files I believe the formatter supports some way of opting out files (or at least regions of files) now? cc @munificent to confirm if that is a thing...

If we had that then the guidance should just be for builders to opt out their own code using that.

[rrousselGit]

If we go for this, I think it'd be worth to change the default behaviour and have dart format not check any *.*.dart

The fewer configuration folks have to do, the better.

[davidmorgan]

I think the end state we're aiming for is that everything is formatted and everything is fast.

If generated code doesn't hit dartfmt bugs, it can be fast; if it's not fast enough, it can run in a different process.

And: if code has already been formatted, such as generated output on disk, there's no need to format it again :)

[jakemac53]

If we go for this, I think it'd be worth to change the default behaviour and have dart format not check any *.*.dart

I disagree, I think it would be very weird for the formatter to bake in assumptions like this. I have definitely seen people use multiple file extensions before for non-generated files.

It should just be configured, but by builder authors and not all users, so there isn't a lot of toil involved.

[rrousselGit]

If formatting should be disabled by default for generated files, then we need a way to tell dartfmt not to fail on those. If excluding *.*.dart isn't good, then we could have it detect other things. Like:

@generated
library;

Or:

// ignore_for_file: dartfmt

Or something else included in the file to notify that it's generated.

If the formatter doesn't exclude generated files by default, then generated files should be formatted by default. I see no in-between. The two tools should have a matching default behaviour IMO.

[jakemac53]

If formatting should be disabled by default for generated files, then we need a way to tell dartfmt not to fail on those.

Yes this is exactly what I said above, I believe it already supports opting out a file and/or region. But if not we should try and push on that.

I think it would be ideal actually for all builders to always add this, because even if they do format their output you don't want the command line formatter also trying to format it (technically, they could be different versions).

[rrousselGit]

We agree then. My point was more about having build/source_gen disable formatting themselves. I think a convention here is important.

[jakemac53]

The source_gen combined builder should definitely just do this by default, which will cover many use cases, and we can document for the custom use cases that it is best practice to do this for sure.

[munificent]

Yes this is exactly what I said above, I believe it already supports opting out a file and/or region.

Yes, the syntax looks like:

// dart format off
nothing + here + gets + formatted;
// dart format on

If you want to disable formatting the whole file, you can just put // dart format off at the top and not close it at the end.

Note that this only works with the new short style, so it will only help generated code whose target language version is 3.7 or higher.

[davidmorgan]

I currently suspect that the right way to do this is to stop doing formatting in builders and leave it to build_runner.

Then it can take care of when to run the formatter; it can also arrange that it happens in parallel instead of blocking generation.

Planning to take a look at this soon as part of the next batch of performance improvements.