Smarter versioned tests

example/format.dart

@@ -116,8 +116,15 @@ Future<void> _runTest(
   var actualText = actual.textWithSelectionMarkers;
   if (!testFile.isCompilationUnit) actualText += '\n';
 
-  // TODO(rnystrom): Handle multiple outputs.
-  var expectedText = formatTest.outputs.first.code.textWithSelectionMarkers;
+  String expectedText;
+  switch (formatTest) {
+    case UnversionedFormatTest():
+      expectedText = formatTest.output.code.textWithSelectionMarkers;
+    case VersionedFormatTest():
+      // Pick the newest style for the expectation.
+      expectedText =
+          formatTest.outputs.entries.last.value.code.textWithSelectionMarkers;
+  }
 
   print('$path ${formatTest.input.description}');
   _drawRuler('before', pageWidth);

lib/src/testing/test_file.dart

@@ -77,6 +77,7 @@ final class TestFile {
     var lines = file.readAsLinesSync();
 
     var isCompilationUnit = file.path.endsWith('.unit');
+    var isTall = p.split(file.path).contains('tall');
 
     // The first line may have a "|" to indicate the page width.
     var i = 0;
@@ -115,6 +116,7 @@ final class TestFile {
       var lineNumber = i + 1;
       var line = readLine().replaceAll('>>>', '');
       var (options, description) = _parseOptions(line);
+      description = description.trim();
 
       var inputComments = readComments();
       var inputBuffer = StringBuffer();
@@ -127,9 +129,27 @@ final class TestFile {
         isCompilationUnit: isCompilationUnit,
       );
 
-      var input = TestEntry(description.trim(), null, inputComments, inputCode);
+      var input = TestEntry(description, inputComments, inputCode);
+
+      // Read the outputs. A single test should have outputs in one of two
+      // forms:
+      //
+      // - One single unversioned output which is the expected output across
+      //   all supported versions.
+      // - One or more versioned outputs, each of which defines the expected
+      //   output at that language version or later until reaching the next
+      //   output's version.
+      //
+      // The parser here collects all of the outputs, versioned and unversioned
+      // and then reports an error if the result is not one of those two styles.
+      void fail(String error) {
+        throw FormatException(
+          'Test format error in $relativePath, line $lineNumber: $error',
+        );
+      }
 
-      var outputs = <TestEntry>[];
+      var unversionedOutputs = <TestEntry>[];
+      var versionedOutputs = <Version, TestEntry>{};
       while (i < lines.length && lines[i].startsWith('<<<')) {
         var match = _outputPattern.firstMatch(readLine())!;
         var outputDescription = match[4]!;
@@ -166,17 +186,43 @@ final class TestFile {
           isCompilationUnit: isCompilationUnit,
         );
 
-        outputs.add(
-          TestEntry(
-            outputDescription.trim(),
-            outputVersion,
-            outputComments,
-            outputCode,
-          ),
+        var entry = TestEntry(
+          outputDescription.trim(),
+          outputComments,
+          outputCode,
         );
+        if (outputVersion != null) {
+          if (versionedOutputs.containsKey(outputVersion)) {
+            fail('Multiple outputs with the same version $outputVersion.');
+          }
+
+          versionedOutputs[outputVersion] = entry;
+        } else {
+          unversionedOutputs.add(entry);
+        }
       }
 
-      tests.add(FormatTest(lineNumber, options, input, outputs));
+      switch ((unversionedOutputs.length, versionedOutputs.length)) {
+        case (0, 0):
+          fail('Test must have at least one output.');
+        case (0, > 0):
+          tests.add(
+            VersionedFormatTest(lineNumber, options, input, versionedOutputs),
+          );
+        case (1, 0):
+          tests.add(
+            UnversionedFormatTest(
+              lineNumber,
+              options,
+              input,
+              unversionedOutputs.first,
+            ),
+          );
+        case (> 1, 0):
+          fail('Test can\'t have multiple unversioned outputs.');
+        default:
+          fail('Test can\'t have both versioned and unversioned outputs.');
+      }
     }
 
     return TestFile._(
@@ -276,7 +322,7 @@ final class TestFile {
 }
 
 /// A single formatting test inside a [TestFile].
-final class FormatTest {
+sealed class FormatTest {
   /// The 1-based index of the line where this test begins.
   final int line;
 
@@ -286,13 +332,7 @@ final class FormatTest {
   /// The unformatted input.
   final TestEntry input;
 
-  // TODO(rnystrom): Consider making this a map of version (or null) to output
-  // and then validating that there aren't duplicate outputs for a single
-  // version.
-  /// The expected output.
-  final List<TestEntry> outputs;
-
-  FormatTest(this.line, this.options, this.input, this.outputs);
+  FormatTest(this.line, this.options, this.input);
 
   /// The line and description of the test.
   String get label {
@@ -301,20 +341,45 @@ final class FormatTest {
   }
 }
 
+/// A test for formatting that should be the same across all language versions.
+///
+/// Most tests are of this form.
+final class UnversionedFormatTest extends FormatTest {
+  /// The expected output.
+  final TestEntry output;
+
+  UnversionedFormatTest(super.line, super.options, super.input, this.output);
+}
+
+/// A test whose expected formatting changes at specific versions.
+final class VersionedFormatTest extends FormatTest {
+  /// The expected output by version.
+  ///
+  /// Each key is the lowest version where that output is expected. If there are
+  /// supported versions lower than the lowest key here, then the test is not
+  /// run on those versions at all. These tests represent new syntax that isn't
+  /// supported in later versions. For example, if the map has only a single
+  /// entry whose key is 3.8, then the test is skipped on 3.8, run at 3.8, and
+  /// should be valid at any higher version.
+  ///
+  /// If there are multiple entries in the map, they represent versions where
+  /// the formatting style has changed.
+  final Map<Version, TestEntry> outputs;
+
+  VersionedFormatTest(super.line, super.options, super.input, this.outputs);
+}
+
 /// A single test input or output.
 final class TestEntry {
   /// Any remark on the "<<<" or ">>>" line.
   final String description;
 
-  /// If this is a test output for a specific version, the version.
-  final Version? version;
-
   /// The `###` comment lines appearing after the header line before the code.
   final List<String> comments;
 
   final SourceCode code;
 
-  TestEntry(this.description, this.version, this.comments, this.code);
+  TestEntry(this.description, this.comments, this.code);
 }
 
 /// Options for configuring all tests in a file or an individual test.

test/README.md

@@ -37,15 +37,57 @@ description for the test. Lines after that define the input code to be
 formatted.
 
 After the input are one or more output sections. Each output section starts
-with a header like:
+with a header that starts with `<<<`. There are two styles of output:
+
+#### Unversioned output
+
+Most code is supported across all language versions and formats the same way in
+all of them. For those, the output is a single section like:
+
+```
+>>> Optional input description.
+some.code();
+<<< Optional description.
+some.code();
+```
+
+The formatter will run this tests against the oldest and newest supported
+version and verify that both produce that output. (We assume that if it formats
+the same at two versions, it will do so in any version between those for
+performance reasons. Otherwise, every time a new Dart SDK release comes out,
+the number of tests being run increases by thousands.)
+
+#### Versioned outputs
+
+The formatter's behavior may depend on language version for two reasons:
+
+* New syntax was added to the language in a later version, so we can't format
+  it on an older version at all.
+
+* The formatting style changed and we versioned the style change so that code
+  at older versions keeps the older style.
+
+To accommodate those, a test can have multiple output sections which each start
+with a version number like this:
 
 ```
-<<< 3.7 Optional description.
+>>> Optional input description.
+some.code();
+<<< 3.8 Optional description.
+some.code();
+<<< 3.10 Optional description.
+some . code();
 ```
 
-The `<<<` marks the beginning of a new output section. If it has a language
-version number, then this output is expected only on that language version. If
-it has no version number, then this is the expected output on all versions.
+Each output section specifies the minimum version where that output becomes
+expected. The version number of the first section specifies the lowest version
+number that the test will be run at. Every section after that specifies a
+version where the formatting style was changed.
+
+The test is run at multiple language versions and the result compared to the
+appropriate output section for that version. In the example here, we won't test
+it at all at 3.7, will test at 3.8 and 3.9 with the first output, and at
+3.10 and higher with the last output.
 
 ### Test options
 

test/tall/declaration/typedef.unit

@@ -160,26 +160,26 @@ typedef Generic<T, R> =
     );
 >>> Allow block-formatting a record typedef.
 typedef SomeType = (int first, int second);
-<<<
-typedef SomeType = (
-  int first,
-  int second,
-);
 <<< 3.7
 typedef SomeType =
     (int first, int second);
->>> Don't allow block-formatting a record typedef.
-typedef SomeType = (int first, int second, String third);
-<<<
+<<< 3.8
 typedef SomeType = (
   int first,
   int second,
-  String third,
 );
+>>> Don't allow block-formatting a record typedef.
+typedef SomeType = (int first, int second, String third);
 <<< 3.7
 typedef SomeType =
     (
       int first,
       int second,
       String third,
     );
+<<< 3.8
+typedef SomeType = (
+  int first,
+  int second,
+  String third,
+);

test/tall/expression/assignment.stmt

@@ -81,13 +81,6 @@ target
     reallyLongValue;
 >>> Allow block formatting through nested assignments.
 outer = inner = [element1, element2, element3, element4];
-<<<
-outer = inner = [
-  element1,
-  element2,
-  element3,
-  element4,
-];
 <<< 3.7
 outer =
     inner = [
@@ -96,19 +89,26 @@ outer =
       element3,
       element4,
     ];
+<<< 3.8
+outer = inner = [
+  element1,
+  element2,
+  element3,
+  element4,
+];
 >>> Headline format unsplit target of call chain.
 variable = (tar + get).method().another().third();
-<<<
-variable = (tar + get)
-    .method()
-    .another()
-    .third();
 <<< 3.7
 variable =
     (tar + get)
         .method()
         .another()
         .third();
+<<< 3.8
+variable = (tar + get)
+    .method()
+    .another()
+    .third();
 >>> Don't headline format target of call chain if target splits.
 variable = (veryLongTarget + expressionThatSplits).method().another().third();
 <<<
@@ -120,14 +120,14 @@ variable =
         .third();
 >>> Headline format unsplit properties of call chain.
 variable = (tar + get).prop.erty.method().another().third();
-<<<
-variable = (tar + get).prop.erty
-    .method()
-    .another()
-    .third();
 <<< 3.7
 variable =
     (tar + get).prop.erty
         .method()
         .another()
         .third();
+<<< 3.8
+variable = (tar + get).prop.erty
+    .method()
+    .another()
+    .third();