Allow passing the language version in to the DartFormatter API. (#1506)

For users of the dart_style library API, this lets them control which
language version formatted files are parsed as. Currently, the
constructor parameter is optional and defaults to the latest supported
version. In a future major release, it will become required.

Right now, the language version affects how the formatted code is
parsed, but does not affect the applied style. When the tall-style
experiment flag ships, any language version later than the Dart SDK
version that the tall style ships in which get the tall style applied.

Author: munificent

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## 2.3.7-wip
 
+* Allow passing a language version to `DartFomatter()`. Formatted code will be
+  parsed at that version. If omitted, defaults to the latest version. In a
+  future release, this parameter will become required.
 * Remove temporary work around for analyzer 6.2.0 from dart_style 2.3.6.
 * Require `package:analyzer` `>=6.5.0 <7.0.0`.
 

benchmark/run.dart

@@ -18,7 +18,6 @@ import 'package:dart_style/src/short/source_visitor.dart';
 import 'package:dart_style/src/testing/benchmark.dart';
 import 'package:dart_style/src/testing/test_file.dart';
 import 'package:path/path.dart' as p;
-import 'package:pub_semver/pub_semver.dart';
 
 /// The number of trials to run before measuring results.
 const _warmUpTrials = 100;
@@ -126,7 +125,8 @@ List<double> _runTrials(String verb, Benchmark benchmark, int trials) {
   var parseResult = parseString(
       content: source.text,
       featureSet: FeatureSet.fromEnableFlags2(
-          sdkLanguageVersion: Version(3, 3, 0), flags: const []),
+          sdkLanguageVersion: DartFormatter.latestLanguageVersion,
+          flags: const []),
       path: source.uri,
       throwIfDiagnostics: false);
 

lib/src/dart_formatter.dart

@@ -25,6 +25,22 @@ import 'string_compare.dart' as string_compare;
 
 /// Dart source code formatter.
 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);
