Make the formatter aware of the language version of what it's formatting

[munificent]

Dart format is moving to a new "tall" style designed to be more readable for the kind of deeply nested declarative code that is common in Flutter (#1253). When users install a new Dart SDK, they don't generally expect the formatter to radically change the style of their already-formatted code, and we don't want that to happen when the new style ships either.

We've discussed a few ways to let users control when they adopt the new tall style, and the best approach we've been able to come up with is to tie it to language version. Let's say that dart format support for the tall style ships in Dart SDK 3.x.0. The idea is:

• Any Dart file whose language version is 3.x or greater will automatically be formatted using the new tall style.

• Any Dart file whose language version is older than 3.x will continue to use the current "short" style.

This means that by default, when you update to a new Flutter or Dart SDK, your formatting won't change. It's only when you change the Dart SDK constraint in your pubspec to opt into the new language version that you'll also get a formatting change. (We also intend to support some explicit way to opt in to the new style for older language versioned files.)

To implement that, dart_style needs to know the language version of everything it's formatting. Currently, it doesn't. This issue is to track adding support for that. It may need some elaboration, but the basic idea is:

• The FormatCommand class which drives the dart format command-line tool and accesses the file system directly should determine the language version of each file it processes. This means looking for either a pubspec or package configuration file in surrounding directories to determine the default language version. I'm not sure what the exact logic should be here, but it should probably follow how dart analyze behaves. Ideally, there is some code we can reuse.

• Extend the DartFormatter library API to support passing in a language version for each file. If omitted, default to something reasonable (without hitting the file system). Probably just the most recent language version.

• If a compilation unit being formatted (from the command line or through the DartFormatter library API) has a @dart= language version comment, honor that to determine the style used for that file.

[munificent]

I'm working on this now. One key question is how the formatter should behave when it can't determine the language version of what it's formatting. Cases where that can occur:

• You run dart format on the command line and the file paths you pass in aren't inside a package with a package_config.json file that specifies a default language version.

• You run dart format, pass in the code to format on stdin, and don't specify some sort of --language-version= option.

• You use the DartFormatter library API and don't pass in a language version.

Currently, the formatter's behavior is to always format the code as the latest version. When the tall style ships, defaulting to the latest version means it will also default to the tall style. That might be disruptive to users because the same way they used to format code could spontaneously change the style under them.

Eventually, this won't be a problem. The ecosystem will move to the new style and they won't see any churn if the formatter defaults to a language version that gives them that style. But I don't know if that's the right behavior when we first launch the new style.

Options

I can think of a few options:

Default to the latest language version

This is what I currently proposed. As the formatter does today, if it doesn't know what language version you want, you get the very latest version that it supports. That means that as soon as we ship the tall style, whenever the formatter doesn't know your language version you will also get the tall style.

Require a language version

Because some folks on the team are rightly worried about unexpected format changes with the above option, they have suggested that the formatter should simply not support formatting code at all when it doesn't clearly know the desired language version. That would mean:

• If you run dart format on the command line and the file paths aren't in a package (that you have run pub get on), it errors out unless you pass in some --language-version= option to specify a language version.

• Likewise, if you run dart format and pass in code to format on stdin, you get an error if you don't also pass --language-version=.

• The DartFormatter constructor is changed to have a non-optional parameter for the desired language version. This is a breaking API change.

This avoids any user confusion or surprise about what output they get. But it may be that it simply replaces it with user pain. "Format using the latest version" has worked fine 99.99% of the time for the past decade and likely will again once we're through the style migration, so not allowing that at all feels unpleasant.

Default to the latest short style version

There is a middle path which I hadn't considered until now. Let's say we ship the new tall style in Dart 3.7. That means any code whose language version is 3.7 or higher will get the new style.

We could say that if you run the formatter and it doesn't know the language version you want, it defaults to 3.6. That means any previously formatted short style code will continue to be formatted using the short style. No formatting disruption.

On the other hand it means that if you try to use any new syntax which may appear in Dart 3.7 or later, the formatter will fail because it can't parse the code.

Eventually, I intend to sunset and remove support for the short style entirely, even when formatting older language versions. When that happens, we would change this to behavior to again default to the actual latest version supported by the formatter and not the latest short style version.

Thoughts, @dart-lang/language-team (or anyone else)?

[mit-mit]

I'm pretty sure we already have a "default language version" defined somewhere, which is the language version we use if the dart cli is asked to execute a program that doesn't have a package config (and we thus can't read the it's language version). I think the current default is 2.12?

Wouldn't it be more consistent to use this as the default for formatting too?

[jakemac53]

Wouldn't it be more consistent to use this as the default for formatting too?

Yes, I agree this is absolutely correct behavior for the dart format CLI tool (but not the library). The default is the latest language version.

• The DartFormatter constructor is changed to have a non-optional parameter for the desired language version. This is a breaking API change.

This is the change I was advocating for. It could be nullable still (which means latest version), or it could force the user to pass in the latest language version explicitly for code which doesn't have an explicit version. But it should be explicit, because all existing usages should be passing in a language version when they have one available.

I would suggest releasing this first as an optional parameter, so that packages can migrate to it. Then make it required, with a breaking change. That way the dependency constraint can continue to be ^<version-with-the-optional-arg> for most packages, even after they have migrated, so they get the good behavior regardless of whether all packages in the version solve have migrated. Both of these versions could be released at the same time essentially even, it doesn't really matter. Possibly that constraint could even be suggested in the changelog for the breaking change release.

[munificent]

I would suggest releasing this first as an optional parameter, so that packages can migrate to it.

In this release, would it always apply the short style if you omit the parameter?

[jakemac53]

In this release, would it always apply the short style if you omit the parameter?

There is no right answer here... but I would probably do the long style. You could set a minimum SDK for the release, which will guarantee only users which might have a package on the latest version would get this behavior, and users on old SDKs will not get weird formatting. But it is really a toss up what you choose, you have no signal to differentiate.

[jakemac53]

Fwiw, I would expect the vast majority of people hitting this will be the ones using build_runner, and specifically users of code generators built on the source_gen package, which does formatting by default.

Just fixing that one package will probably handle most of the cases.

[natebosch]

• +1 to defaulting to latest for dart format.
• +1 to adding an optional argument for DartFormatter and making it required in a breaking release.

I'd be happy either way for

• switching DartFormatter to use tall style by default before the breaking release
• making the required argument nullable to default to latest

[munificent]

OK, I have the DartFormatter class taking a currently optional parameter for the language version. If passed, it uses that language version for parsing. If omitted, it defaults to the latest supported version. In a future commit, I'll bump the major version of dart_style and make that parameter mandatory.

The Dart CLI will search the file system for a surrounding package config and use that to infer the language version of the file. You can also pass --language-version to disable that file system behavior and use a specified version. Currently, this behavior is behind the "tall-style" experiment flag and off by default.

The hack to try to parse at the latest version and fall back to a pre-patterns version if that fails is still in there. But it only kicks in if the language version is omitted. Once the version is mandatory in DartFormatter, I can remove it.

The remaining task is to coordinate with analyzer and other folks using the dart_style library API to get them to start passing in a language version. But first I'll need to publish a new version of dart_style with language version support and roll it into the Dart SDK.

[jakemac53]

In a future commit, I'll bump the major version of dart_style and make that parameter mandatory.

Fwiw I do think this should be landed prior to the new formatter being shipped in a stable release, if possible. This is just because I have an assumption that nobody outside the people on this thread will know about or use this new parameter until it is required. That does however cover source_gen, so hopefully most cases will still do the right thing. But the more people get ahead of providing this the better.

[lrhn]

The Dart CLI will search the file system for a surrounding package config and use that to infer the language version of the file.

We could do this for passing code into stdin too, based on CWD instead of the file path. If you are working inside a Pub package directory, the CLI can treat that Pub package's package_config.json as the default for code-with-no-path. It's probably part of the current package, otherwise why would you be there.
(But requiring an explicit language version is probably safer, even if the input ends up having a //@dart=3.17 marker. The input in stdin should still be assumed to be a complete file, because maybe it is, so it can contain a leading //@dart=... marker.)

[munificent]

Fwiw I do think this should be landed prior to the new formatter being shipped in a stable release, if possible.

I think it's harmless to publish a patch release of dart_style with support for the optional language version parameter. It behaves essentially the same way dart_style currently behaves, but it does give users the ability to pass in a version if they want and start migrating to get their code ready for that parameter being mandatory.

This is just because I have an assumption that nobody outside the people on this thread will know about or use this new parameter until it is required.

Maybe, but I'm always surprised to discover how many users are out there using our stuff in unexpected ways.

[munificent]

We could do this for passing code into stdin too, based on CWD instead of the file path. If you are working inside a Pub package directory, the CLI can treat that Pub package's package_config.json as the default for code-with-no-path. It's probably part of the current package, otherwise why would you be there.

I think using CWD might be surprising, but I'm not sure. My current plan is to just make it mandatory so that users are explicit about what they want. In practice, I don't think stdin support is very widely used. I think maybe the Vim integration relies on it?

(But requiring an explicit language version is probably safer, even if the input ends up having a //@dart=3.17 marker. The input in stdin should still be assumed to be a complete file, because maybe it is, so it can contain a leading //@dart=... marker.)

Yeah, explicit is clearer. The // @dart= comment in the input will override whatever language version the user passes in.

[jakemac53]

I think it's harmless to publish a patch release of dart_style with support for the optional language version parameter.

I agree publishing the patch release is harmless (actually, purely has positive value).

I also think that publishing the breaking change has positive value though. It will signal to people that there is something to be done.

As a package maintainer with many transitive deps, why would you look at the changelog for a patch release in one of those deps? There is no motivation for doing so... I just don't see how anybody would reasonably know they need to update here. And they do need to update, if they don't weirdness will happen. That weirdness will mostly occur once the new version of the formatter ships in a stable SDK, and people start opting into the new style. So, that is why I say the breaking change should be published prior to the formatter shipping in a stable SDK, so that package authors become aware of this change (by seeing the new major version, which requires action on their part to allow).

[munificent]

So, that is why I say the breaking change should be published prior to the formatter shipping in a stable SDK, so that package authors become aware of this change (by seeing the new major version, which requires action on their part to allow).

Yeah, I do intend to publish dart_style with a major version bump before the Dart SDK ships on stable.

[natebosch]

I don't think stdin support is very widely used. I think maybe the Vim integration relies on it?

The vim plugin does use stdin, and it has no understanding of language versions. I prefer formatting through stdin so we don't have to force a file write before formatting.

Can we add some argument to make this easier? What do you think about something like --stdin-file-path?

Edit:

I think we added --stdin-name for vim and the plugin is currently passing the file path to that argument. WDYT about checking if --stdin-name points to a file, and checking the language version from there?

[munificent]

The vim plugin does use stdin, and it has no understanding of language versions. I prefer formatting through stdin so we don't have to force a file write before formatting.

Can we add some argument to make this easier? What do you think about something like --stdin-file-path?

Edit:

I think we added --stdin-name for vim and the plugin is currently passing the file path to that argument. WDYT about checking if --stdin-name points to a file, and checking the language version from there?

Having --stdin-name hit the file system by default feels a little weird to me, but I'd be up for maybe --stdin-path? Want to file a separate issue for this?

[munificent]

I moved the task of coordinating with the analyzer and other teams to pass language versions to the formatter up into the main implementation issue (#1403), so this is done.