From 82afcb4eddf86b69bba0be4b03a6053929a2df33 Mon Sep 17 00:00:00 2001 From: Robert Nystrom Date: Mon, 11 Nov 2024 23:36:35 +0000 Subject: [PATCH] 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 Auto-Submit: Bob Nystrom Commit-Queue: Nate Bosch --- pkg/test_runner/lib/src/update_errors.dart | 23 ++++---- pkg/test_runner/test/update_errors_test.dart | 62 +++++++++----------- 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/pkg/test_runner/lib/src/update_errors.dart b/pkg/test_runner/lib/src/update_errors.dart index f8068c4b1ac2..e7c1463d3b65 100644 --- a/pkg/test_runner/lib/src/update_errors.dart +++ b/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}]"); diff --git a/pkg/test_runner/test/update_errors_test.dart b/pkg/test_runner/test/update_errors_test.dart index 9477be9fc4c8..953128d04ccd 100644 --- a/pkg/test_runner/test/update_errors_test.dart +++ b/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.