Skip to content

Commit

Permalink
Merge branch 'main' into remove-old-cli
Browse files Browse the repository at this point in the history
# Conflicts:
#	CHANGELOG.md
#	bin/format.dart
#	lib/src/cli/options.dart
#	test/cli_test.dart
#	test/command_line_test.dart
  • Loading branch information
munificent committed Sep 11, 2024
2 parents 3075b76 + 87e527d commit 6d0cdb3
Show file tree
Hide file tree
Showing 30 changed files with 152 additions and 1,601 deletions.
50 changes: 41 additions & 9 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,46 @@
## 3.0.0-wip

* **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.
* **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
apply a few fixes and hasn't been maintained in many years. The `dart fix`
command is actively maintained, can apply all of the fixes that
`dart format --fix` could apply and many many more.

In order to avoid duplicate engineering effort, we decided to consolidate on
`dart fix` as the one way to make automated changes that go beyond the simple
formatting and style changes that `dart format` applies.

The ability to apply fixes is also removed from the `DartFormatter()` library
API.

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

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.

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

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 Down
23 changes: 0 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,6 @@ if (tag == 'style' ||
The formatter will never break your code—you can safely invoke it
automatically from build and presubmit scripts.

## Style fixes

The formatter can also apply non-whitespace changes to make your code
consistently idiomatic. You must opt into these by passing either `--fix` which
applies all style fixes, or any of the `--fix-`-prefixed flags to apply specific
fixes.

For example, running with `--fix-named-default-separator` changes this:

```dart
greet(String name, {String title: "Captain"}) {
print("Greetings, $title $name!");
}
```

into:

```dart
greet(String name, {String title = "Captain"}) {
print("Greetings, $title $name!");
}
```

## Using the formatter

The formatter is part of the unified [`dart`][] developer tool included in the
Expand Down
1 change: 1 addition & 0 deletions benchmark/directory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ void main(List<String> arguments) async {
void _runFormatter(String source) {
try {
var formatter = DartFormatter(
languageVersion: DartFormatter.latestLanguageVersion,
experimentFlags: [if (!_isShort) tallStyleExperimentFlag]);

var result = formatter.format(source);
Expand Down
1 change: 1 addition & 0 deletions benchmark/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ List<double> _runTrials(String verb, Benchmark benchmark, int trials) {
throwIfDiagnostics: false);

var formatter = DartFormatter(
languageVersion: DartFormatter.latestLanguageVersion,
pageWidth: benchmark.pageWidth,
lineEnding: '\n',
experimentFlags: [if (!_isShort) tallStyleExperimentFlag]);
Expand Down
3 changes: 2 additions & 1 deletion example/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ 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]);

Expand Down Expand Up @@ -72,9 +73,9 @@ Future<void> _runTest(String path, int line,
var formatTest = testFile.tests.firstWhere((test) => test.line == line);

var formatter = DartFormatter(
languageVersion: formatTest.languageVersion,
pageWidth: testFile.pageWidth,
indent: formatTest.leadingIndent,
fixes: formatTest.fixes,
experimentFlags: tall
? const ['inline-class', tallStyleExperimentFlag]
: const ['inline-class']);
Expand Down
1 change: 0 additions & 1 deletion lib/dart_style.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@
// BSD-style license that can be found in the LICENSE file.
export 'src/dart_formatter.dart';
export 'src/exceptions.dart';
export 'src/short/style_fix.dart';
export 'src/source_code.dart';
26 changes: 0 additions & 26 deletions lib/src/cli/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:pub_semver/pub_semver.dart';

import '../dart_formatter.dart';
import '../io.dart';
import '../short/style_fix.dart';
import 'formatter_options.dart';
import 'output.dart';
import 'show.dart';
Expand Down Expand Up @@ -77,18 +76,6 @@ class FormatCommand extends Command<int> {
negatable: false,
help: 'Return exit code 1 if there are any formatting changes.');

if (verbose) {
argParser.addSeparator('Non-whitespace fixes (off by default):');
}

argParser.addFlag('fix',
negatable: false, help: 'Apply all style fixes.', hide: !verbose);

for (var fix in StyleFix.all) {
argParser.addFlag('fix-${fix.name}',
negatable: false, help: fix.description, hide: !verbose);
}

if (verbose) argParser.addSeparator('Other options:');

argParser.addOption('line-length',
Expand Down Expand Up @@ -211,18 +198,6 @@ class FormatCommand extends Command<int> {
'"${argResults['indent']}".');
}

var fixes = <StyleFix>[];
if (argResults['fix'] as bool) fixes.addAll(StyleFix.all);
for (var fix in StyleFix.all) {
if (argResults['fix-${fix.name}'] as bool) {
if (argResults['fix'] as bool) {
usageException('--fix-${fix.name} is redundant with --fix.');
}

fixes.add(fix);
}
}

List<int>? selection;
try {
selection = _parseSelection(argResults, 'selection');
Expand Down Expand Up @@ -255,7 +230,6 @@ class FormatCommand extends Command<int> {
indent: indent,
pageWidth: pageWidth,
followLinks: followLinks,
fixes: fixes,
show: show,
output: output,
summary: summary,
Expand Down
8 changes: 1 addition & 7 deletions lib/src/cli/formatter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:io';

import 'package:pub_semver/pub_semver.dart';

import '../short/style_fix.dart';
import '../source_code.dart';
import 'output.dart';
import 'show.dart';
Expand All @@ -31,9 +30,6 @@ class FormatterOptions {
/// Whether symlinks should be traversed when formatting a directory.
final bool followLinks;

/// The style fixes to apply while formatting.
final List<StyleFix> fixes;

/// Which affected files should be shown.
final Show show;

Expand All @@ -55,14 +51,12 @@ class FormatterOptions {
this.indent = 0,
this.pageWidth = 80,
this.followLinks = false,
Iterable<StyleFix>? fixes,
this.show = Show.changed,
this.output = Output.write,
this.summary = Summary.none,
this.setExitIfChanged = false,
List<String>? experimentFlags})
: fixes = [...?fixes],
experimentFlags = [...?experimentFlags];
: experimentFlags = [...?experimentFlags];

/// Called when [file] is about to be formatted.
///
Expand Down
85 changes: 14 additions & 71 deletions lib/src/dart_formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import 'dart:math' as math;

import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
Expand All @@ -19,7 +18,6 @@ import 'constants.dart';
import 'exceptions.dart';
import 'front_end/ast_node_visitor.dart';
import 'short/source_visitor.dart';
import 'short/style_fix.dart';
import 'source_code.dart';
import 'string_compare.dart' as string_compare;

Expand All @@ -29,18 +27,11 @@ class DartFormatter {
/// version of the formatter.
static final latestLanguageVersion = Version(3, 3, 0);

/// The highest Dart language version without support for patterns.
static final _lastNonPatternsVersion = Version(2, 19, 0);

/// The Dart language version that formatted code should be parsed as.
///
/// Note that a `// @dart=` comment inside the code overrides this.
final Version languageVersion;

/// Whether the user passed in a non-`null` language version.
// TODO(rnystrom): Remove this when the language version is required.
final bool _omittedLanguageVersion;

/// The string that newlines should use.
///
/// If not explicitly provided, this is inferred from the source text. If the
Expand All @@ -54,43 +45,27 @@ class DartFormatter {
/// The number of characters of indentation to prefix the output lines with.
final int indent;

final Set<StyleFix> fixes;

/// Flags to enable experimental language features.
///
/// See dart.dev/go/experiments for details.
final List<String> experimentFlags;

/// Creates a new formatter for Dart code at [languageVersion].
///
/// If [languageVersion] is omitted, then it defaults to
/// [latestLanguageVersion]. In a future major release of dart_style, the
/// language version will affect the applied formatting style. At that point,
/// this parameter will become required so that the applied style doesn't
/// change unexpectedly. It is optional now so that users can migrate to
/// versions of dart_style that accept this parameter and be ready for the
/// major version when it's released.
///
/// If [lineEnding] is given, that will be used for any newlines in the
/// output. Otherwise, the line separator will be inferred from the line
/// endings in the source file.
///
/// If [indent] is given, that many levels of indentation will be prefixed
/// before each resulting line in the output.
///
/// While formatting, also applies any of the given [fixes].
DartFormatter(
{Version? languageVersion,
{required this.languageVersion,
this.lineEnding,
int? pageWidth,
int? indent,
Iterable<StyleFix>? fixes,
List<String>? experimentFlags})
: languageVersion = languageVersion ?? latestLanguageVersion,
_omittedLanguageVersion = languageVersion == null,
pageWidth = pageWidth ?? 80,
: pageWidth = pageWidth ?? 80,
indent = indent ?? 0,
fixes = {...?fixes},
experimentFlags = [...?experimentFlags];

/// Formats the given [source] string containing an entire Dart compilation
Expand Down Expand Up @@ -141,33 +116,18 @@ 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);

// Parse it.
var parseResult = _parse(text, source.uri, languageVersion);

// If we couldn't parse it, and the language version supports patterns, it
// may be because of the breaking syntax changes to switch cases. Try
// parsing it again without pattern support.
// TODO(rnystrom): This is a pretty big hack. Before Dart 3.0, every
// language version was a strict syntactic superset of all previous
// versions. When patterns were added, a small number of switch cases
// became syntax errors.
//
// For most of its history, the formatter simply parsed every file at the
// latest language version without having to detect each file's actual
// version. We are moving towards requiring the language version when
// formatting, but for now, try to degrade gracefully if the user omits the
// version.
//
// Remove this when the languageVersion constructor parameter is required.
if (_omittedLanguageVersion && parseResult.errors.isNotEmpty) {
var withoutPatternsResult =
_parse(text, source.uri, _lastNonPatternsVersion);

// If we succeeded this time, use this parse instead.
if (withoutPatternsResult.errors.isEmpty) {
parseResult = withoutPatternsResult;
}
}
var parseResult = parseString(
content: text,
featureSet: featureSet,
path: source.uri,
throwIfDiagnostics: false);

// Infer the line ending if not given one. Do it here since now we know
// where the lines start.
Expand Down Expand Up @@ -226,27 +186,10 @@ class DartFormatter {
}

// Sanity check that only whitespace was changed if that's all we expect.
if (fixes.isEmpty &&
!string_compare.equalIgnoringWhitespace(source.text, output.text)) {
if (!string_compare.equalIgnoringWhitespace(source.text, output.text)) {
throw UnexpectedOutputException(source.text, output.text);
}

return output;
}

/// Parse [source] from [uri] at language [version].
ParseStringResult _parse(String source, String? uri, Version version) {
// Don't pass the formatter's own experiment flag to the parser.
var experiments = experimentFlags.toList();
experiments.remove(tallStyleExperimentFlag);

var featureSet = FeatureSet.fromEnableFlags2(
sdkLanguageVersion: version, flags: experiments);

return parseString(
content: source,
featureSet: featureSet,
path: uri,
throwIfDiagnostics: false);
}
}
5 changes: 2 additions & 3 deletions lib/src/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ Future<void> formatStdin(
var input = StringBuffer();
stdin.transform(const Utf8Decoder()).listen(input.write, onDone: () {
var formatter = DartFormatter(
languageVersion: options.languageVersion,
languageVersion:
options.languageVersion ?? DartFormatter.latestLanguageVersion,
indent: options.indent,
pageWidth: options.pageWidth,
fixes: options.fixes,
experimentFlags: options.experimentFlags);
try {
options.beforeFile(null, name);
Expand Down Expand Up @@ -157,7 +157,6 @@ Future<bool> _processFile(
languageVersion: languageVersion ?? DartFormatter.latestLanguageVersion,
indent: options.indent,
pageWidth: options.pageWidth,
fixes: options.fixes,
experimentFlags: options.experimentFlags);

try {
Expand Down
Loading

0 comments on commit 6d0cdb3

Please sign in to comment.