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

Ship the "tall-style" experiment. #1578

Merged
merged 3 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 99 additions & 19 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,87 @@
## 3.0.0-wip

This is a large change. Under the hood, the formatter was almost completely
rewritten, with the codebase now containing both the old and new
implementations. The old formatter exists to support the older "short" style
and the new code implements [the new "tall" style][tall].

[tall]: https://github.com/dart-lang/dart_style/issues/1253

The formatter uses the language version of the formatted code to determine
which style you get. If the language version is 3.6 or lower, the code is
formatted with the old style. If 3.7 or later, you get the new tall style. You
typically control the language version by [setting a min SDK constraint in your
package's pubspec][versioning].

[versioning]: https://dart.dev/guides/language/evolution

In addition to the new formatting style, a number of other API and CLI changes
are included, some of them breaking:

* **Support project-wide page width configuration.** By long request, you can
now configure your preferred formatting page width on a project-wide basis.
When formatting files, the formatter will look in the file's directory and
any surrounding directories for an `analysis_options.yaml` file. If it finds
one, it looks for the following YAML:

```yaml
formatter:
page_width: 123
```

If it finds a `formatter` key containing a map with a `page_width` key whose
value is an integer, then that is the page width that the file is formatted
using. Since the formatter will walk the surrounding directories until it
finds an `analysis_options.yaml` file, this can be used to globally set the
page width for an entire directory, package, or even collection of packages.

* **Support overriding the page width for a single file.** In code formatted
using the new tall style, you can use a special marker comment to control the
page width that it's formatted using:

```dart
// dart format width=30
main() {
someExpression +
thatSplitsAt30;
}
```

This comment must appear before any code in the file and must match that
format exactly. The width set by the comment overrides the width set by any
surrounding `analysis_options.yaml` file.

This feature is mainly for code generators that generate and immediately
format code but don't know what about any surrounding `analysis_options.yaml`
munificent marked this conversation as resolved.
Show resolved Hide resolved
that might be configuring the page width. By inserting this comment in the
generated code before formatting, it ensures that the code generator's
behavior matches the behavior of `dart format`.

End users should mostly use `analysis_options.yaml` for configuring their
preferred page width (or do nothing and use the default page width of 80).

* **Support opting out a region of code from formatting.** In code formatted
using the new tall style, you can use a pair of special marker comments to
opt a region of code out of automated formatting:

```dart
main() {
this.isFormatted();
// dart format off
no + formatting
+
here;
// dart format on
formatting.isBackOnHere();
}
```

The comments must be exactly `// dart format off` and `// dart format on`.
A file may have multiple regions, but they can't overlap or nest.

This can be useful for highly structured data where custom layout can help
a reader understand the data, like large lists of numbers.

* **Remove support for fixes and `--fix`.** The tools that come with the Dart
SDK provide two ways to apply automated changes to code: `dart format --fix`
and `dart fix`. The former is older and used to be faster. But it can only
Expand All @@ -16,31 +98,29 @@

* **Make the language version parameter to `DartFormatter()` mandatory.** This
way, the formatter always knows what language version the input is intended
to be treated as. Note that a `// @dart=` language version comment if present
overrides the specified language version. You can think of the version passed
to the `DartFormatter()` constructor as a "default" language version which
the file's contents may then override.
to be treated as. Note that a `// @dart=` language version comment, if
present, overrides the specified language version. You can think of the
version passed to the `DartFormatter()` constructor as a "default" language
version which the file's contents may then override.

If you don't particularly care about the version of what you're formatting,
you can pass in `DartFormatter.latestLanguageVersion` to unconditionally get
the latest language version that the formatter supports. Note that doing so
means you will also implicitly opt into the new tall style when that style
becomes available.
means you will also implicitly opt into the new tall style.

This change only affects the library API. When using the formatter from the
command line, you can use `--language-version=` to specify a language version
or pass `--language-version=latest` to use the latest supported version. If
omitted, the formatter will look up the surrounding directories for a package
omitted, the formatter will look in the surrounding directories for a package
config file and infer the language version for the package from that, similar
to how other Dart tools behave like `dart analyze` and `dart run`.

* **Remove the old formatter executables and CLI options.** Before the
`dart format` command was added to the core Dart SDK, users accessed the
formatter by running a separate `dartfmt` executable that was included with
the Dart SDK. That executable had a different CLI interface. For example, you
had to pass `-w` to get it to overwrite files and if you passed no arguments
at all, it silently sat there waiting for input on stdin. When we added
`dart format`, we took that opportunity to revamp the CLI options.
had to pass `-w` to get it to overwrite files. When we added `dart format`,
we took that opportunity to revamp the CLI options.

However, the dart_style package still exposed an executable with the old CLI.
If you ran `dart pub global activate dart_style`, this would give you a
Expand All @@ -64,25 +144,25 @@
`--language-version=`, or use `--language-version=latest` to parse the input
using the latest language version supported by the formatter.

If `--stdin-name` and `--language-version` are both omitted, then parses
stdin using the latest supported language version.

* **Apply class modifiers to API classes.** The dart_style package exposes only
a few classes in its public API: `DartFormatter`, `SourceCode`,
`FormatterException`, and `UnexpectedOutputException`. None were ever
intended to be extended or implemented. They are now all marked `final` to
make that intention explicit.
If `--stdin-name` and `--language-version` are both omitted, then the
formatter parses stdin using the latest supported language version.

* **Rename the `--line-length` option to `--page-width`.** This is consistent
with the public API, internal implementation, and docs, which all use "page
width" to refer to the limit that the formatter tries to fit code into.

The `--line-length` name is still supported for backwards compatibility, but
may be removed at some point in the future. You're encouraged to move to
`--page-width`. Use of this option (however its named) is rare, and will
`--page-width`. Use of this option (however it's named) is rare, and will
likely be even rarer now that project-wide configuration is supported, so
this shouldn't affect many users.

* **Apply class modifiers to API classes.** The dart_style package exposes only
a few classes in its public API: `DartFormatter`, `SourceCode`,
`FormatterException`, and `UnexpectedOutputException`. None were ever
intended to be extended or implemented. They are now all marked `final` to
make that intention explicit.

## 2.3.7

* Allow passing a language version to `DartFomatter()`. Formatted code will be
Expand Down
6 changes: 3 additions & 3 deletions benchmark/directory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'dart:io';
import 'package:args/args.dart';
import 'package:collection/collection.dart';
import 'package:dart_style/dart_style.dart';
import 'package:dart_style/src/constants.dart';
import 'package:dart_style/src/profile.dart';
import 'package:dart_style/src/testing/benchmark.dart';

Expand Down Expand Up @@ -64,8 +63,9 @@ void main(List<String> arguments) async {
void _runFormatter(String source) {
try {
var formatter = DartFormatter(
languageVersion: DartFormatter.latestLanguageVersion,
experimentFlags: [if (!_isShort) tallStyleExperimentFlag]);
languageVersion: _isShort
? DartFormatter.latestShortStyleLanguageVersion
: DartFormatter.latestLanguageVersion);

var result = formatter.format(source);

Expand Down
8 changes: 4 additions & 4 deletions benchmark/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:args/args.dart';
import 'package:dart_style/dart_style.dart';
import 'package:dart_style/src/constants.dart';
import 'package:dart_style/src/debug.dart' as debug;
import 'package:dart_style/src/front_end/ast_node_visitor.dart';
import 'package:dart_style/src/profile.dart';
Expand Down Expand Up @@ -131,10 +130,11 @@ List<double> _runTrials(String verb, Benchmark benchmark, int trials) {
throwIfDiagnostics: false);

var formatter = DartFormatter(
languageVersion: DartFormatter.latestLanguageVersion,
languageVersion: _isShort
? DartFormatter.latestShortStyleLanguageVersion
: DartFormatter.latestLanguageVersion,
pageWidth: benchmark.pageWidth,
lineEnding: '\n',
experimentFlags: [if (!_isShort) tallStyleExperimentFlag]);
lineEnding: '\n');

var measuredTimes = <double>[];
for (var i = 1; i <= trials; i++) {
Expand Down
13 changes: 5 additions & 8 deletions example/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:dart_style/dart_style.dart';
import 'package:dart_style/src/constants.dart';
import 'package:dart_style/src/debug.dart' as debug;
import 'package:dart_style/src/testing/test_file.dart';

Expand Down Expand Up @@ -40,9 +39,10 @@ void _runFormatter(String source, int pageWidth,
{required bool tall, required bool isCompilationUnit}) {
try {
var formatter = DartFormatter(
languageVersion: DartFormatter.latestLanguageVersion,
pageWidth: pageWidth,
experimentFlags: [if (tall) tallStyleExperimentFlag]);
languageVersion: tall
? DartFormatter.latestShortStyleLanguageVersion
: DartFormatter.latestLanguageVersion,
pageWidth: pageWidth);

String result;
if (isCompilationUnit) {
Expand Down Expand Up @@ -75,10 +75,7 @@ Future<void> _runTest(String path, int line,
var formatter = DartFormatter(
languageVersion: formatTest.languageVersion,
pageWidth: testFile.pageWidth,
indent: formatTest.leadingIndent,
experimentFlags: tall
? const ['inline-class', tallStyleExperimentFlag]
: const ['inline-class']);
indent: formatTest.leadingIndent);

var actual = formatter.formatSource(formatTest.input);

Expand Down
3 changes: 1 addition & 2 deletions lib/src/cli/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ final class FormatCommand extends Command<int> {
help: 'Language version of formatted code.\n'
'Use "latest" to parse as the latest supported version.\n'
'Omit to look for a surrounding package config.',
// TODO(rnystrom): Show this when the tall-style experiment ships.
hide: true);
hide: !verbose);

argParser.addFlag('set-exit-if-changed',
negatable: false,
Expand Down
1 change: 1 addition & 0 deletions lib/src/config_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ final class ConfigCache {
// Otherwise, walk the file system and look for it.
var config =
await _findPackageConfig(file, displayPath, forLanguageVersion: true);

if (config?.packageOf(file.absolute.uri)?.languageVersion
case var languageVersion?) {
// Store the version as pub_semver's [Version] type because that's
Expand Down
9 changes: 0 additions & 9 deletions lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

/// The in-progress "tall" formatting style is enabled by passing an experiment
/// flag with this name.
///
/// Note that this isn't a real Dart SDK experiment: Only the formatter supports
/// it. We use the [experimentFlags] API to pass this in so that support for it
/// can be removed in a later version without it being a breaking change to the
/// dart_style library API.
const tallStyleExperimentFlag = 'tall-style';

/// Constants for the cost heuristics used to determine which set of splits is
/// most desirable.
final class Cost {
Expand Down
19 changes: 12 additions & 7 deletions lib/src/dart_formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import 'package:analyzer/src/dart/scanner/scanner.dart';
import 'package:analyzer/src/string_source.dart';
import 'package:pub_semver/pub_semver.dart';

import 'constants.dart';
import 'exceptions.dart';
import 'front_end/ast_node_visitor.dart';
import 'short/source_visitor.dart';
Expand All @@ -30,7 +29,14 @@ final RegExp _widthCommentPattern = RegExp(r'^// dart format width=(\d+)$');
final class DartFormatter {
/// The latest Dart language version that can be parsed and formatted by this
/// version of the formatter.
static final latestLanguageVersion = Version(3, 3, 0);
static final latestLanguageVersion = Version(3, 7, 0);

/// The latest Dart language version that will be formatted using the older
/// "short" style.
///
/// Any Dart code at a language version later than this will be formatted
/// using the new "tall" style.
static final latestShortStyleLanguageVersion = Version(3, 6, 0);

/// The page width that the formatter tries to fit code inside if no other
/// width is specified.
Expand Down Expand Up @@ -125,11 +131,8 @@ final class DartFormatter {
);
}

// Don't pass the formatter's own experiment flag to the parser.
var experiments = experimentFlags.toList();
experiments.remove(tallStyleExperimentFlag);
var featureSet = FeatureSet.fromEnableFlags2(
sdkLanguageVersion: languageVersion, flags: experiments);
sdkLanguageVersion: languageVersion, flags: experimentFlags);

// Parse it.
var parseResult = parseString(
Expand Down Expand Up @@ -185,8 +188,9 @@ final class DartFormatter {
// Format it.
var lineInfo = parseResult.lineInfo;

// Use language version to determine what formatting style to apply.
SourceCode output;
if (experimentFlags.contains(tallStyleExperimentFlag)) {
if (languageVersion > latestShortStyleLanguageVersion) {
// Look for a page width comment before the code.
int? pageWidthFromComment;
for (Token? comment = node.beginToken.precedingComments;
Expand All @@ -203,6 +207,7 @@ final class DartFormatter {
var visitor = AstNodeVisitor(this, lineInfo, unitSourceCode);
output = visitor.run(unitSourceCode, node, pageWidthFromComment);
} else {
// Use the old style.
var visitor = SourceVisitor(this, lineInfo, unitSourceCode);
output = visitor.run(node);
}
Expand Down
Loading