+
+  /// 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
@@ -45,7 +61,15 @@ class DartFormatter {
   /// See dart.dev/go/experiments for details.
   final List<String> experimentFlags;
 
-  /// Creates a new formatter for Dart code.
+  /// 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
@@ -56,12 +80,15 @@ class DartFormatter {
   ///
   /// While formatting, also applies any of the given [fixes].
   DartFormatter(
-      {this.lineEnding,
+      {Version? languageVersion,
+      this.lineEnding,
       int? pageWidth,
       int? indent,
       Iterable<StyleFix>? fixes,
       List<String>? experimentFlags})
-      : pageWidth = pageWidth ?? 80,
+      : languageVersion = languageVersion ?? latestLanguageVersion,
+        _omittedLanguageVersion = languageVersion == null,
+        pageWidth = pageWidth ?? 80,
         indent = indent ?? 0,
         fixes = {...?fixes},
         experimentFlags = [...?experimentFlags];
@@ -72,18 +99,15 @@ class DartFormatter {
   /// If [uri] is given, it is a [String] or [Uri] used to identify the file
   /// being formatted in error messages.
   String format(String source, {Object? uri}) {
-    if (uri == null) {
-      // Do nothing.
-    } else if (uri is Uri) {
-      uri = uri.toString();
-    } else if (uri is String) {
-      // Do nothing.
-    } else {
-      throw ArgumentError('uri must be `null`, a Uri, or a String.');
-    }
+    var uriString = switch (uri) {
+      null => null,
+      Uri() => uri.toString(),
+      String() => uri,
+      _ => throw ArgumentError('uri must be `null`, a Uri, or a String.'),
+    };
 
     return formatSource(
-            SourceCode(source, uri: uri as String?, isCompilationUnit: true))
+            SourceCode(source, uri: uriString, isCompilationUnit: true))
         .text;
   }
 
@@ -118,13 +142,26 @@ class DartFormatter {
     }
 
     // Parse it.
-    var parseResult = _parse(text, source.uri, patterns: true);
-
-    // If we couldn't parse it with patterns enabled, it may be because of
-    // one of the breaking syntax changes to switch cases. Try parsing it
-    // again without patterns.
-    if (parseResult.errors.isNotEmpty) {
-      var withoutPatternsResult = _parse(text, source.uri, patterns: false);
+    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) {
@@ -197,31 +234,8 @@ class DartFormatter {
     return output;
   }
 
-  /// Parse [source] from [uri].
-  ///
-  /// If [patterns] is `true`, the parse at the latest language version
-  /// which supports patterns and treats switch cases as patterns. If `false`,
-  /// then parses using an older language version where switch cases are
-  /// constant expressions.
-  ///
-  // TODO(rnystrom): This is a pretty big hack. Up until now, every language
-  // version was a strict syntactic superset of all previous versions. That let
-  // the formatter parse every file at the latest language version without
-  // having to detect each file's actual version, which requires digging around
-  // in the file system for package configs and looking for "@dart" comments in
-  // files. It also means the library API that parses arbitrary strings doesn't
-  // have to worry about what version the code should be interpreted as.
-  //
-  // But with patterns, a small number of switch cases are no longer
-  // syntactically valid. Breakage from this is very rare. Instead of adding
-  // the machinery to detect language versions (which is likely to be slow and
-  // brittle), we just try parsing everything with patterns enabled. When a
-  // parse error occurs, we try parsing it again with pattern disabled. If that
-  // happens to parse without error, then we use that result instead.
-  ParseStringResult _parse(String source, String? uri,
-      {required bool patterns}) {
-    var version = patterns ? Version(3, 3, 0) : Version(2, 19, 0);
-
+  /// 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);

test/dart_formatter_test.dart

@@ -0,0 +1,171 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+import 'package:dart_style/dart_style.dart';
+import 'package:pub_semver/pub_semver.dart';
+import 'package:test/test.dart';
+
+void main() async {
+  group('language version', () {
+    test('defaults to latest if omitted', () {
+      var formatter = DartFormatter();
+      expect(formatter.languageVersion, DartFormatter.latestLanguageVersion);
+    });
+
+    test('defaults to latest if null', () {
+      var formatter = DartFormatter(languageVersion: null);
+      expect(formatter.languageVersion, DartFormatter.latestLanguageVersion);
+    });
+
+    test('parses at given older language version', () {
+      // Use a language version before patterns were supported and a pattern
+      // is an error.
+      var formatter = DartFormatter(languageVersion: Version(2, 19, 0));
+      expect(() => formatter.format('main() {switch (o) {case var x: break;}}'),
+          throwsA(isA<FormatterException>()));
+    });
+
+    test('parses at given newer language version', () {
+      // Use a language version after patterns were supported and `1 + 2` is an
+      // error.
+      var formatter = DartFormatter(languageVersion: Version(3, 0, 0));
+      expect(() => formatter.format('main() {switch (o) {case 1+2: break;}}'),
+          throwsA(isA<FormatterException>()));
+    });
+    test('@dart comment overrides version', () {
+      // Use a language version after patterns were supported and `1 + 2` is an
+      // error.
+      var formatter = DartFormatter(languageVersion: Version(3, 0, 0));
+
+      // But then have the code opt to the older version.
+      const before = '''
+// @dart=2.19
+main() { switch (o) { case 1+2: break; } }
+''';
+
+      const after = '''
+// @dart=2.19
+main() {
+  switch (o) {
+    case 1 + 2:
+      break;
+  }
+}
+''';
+
+      expect(formatter.format(before), after);
+    });
+  });
+
+  test('throws a FormatterException on failed parse', () {
+    var formatter = DartFormatter();
+    expect(() => formatter.format('wat?!'), throwsA(isA<FormatterException>()));
+  });
+
+  test('FormatterException.message() does not throw', () {
+    // This is a regression test for #358 where an error whose position is
+    // past the end of the source caused FormatterException to throw.
+    expect(
+        () => DartFormatter().format('library'),
+        throwsA(isA<FormatterException>().having(
+            (e) => e.message(), 'message', contains('Could not format'))));
+  });
+
+  test('FormatterException describes parse errors', () {
+    expect(() {
+      DartFormatter().format('''
+
+      var a = some error;
+
+      var b = another one;
+      ''', uri: 'my_file.dart');
+
+      fail('Should throw.');
+    },
+        throwsA(isA<FormatterException>().having(
+            (e) => e.message(),
+            'message',
+            allOf(contains('Could not format'), contains('line 2'),
+                contains('line 4')))));
+  });
+
+  test('adds newline to unit', () {
+    expect(DartFormatter().format('var x = 1;'), equals('var x = 1;\n'));
+  });
+
+  test('adds newline to unit after trailing comment', () {
+    expect(DartFormatter().format('library foo; //zamm'),
+        equals('library foo; //zamm\n'));
+  });
+
+  test('removes extra newlines', () {
+    expect(DartFormatter().format('var x = 1;\n\n\n'), equals('var x = 1;\n'));
+  });
+
+  test('does not add newline to statement', () {
+    expect(DartFormatter().formatStatement('var x = 1;'), equals('var x = 1;'));
+  });
+
+  test('fails if anything is after the statement', () {
+    expect(
+        () => DartFormatter().formatStatement('var x = 1;;'),
+        throwsA(isA<FormatterException>()
+            .having((e) => e.errors.length, 'errors.length', equals(1))
+            .having((e) => e.errors.first.offset, 'errors.length.first.offset',
+                equals(10))));
+  });
+
+  test('preserves initial indent', () {
+    var formatter = DartFormatter(indent: 3);
+    expect(
+        formatter.formatStatement('if (foo) {bar;}'),
+        equals('   if (foo) {\n'
+            '     bar;\n'
+            '   }'));
+  });
+
+  group('line endings', () {
+    test('uses given line ending', () {
+      // Use zero width no-break space character as the line ending. We have
+      // to use a whitespace character for the line ending as the formatter
+      // will throw an error if it accidentally makes non-whitespace changes
+      // as will occur
+      var lineEnding = '\t';
+      expect(DartFormatter(lineEnding: lineEnding).format('var i = 1;'),
+          equals('var i = 1;\t'));
+    });
+
+    test('infers \\r\\n if the first newline uses that', () {
+      expect(DartFormatter().format('var\r\ni\n=\n1;\n'),
+          equals('var i = 1;\r\n'));
+    });
+
+    test('infers \\n if the first newline uses that', () {
+      expect(DartFormatter().format('var\ni\r\n=\r\n1;\r\n'),
+          equals('var i = 1;\n'));
+    });
+
+    test('defaults to \\n if there are no newlines', () {
+      expect(DartFormatter().format('var i =1;'), equals('var i = 1;\n'));
+    });
+
+    test('handles Windows line endings in multiline strings', () {
+      expect(
+          DartFormatter(lineEnding: '\r\n').formatStatement('  """first\r\n'
+              'second\r\n'
+              'third"""  ;'),
+          equals('"""first\r\n'
+              'second\r\n'
+              'third""";'));
+    });
+  });
+
+  test('throws an UnexpectedOutputException on non-whitespace changes', () {
+    // Use an invalid line ending character to ensure the formatter will
+    // attempt to make non-whitespace changes.
+    var formatter = DartFormatter(lineEnding: '%');
+    expect(() => formatter.format('var i = 1;'),
+        throwsA(isA<UnexpectedOutputException>()));
+  });
+}