Don't use explicit error locations for CFE-only errors.

munificent · commit-bot@chromium.org · commit 1f10afc628f7 · 2019-08-29T01:02:45.000Z
The CFE currently does not report the length in an error location, just the starting position. When the static error update tool inserts an error marker for an error reported only by the CFE, it doesn't know what length to use. It used to deal with this by always writing an explicit error location like: // [error line 1, column 9] Those are kind of ugly and brittle, though. This changes it to treat the error as implicitly having length 1. This way, it can just output: // ^ When validating an error expectation against a report CFE error, the length is ignored anyway (since the CFE doesn't report it). In order to ensure that the parsed output of the tool matches the reported data that produced it, the parser also ignores the length when parsing an error expectation for a CFE-only error with length one. Fix #37991. Change-Id: I20e109142546b7e82a5f796a1a40613b90dc89bd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114745 Reviewed-by: Leaf Petersen <leafp@google.com> Commit-Queue: Bob Nystrom <rnystrom@google.com>
diff --git a/pkg/test_runner/lib/src/test_file.dart b/pkg/test_runner/lib/src/test_file.dart
@@ -921,6 +921,14 @@ class ErrorExpectationParser {
           "error.");
     }
 
+    // Hack: If the error is CFE-only and the length is one, treat it as no
+    // length. The CFE does not output length information, and when the update
+    // tool writes a CFE-only error, it implicitly uses a length of one. Thus,
+    // when we parse back in a length one CFE error, we ignore the length so
+    // that the error round-trips correctly.
+    // TODO(rnystrom): Stop doing this when the CFE reports error lengths.
+    if (code == null && length == 1) length = null;
+
     _errors.add(StaticError(
         line: line,
         column: column,
diff --git a/pkg/test_runner/lib/src/update_errors.dart b/pkg/test_runner/lib/src/update_errors.dart
@@ -96,13 +96,15 @@ String updateErrorExpectations(String source, List<StaticError> errors,
 
       var comment = (" " * indent) + "//";
 
-      // If the error can't fit in a line comment or doesn't have a length, use
-      // an explicit location.
-      if (error.length == null) {
-        result.add("$comment [error line $codeLine, column ${error.column}]");
-      } else if (error.column <= 2) {
-        result.add("$comment [error line $codeLine, column ${error.column}, "
-            "length ${error.length}]");
+      // If the error can't fit in a line comment, use an explicit location.
+      if (error.column <= 2) {
+        if (error.length == null) {
+          result.add("$comment [error line $codeLine, column "
+              "${error.column}]");
+        } else {
+          result.add("$comment [error line $codeLine, column "
+              "${error.column}, length ${error.length}]");
+        }
       } else {
         var spacing = " " * (error.column - 1 - 2 - indent);
         // A CFE-only error may not have a length, so treat it as length 1.
diff --git a/pkg/test_runner/test/test_file_test.dart b/pkg/test_runner/test/test_file_test.dart
@@ -457,6 +457,22 @@ int i = "s";
 /\/      ^^^
 /\/ [analyzer] Not error code.
 """);
+
+  // A CFE-only error with length one is treated as having no length.
+  expectParseErrorExpectations("""
+int i = "s";
+/\/      ^
+/\/ [cfe] Message.
+
+int j = "s";
+/\/      ^
+/\/ [analyzer] Error.BAD
+/\/ [cfe] Message.
+""", [
+    StaticError(line: 1, column: 9, length: null, message: "Message."),
+    StaticError(
+        line: 5, column: 9, length: 1, code: "Error.BAD", message: "Message."),
+  ]);
 }
 
 void testName() {
diff --git a/pkg/test_runner/test/update_errors_test.dart b/pkg/test_runner/test/update_errors_test.dart
@@ -192,12 +192,12 @@ int j =
 /\/ [cfe] Error.
 """);
 
-  // Uses an explicit error location if there's no length.
+  // Uses length one if there's no length.
   expectUpdate("""
 int i = "bad";
 """, errors: [StaticError(line: 1, column: 9, message: "Error.")], expected: """
 int i = "bad";
-/\/ [error line 1, column 9]
+/\/      ^
 /\/ [cfe] Error.
 """);
 
@@ -322,7 +322,7 @@ class A {
     yy(4);
 /\/  ^^^^^^^^^^^^^^
 /\/ [analyzer] unspecified
-/\/ [error line 12, column 7]
+/\/    ^
 /\/ [cfe] The method 'call' isn't defined for the class 'int'.
 
   }
