Prepare static error test updater tool to handle web tests.

This doesn't actually run DDC to generate the web errors yet, but it
changes the CLI in anticipation of that, and adds tests to verify that
once web errors are reported that the updater handles them correctly.

Change-Id: I31264e3d468969b07f9eb60353a9b02a93bec7ea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155102
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
This commit is contained in:
Robert Nystrom 2020-07-22 22:48:49 +00:00 committed by commit-bot@chromium.org
parent b258585f2f
commit 1094b3c61d
3 changed files with 255 additions and 116 deletions

View file

@ -12,12 +12,11 @@ final _lineCommentRegExp = RegExp(r"^\s*//");
/// Removes existing static error marker comments in [source] and adds markers
/// for the given [errors].
///
/// If [removeAnalyzer] is `false`, then existing analyzer errors in [source]
/// are preserved. Likewise for [removeCfe] and CFE errors.
/// If [remove] is not `null`, then only removes existing errors for the given
/// sources.
String updateErrorExpectations(String source, List<StaticError> errors,
{bool removeAnalyzer, bool removeCfe}) {
removeAnalyzer ??= true;
removeCfe ??= true;
{Set<ErrorSource> remove}) {
remove ??= {};
var existingErrors = StaticError.parseExpectations(source);
var lines = source.split("\n");
@ -38,13 +37,9 @@ String updateErrorExpectations(String source, List<StaticError> errors,
}
// Re-add errors for the portions we intend to preserve.
var keepAnalyzer = !removeAnalyzer && error.hasError(ErrorSource.analyzer);
var keepCfe = !removeCfe && error.hasError(ErrorSource.cfe);
var keptErrors = {
if (keepAnalyzer)
ErrorSource.analyzer: error.errorFor(ErrorSource.analyzer),
if (keepCfe) ErrorSource.cfe: error.errorFor(ErrorSource.cfe),
for (var source in ErrorSource.all.toSet().difference(remove))
if (error.hasError(source)) source: error.errorFor(source)
};
if (keptErrors.isNotEmpty) {

View file

@ -1,6 +1,7 @@
// Copyright (c) 2019, 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 'dart:math' as math;
import 'package:expect/expect.dart';
@ -16,7 +17,7 @@ import 'utils.dart';
// escapes here like `\/`.
void main() {
// Inserts analyzer, CFE, and both errors.
// Inserts single-front end errors.
expectUpdate("""
int i = "bad";
@ -26,12 +27,7 @@ int third = "boo";
""", errors: [
makeError(line: 1, column: 9, length: 5, analyzerError: "some.error"),
makeError(line: 3, column: 15, length: 7, cfeError: "Bad."),
makeError(
line: 5,
column: 13,
length: 5,
analyzerError: "an.error",
cfeError: "Wrong.\nLine.\nAnother."),
makeError(line: 5, column: 13, length: 5, webError: "Web.\nError."),
], expected: """
int i = "bad";
/\/ ^^^^^
@ -43,15 +39,72 @@ int another = "wrong";
int third = "boo";
/\/ ^^^^^
/\/ [analyzer] an.error
/\/ [cfe] Wrong.
/\/ Line.
/\/ Another.
/\/ [web] Web.
/\/ Error.
""");
// Inserts errors for multiple front ends.
expectUpdate("""
int i = "bad";
int another = "wrong";
int third = "boo";
int last = "oops";
""", errors: [
makeError(
line: 1,
column: 9,
length: 5,
analyzerError: "some.error",
cfeError: "Bad."),
makeError(
line: 3,
column: 15,
length: 7,
cfeError: "Another bad.",
webError: "Web.\nError."),
makeError(
line: 5,
column: 13,
length: 5,
analyzerError: "third.error",
webError: "Web error."),
makeError(
line: 7,
column: 12,
length: 6,
analyzerError: "last.error",
cfeError: "Final.\nError.",
webError: "Web error."),
], expected: """
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
/\/ [cfe] Bad.
int another = "wrong";
/\/ ^^^^^^^
/\/ [cfe] Another bad.
/\/ [web] Web.
/\/ Error.
int third = "boo";
/\/ ^^^^^
/\/ [analyzer] third.error
/\/ [web] Web error.
int last = "oops";
/\/ ^^^^^^
/\/ [analyzer] last.error
/\/ [cfe] Final.
/\/ Error.
/\/ [web] Web error.
""");
// Removes only analyzer errors.
expectUpdate(
"""
expectUpdate("""
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
@ -64,9 +117,7 @@ int third = "boo";
/\/ ^^^^^
/\/ [analyzer] an.error
/\/ [cfe] Wrong.
""",
removeCfe: false,
expected: """
""", remove: {ErrorSource.analyzer}, expected: """
int i = "bad";
int another = "wrong";
@ -79,8 +130,7 @@ int third = "boo";
""");
// Removes only CFE errors.
expectUpdate(
"""
expectUpdate("""
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
@ -93,9 +143,7 @@ int third = "boo";
/\/ ^^^^^
/\/ [analyzer] an.error
/\/ [cfe] Wrong.
""",
removeAnalyzer: false,
expected: """
""", remove: {ErrorSource.cfe}, expected: """
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
@ -105,6 +153,60 @@ int another = "wrong";
int third = "boo";
/\/ ^^^^^
/\/ [analyzer] an.error
""");
// Removes only web errors.
expectUpdate("""
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
int another = "wrong";
/\/ ^^^^^^^
/\/ [web] Bad.
int third = "boo";
/\/ ^^^^^
/\/ [cfe] Error.
/\/ [web] Wrong.
""", remove: {ErrorSource.web}, expected: """
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
int another = "wrong";
int third = "boo";
/\/ ^^^^^
/\/ [cfe] Error.
""");
// Removes multiple error sources.
expectUpdate("""
int i = "bad";
/\/ ^^^^^
/\/ [analyzer] some.error
/\/ [cfe] CFE error.
int another = "wrong";
/\/ ^^^^^^^
/\/ [web] Bad.
int third = "boo";
/\/ ^^^^^
/\/ [analyzer] another.error
/\/ [cfe] Error.
/\/ [web] Wrong.
""", remove: {ErrorSource.analyzer, ErrorSource.web}, expected: """
int i = "bad";
/\/ ^^^^^
/\/ [cfe] CFE error.
int another = "wrong";
int third = "boo";
/\/ ^^^^^
/\/ [cfe] Error.
""");
// Preserves previous error's indentation if possible.
@ -303,8 +405,7 @@ x
void regression() {
// https://github.com/dart-lang/sdk/issues/37990.
expectUpdate(
"""
expectUpdate("""
int get xx => 3;
int get yy => 3;
@ -325,20 +426,16 @@ class A {
}
}
""",
removeAnalyzer: false,
errors: [
makeError(
line: 6,
column: 5,
length: 14,
cfeError: "Setter not found: 'xx'."),
makeError(
line: 16,
column: 7,
cfeError: "The method 'call' isn't defined for the class 'int'.")
],
expected: """
""", remove: {
ErrorSource.cfe
}, errors: [
makeError(
line: 6, column: 5, length: 14, cfeError: "Setter not found: 'xx'."),
makeError(
line: 16,
column: 7,
cfeError: "The method 'call' isn't defined for the class 'int'.")
], expected: """
int get xx => 3;
int get yy => 3;
@ -362,18 +459,62 @@ class A {
}
void expectUpdate(String original,
{List<StaticError> errors,
bool removeAnalyzer = true,
bool removeCfe = true,
String expected}) {
{List<StaticError> errors, Set<ErrorSource> remove, String expected}) {
errors ??= const [];
remove ??= ErrorSource.all.toSet();
var actual = updateErrorExpectations(original, errors,
removeAnalyzer: removeAnalyzer, removeCfe: removeCfe);
var actual = updateErrorExpectations(original, errors, remove: remove);
if (actual != expected) {
// Not using Expect.equals() because the diffs it shows aren't helpful for
// strings this large.
Expect.fail("Output did not match expectation. Expected:\n$expected"
"\n\nWas:\n$actual");
var actualLines = actual.split("\n");
var expectedLines = expected.split("\n");
// Figure out which leading lines do match so we can ignore those and
// highlight the offending ones.
var matchingActual = <int>{};
var matchingExpected = <int>{};
for (var i = 0;
i < math.min(actualLines.length, expectedLines.length);
i++) {
if (actualLines[i] != expectedLines[i]) break;
matchingActual.add(i);
matchingExpected.add(i);
}
// Find which trailing lines are the same so we can hide those too.
for (var i = 0;
i < math.min(actualLines.length, expectedLines.length);
i++) {
// Count backwards from the ends of each list.
var actualIndex = actualLines.length - i - 1;
var expectedIndex = expectedLines.length - i - 1;
if (actualLines[actualIndex] != expectedLines[expectedIndex]) break;
matchingActual.add(actualIndex);
matchingExpected.add(expectedIndex);
}
var buffer = StringBuffer();
void writeLine(int index, String line, Set<int> matchingLines) {
// Only show the line if it was different from the expectation.
if (matchingLines.contains(index)) {
buffer.writeln(" : $line");
} else {
buffer.writeln("${(index + 1).toString().padLeft(4)}: $line");
}
}
buffer.writeln("Output did not match expectation. Expected:");
for (var i = 0; i < expectedLines.length; i++) {
writeLine(i, expectedLines[i], matchingExpected);
}
buffer.writeln();
buffer.writeln("Was:");
for (var i = 0; i < actualLines.length; i++) {
writeLine(i, actualLines[i], matchingActual);
}
Expect.fail(buffer.toString());
}
}

View file

@ -19,6 +19,8 @@ const _usage =
"Usage: dart update_static_error_tests.dart [flags...] <path glob>";
Future<void> main(List<String> args) async {
var sources = ErrorSource.all.map((e) => e.marker).toList();
var parser = ArgParser();
parser.addFlag("help", abbr: "h");
@ -28,41 +30,32 @@ Future<void> main(List<String> args) async {
help: "Print result but do not overwrite any files.",
negatable: false);
parser.addSeparator("Strip expectations out of the tests:");
parser.addFlag("remove",
parser.addSeparator("What operations to perform:");
parser.addFlag("remove-all",
abbr: "r",
help: "Remove all existing error expectations.",
negatable: false);
parser.addFlag("remove-analyzer",
help: "Remove existing analyzer error expectations.", negatable: false);
parser.addFlag("remove-cfe",
help: "Remove existing CFE error expectations.", negatable: false);
parser.addSeparator(
"Insert expectations in the tests based on current front end output:");
parser.addFlag("insert",
parser.addMultiOption("remove",
help: "Remove error expectations for given front ends.",
allowed: sources);
parser.addFlag("insert-all",
abbr: "i",
help: "Insert analyzer and CFE error expectations.",
help: "Insert error expectations for all front ends.",
negatable: false);
parser.addFlag("insert-analyzer",
help: "Insert analyzer error expectations.", negatable: false);
parser.addFlag("insert-cfe",
help: "Insert CFE error expectations.", negatable: false);
parser.addSeparator("Update combines remove and insert:");
parser.addFlag("update",
parser.addMultiOption("insert",
help: "Insert error expectations from given front ends.",
allowed: sources);
parser.addFlag("update-all",
abbr: "u",
help: "Replace analyzer and CFE error expectations.",
help: "Replace error expectations for all front ends.",
negatable: false);
parser.addFlag("update-analyzer",
help: "Replace analyzer error expectations.", negatable: false);
parser.addFlag("update-cfe",
help: "Replace CFE error expectations.", negatable: false);
parser.addMultiOption("update",
help: "Update error expectations for given front ends.",
allowed: sources);
parser.addSeparator("Other flags:");
parser.addFlag("nnbd",
help: "Analyze with the 'non-nullable' experiment enabled.",
negatable: false);
parser.addFlag("null-safety",
help: "Enable the 'non-nullable' experiment.", negatable: false);
var results = parser.parse(args);
@ -76,29 +69,38 @@ Future<void> main(List<String> args) async {
}
var dryRun = results["dry-run"] as bool;
var removeAnalyzer = results["remove-analyzer"] as bool ||
results["remove"] as bool ||
results["update-analyzer"] as bool ||
results["update"] as bool;
var nullSafety = results["null-safety"] as bool;
var removeCfe = results["remove-cfe"] as bool ||
results["remove"] as bool ||
results["update-cfe"] as bool ||
results["update"] as bool;
var removeSources = <ErrorSource>{};
var insertSources = <ErrorSource>{};
var insertAnalyzer = results["insert-analyzer"] as bool ||
results["insert"] as bool ||
results["update-analyzer"] as bool ||
results["update"] as bool;
for (var source in results["remove"] as List<String>) {
removeSources.add(ErrorSource.find(source));
}
var insertCfe = results["insert-cfe"] as bool ||
results["insert"] as bool ||
results["update-cfe"] as bool ||
results["update"] as bool;
for (var source in results["insert"] as List<String>) {
insertSources.add(ErrorSource.find(source));
}
var nnbd = results["nnbd"] as bool;
for (var source in results["update"] as List<String>) {
removeSources.add(ErrorSource.find(source));
insertSources.add(ErrorSource.find(source));
}
if (!removeAnalyzer && !removeCfe && !insertAnalyzer && !insertCfe) {
if (results["remove-all"] as bool) {
removeSources.addAll(ErrorSource.all);
}
if (results["insert-all"] as bool) {
insertSources.addAll(ErrorSource.all);
}
if (results["update-all"] as bool) {
removeSources.addAll(ErrorSource.all);
insertSources.addAll(ErrorSource.all);
}
if (removeSources.isEmpty && insertSources.isEmpty) {
_usageError(
parser, "Must provide at least one flag for an operation to perform.");
}
@ -109,11 +111,13 @@ Future<void> main(List<String> args) async {
}
var result = results.rest.single;
// Allow tests to be specified without the extension for compatibility with
// the regular test runner syntax.
if (!result.endsWith(".dart")) {
result += ".dart";
}
// Allow tests to be specified either relative to the "tests" directory
// or relative to the current directory.
var root = result.startsWith("tests") ? "." : "tests";
@ -124,17 +128,15 @@ Future<void> main(List<String> args) async {
if (entry is File) {
await _processFile(entry,
dryRun: dryRun,
removeAnalyzer: removeAnalyzer,
removeCfe: removeCfe,
insertAnalyzer: insertAnalyzer,
insertCfe: insertCfe,
nnbd: nnbd);
remove: removeSources,
insert: insertSources,
nullSafety: nullSafety);
}
}
}
void _usageError(ArgParser parser, String message) {
stderr.writeln("Usage error: $message");
stderr.writeln(message);
stderr.writeln();
stderr.writeln(_usage);
stderr.writeln(parser.usage);
@ -143,17 +145,15 @@ void _usageError(ArgParser parser, String message) {
Future<void> _processFile(File file,
{bool dryRun,
bool removeAnalyzer,
bool removeCfe,
bool insertAnalyzer,
bool insertCfe,
bool nnbd}) async {
Set<ErrorSource> remove,
Set<ErrorSource> insert,
bool nullSafety}) async {
stdout.write("${file.path}...");
var source = file.readAsStringSync();
var testFile = TestFile.parse(Path("."), file.absolute.path, source);
var experiments = [
if (nnbd) "non-nullable",
if (nullSafety) "non-nullable",
if (testFile.experiments.isNotEmpty) ...testFile.experiments
];
@ -163,7 +163,7 @@ Future<void> _processFile(File file,
];
var errors = <StaticError>[];
if (insertAnalyzer) {
if (insert.contains(ErrorSource.analyzer)) {
stdout.write("\r${file.path} (Running analyzer...)");
var fileErrors = await runAnalyzer(file.absolute.path, options);
if (fileErrors == null) {
@ -173,7 +173,7 @@ Future<void> _processFile(File file,
}
}
if (insertCfe) {
if (insert.contains(ErrorSource.cfe)) {
// Clear the previous line.
stdout.write("\r${file.path} ");
stdout.write("\r${file.path} (Running CFE...)");
@ -185,10 +185,13 @@ Future<void> _processFile(File file,
}
}
if (insert.contains(ErrorSource.web)) {
// TODO(rnystrom): Run DDC and collect web errors.
}
errors = StaticError.simplify(errors);
var result = updateErrorExpectations(source, errors,
removeAnalyzer: removeAnalyzer, removeCfe: removeCfe);
var result = updateErrorExpectations(source, errors, remove: remove);
stdout.writeln("\r${file.path} (Updated with ${errors.length} errors)");