Make the static error test updater not write comments at column zero.

The old formatter had a special rule that if a line comment as at the
left edge of the page, it would stay there regardless of the surrounding
indentation. So if you had:

```
class C {
  m() {
// comment
  }
}
```

After formatting, the comment would still be there instead of being
indented. The intent of that was to not shift over code that had been
commented out.

But all of the IDEs I tested don't actually work that way. When they
comment out code, they tend to put the `//` at the indentation of the
surrounding code. So the new formatter doesn't have this special rule
and always indents line comments following the surrounding indentation.
This is good because it also means that code generators that don't write
any leading whitespace will still get nicely formatted comments.

However, the static error test updater took advantage of this rule and
would write static error marker comments at column zero if needed to
get the carets to align with the code on the previous line and assume
that the formatter wouldn't move the comment.

This fixes the static error test updater. It always writes comments
using indentation from the previous line of code and if the caret
doesn't fit that way, it uses an explicit column marker.

Fix #57042.

Change-Id: I40fd7cd19d08dc228b6a6797e6a26965d1343d32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/394363
Reviewed-by: Nate Bosch <[email protected]>
Auto-Submit: Bob Nystrom <[email protected]>
Commit-Queue: Nate Bosch <[email protected]>

Author: munificent

pkg/test_runner/lib/src/update_errors.dart

@@ -105,13 +105,15 @@ String updateErrorExpectations(
     var previousLength = -1;
 
     for (var error in errorsHere) {
-      // Try to indent the line nicely to match either the existing expectation
-      // that is being regenerated, or, barring that, the previous line of code.
-      var indent = indentation[i + 1] ?? previousIndent;
+      // Try to indent the line nicely to match the existing expectation that
+      // is being regenerated. If that collides with the carets, then indent
+      // the line based on the preceding line of code. If the caret still
+      // doesn't fit with that indentation, we'll use an explicit location.
+      var indent = indentation[i + 1];
+      if (indent == null || error.column - 1 < indent + 2) {
+        indent = previousIndent;
+      }
 
-      // If the error is to the left of the indent and the "//", sacrifice the
-      // indentation.
-      if (error.column - 1 < indent + 2) indent = 0;
       var comment = "${" " * indent}//";
 
       // Write the location line, unless we already have an identical one. Allow
@@ -120,12 +122,11 @@ String updateErrorExpectations(
           (previousLength != 0 &&
               error.length != 0 &&
               error.length != previousLength)) {
-        // If the error can't fit in a line comment, or no source location is
-        // specified, use an explicit location.
-        if (error.column <= 2) {
+        // If the error location starts to the left of the line comment, or no
+        // error length is specified, use an explicit location.
+        if (error.column - 1 < indent + 2) {
           if (error.length == 0) {
-            result.add("$comment [error column "
-                "${error.column}]");
+            result.add("$comment [error column ${error.column}]");
           } else {
             result.add("$comment [error column "
                 "${error.column}, length ${error.length}]");

pkg/test_runner/test/update_errors_test.dart

@@ -229,54 +229,46 @@ main() {
 }
 """);
 
-  // Discards indentation if it would collide with carets.
+  // Discards previous error indentation if it would collide with carets.
   expectUpdate("""
+// This fits:
 int i = "bad";
-        /\/    ^^
-        /\/ [analyzer] previous.error
+      /\/    ^^
+      /\/ [analyzer] previous.error
 
-main() {
-  int i =
-  "bad";
-}
+// This does not:
+int j = "bad";
+       /\/    ^^
+       /\/ [analyzer] previous.error
 """, errors: [
-    makeError(line: 1, column: 9, length: 5, cfeError: "Error."),
-    makeError(line: 7, column: 3, length: 5, analyzerError: "new.error"),
+    makeError(line: 2, column: 9, length: 5, cfeError: "Error."),
+    makeError(line: 7, column: 9, length: 5, cfeError: "Error."),
   ], expected: """
+// This fits:
 int i = "bad";
+      /\/^^^^^
+      /\/ [cfe] Error.
+
+// This does not:
+int j = "bad";
 /\/      ^^^^^
 /\/ [cfe] Error.
-
-main() {
-  int i =
-  "bad";
-/\/^^^^^
-/\/ [analyzer] new.error
-}
 """);
 
-  // Uses an explicit error location if there's no room for the carets.
+  // Uses explicit location if indenting based on preceding code would collide
+  // with carets.
   expectUpdate("""
-int i =
-"bad";
-    /\/    ^^
-    /\/ [analyzer] previous.error
-
-int j =
-"bad";
+main() {
+  ["bad"];
+}
 """, errors: [
-    makeError(line: 2, column: 1, length: 5, analyzerError: "updated.error"),
-    makeError(line: 7, column: 1, length: 5, cfeError: "Error."),
+    makeError(line: 2, column: 4, length: 5, cfeError: "Error."),
   ], expected: """
-int i =
-"bad";
-/\/ [error column 1, length 5]
-/\/ [analyzer] updated.error
-
-int j =
-"bad";
-/\/ [error column 1, length 5]
-/\/ [cfe] Error.
+main() {
+  ["bad"];
+  /\/ [error column 4, length 5]
+  /\/ [cfe] Error.
+}
 """);
 
   // Uses length one if there's no length.