Skip to content

Commit

Permalink
Remove support for fixes and --fix. (#1564)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
munificent authored Sep 11, 2024
1 parent 8afa44a commit 87e527d
Show file tree
Hide file tree
Showing 28 changed files with 104 additions and 1,600 deletions.
34 changes: 33 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
## 3.0.0-wip

* Make the language version parameter to `DartFormatter()` mandatory.
* **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`.

## 2.3.7

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
14 changes: 0 additions & 14 deletions bin/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'package:dart_style/src/cli/output.dart';
import 'package:dart_style/src/cli/show.dart';
import 'package:dart_style/src/cli/summary.dart';
import 'package:dart_style/src/io.dart';
import 'package:dart_style/src/short/style_fix.dart';

void main(List<String> args) async {
var parser = ArgParser(allowTrailingOptions: true);
Expand Down Expand Up @@ -108,18 +107,6 @@ void main(List<String> args) async {

var followLinks = argResults['follow-links'] as bool;

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) {
usageError(parser, '--fix-${fix.name} is redundant with --fix.');
}

fixes.add(fix);
}
}

if (argResults.wasParsed('stdin-name') && argResults.rest.isNotEmpty) {
usageError(parser, 'Cannot pass --stdin-name when not reading from stdin.');
}
Expand All @@ -128,7 +115,6 @@ void main(List<String> args) async {
indent: indent,
pageWidth: pageWidth,
followLinks: followLinks,
fixes: fixes,
show: show,
output: output,
summary: summary,
Expand Down
1 change: 0 additions & 1 deletion example/format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ Future<void> _runTest(String path, int line,
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';
14 changes: 0 additions & 14 deletions lib/src/cli/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,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 'options.dart';
import 'output.dart';
Expand Down Expand Up @@ -116,18 +115,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 @@ -160,7 +147,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
11 changes: 0 additions & 11 deletions lib/src/cli/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:args/args.dart';

import '../short/style_fix.dart';

void defineOptions(ArgParser parser,
{bool oldCli = false, bool verbose = false}) {
if (oldCli) {
Expand Down Expand Up @@ -78,15 +76,6 @@ void defineOptions(ArgParser parser,
negatable: false,
help: 'Return exit code 1 if there are any formatting changes.');

if (verbose) parser.addSeparator('Non-whitespace fixes (off by default):');
parser.addFlag('fix',
negatable: false, help: 'Apply all style fixes.', hide: !verbose);

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

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

parser.addOption('line-length',
Expand Down
10 changes: 1 addition & 9 deletions lib/src/dart_formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,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 Down Expand Up @@ -46,8 +45,6 @@ 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.
Expand All @@ -61,18 +58,14 @@ class DartFormatter {
///
/// 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(
{required this.languageVersion,
this.lineEnding,
int? pageWidth,
int? indent,
Iterable<StyleFix>? fixes,
List<String>? experimentFlags})
: pageWidth = pageWidth ?? 80,
indent = indent ?? 0,
fixes = {...?fixes},
experimentFlags = [...?experimentFlags];

/// Formats the given [source] string containing an entire Dart compilation
Expand Down Expand Up @@ -193,8 +186,7 @@ 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);
}

Expand Down
2 changes: 0 additions & 2 deletions lib/src/io.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Future<void> formatStdin(
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 @@ -181,7 +180,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 87e527d

Please sign in to comment.