Move the status file parser into its own package.

This required munging a bit of test.dart code too to tease out some
dependencies, but the changes are minor. I considered moving all of
test.dart out into a package and making the status file library a
public one that other packages in the repo could import but this seemed
like the less intrusive change.

R=bkonyi@google.com

Review-Url: https://codereview.chromium.org/2984203002 .

.packages

@@ -88,6 +88,7 @@ source_map_stack_trace:third_party/pkg/source_map_stack_trace/lib
 source_maps:third_party/pkg/source_maps/lib
 source_span:third_party/pkg/source_span/lib
 stack_trace:third_party/pkg/stack_trace/lib
+status_file:pkg/status_file/lib
 stream_channel:third_party/pkg/stream_channel/lib
 string_scanner:third_party/pkg/string_scanner/lib
 telemetry:pkg/telemetry/lib

pkg/pkg.status

@@ -226,3 +226,6 @@ analyzer/test/src/task/strong/checker_test: Pass, Slow
 
 [ $browser ]
 testing/test/analyze_test: SkipByDesign
+
+[ $browser || $runtime == flutter ]
+status_file/*: SkipByDesign # Only meant to run on the standalone VM.

pkg/status_file/.packages

@@ -0,0 +1,4 @@
+# Generated by pub on 2017-07-24 16:32:37.651832.
+expect:../expect/lib/
+path:../../third_party/pkg/path/lib/
+status_file:lib/

pkg/status_file/lib/environment.dart

@@ -0,0 +1,16 @@
+// Copyright (c) 2017, 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.
+
+/// An implementation of this defines the variables that are available for use
+/// inside a status file section header.
+abstract class Environment {
+  /// Validates that the variable with [name] exists and can be compared
+  /// against [value].
+  ///
+  /// If any errors are found, adds them to [errors].
+  void validate(String name, String value, List<String> errors);
+
+  /// Looks up the value of the variable with [name].
+  String lookUp(String name);
+}

pkg/status_file/lib/src/expression.dart

@@ -1,8 +1,8 @@
-// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
+// Copyright (c) 2017, 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 'environment.dart';
+import '../environment.dart';
 
 /// A parsed Boolean expression AST.
 abstract class Expression {
@@ -29,7 +29,7 @@ abstract class Expression {
   /// Ensures that any variable names are known and that any literal values are
   /// allowed for their corresponding variable. If an invalid variable or value
   /// is found, adds appropriate error messages to [errors].
-  void validate(List<String> errors);
+  void validate(Environment environment, List<String> errors);
 
   /// Evaluates the expression where all variables are defined by the given
   /// [environment].
@@ -83,8 +83,8 @@ class _ComparisonExpression implements Expression {
 
   _ComparisonExpression(this.left, this.right, this.negate);
 
-  void validate(List<String> errors) {
-    Environment.validate(left.name, right, errors);
+  void validate(Environment environment, List<String> errors) {
+    environment.validate(left.name, right, errors);
   }
 
   bool evaluate(Environment environment) {
@@ -117,9 +117,9 @@ class _VariableExpression implements Expression {
 
   _VariableExpression(this.variable, {this.negate = false});
 
-  void validate(List<String> errors) {
+  void validate(Environment environment, List<String> errors) {
     // It must be a Boolean, so it should allow either Boolean value.
-    Environment.validate(variable.name, "true", errors);
+    environment.validate(variable.name, "true", errors);
   }
 
   bool evaluate(Environment environment) =>
@@ -138,9 +138,9 @@ class _LogicExpression implements Expression {
 
   _LogicExpression(this.op, this.left, this.right);
 
-  void validate(List<String> errors) {
-    left.validate(errors);
-    right.validate(errors);
+  void validate(Environment environment, List<String> errors) {
+    left.validate(environment, errors);
+    right.validate(environment, errors);
   }
 
   bool evaluate(Environment environment) => (op == _Token.and)

pkg/status_file/lib/status_file.dart

@@ -4,10 +4,11 @@
 
 import 'dart:io';
 
+import 'package:path/path.dart' as p;
+
 import 'environment.dart';
 import 'expectation.dart';
-import 'path.dart';
-import 'status_expression.dart';
+import 'src/expression.dart';
 
 /// Matches the header that begins a new section, like:
 ///
@@ -46,6 +47,8 @@ class StatusFile {
   final List<StatusSection> sections = [];
 
   /// Parses the status file at [_path].
+  ///
+  /// Throws a [SyntaxError] if the file could not be parsed.
   StatusFile.read(this._path) {
     var lines = new File(_path).readAsLinesSync();
 
@@ -58,14 +61,7 @@ class StatusFile {
       lineNumber++;
 
       fail(String message, [List<String> errors]) {
-        print('$message in "$_shortPath" line $lineNumber:\n$line');
-
-        if (errors != null) {
-          for (var error in errors) {
-            print("- ${error.replaceAll('\n', '\n  ')}");
-          }
-        }
-        exit(1);
+        throw new SyntaxError(_shortPath, lineNumber, line, message, errors);
       }
 
       // Strip off the comment and whitespace.
@@ -85,17 +81,7 @@ class StatusFile {
       var match = _sectionPattern.firstMatch(source);
       if (match != null) {
         try {
-          var condition = Expression.parse(match[1].trim());
-
-          var errors = <String>[];
-          condition.validate(errors);
-
-          if (errors.isNotEmpty) {
-            var s = errors.length > 1 ? "s" : "";
-            fail('Validation error$s', errors);
-          }
-
-          section = new StatusSection(condition);
+          section = new StatusSection(Expression.parse(match[1].trim()));
           sections.add(section);
         } on FormatException {
           fail("Status expression syntax error");
@@ -135,12 +121,31 @@ class StatusFile {
     }
   }
 
+  /// Validates that the variables and values used in all of the section
+  /// condition expressions are defined in [environment].
+  ///
+  /// Throws a [SyntaxError] on the first found error.
+  void validate(Environment environment) {
+    // TODO(rnystrom): It would be more useful if it reported all of the errors
+    // instead of stopping on the first.
+    for (var section in sections) {
+      if (section._condition == null) continue;
+
+      var errors = <String>[];
+      section._condition.validate(environment, errors);
+
+      if (errors.isNotEmpty) {
+        var s = errors.length > 1 ? "s" : "";
+        throw new SyntaxError(_shortPath, section.lineNumber,
+            "[ ${section._condition} ]", 'Validation error$s', errors);
+      }
+    }
+  }
+
   /// Gets the path to this status file relative to the Dart repo root.
   String get _shortPath {
-    var repoRoot = new Path(Platform.script
-            .toFilePath(windows: Platform.operatingSystem == "windows"))
-        .join(new Path("../../../../"));
-    return new Path(_path).relativeTo(repoRoot).toString();
+    var repoRoot = p.join(p.dirname(p.fromUri(Platform.script)), "../../../");
+    return p.normalize(p.relative(_path, from: repoRoot));
   }
 
   /// Returns the issue number embedded in [comment] or `null` if there is none.
@@ -154,7 +159,7 @@ class StatusFile {
   String toString() {
     var buffer = new StringBuffer();
     for (var section in sections) {
-      buffer.writeln("[${section._condition}]");
+      buffer.writeln("[ ${section._condition} ]");
 
       for (var entry in section.entries) {
         buffer.write("${entry.path}: ${entry.expectations.join(', ')}");
@@ -196,3 +201,28 @@ class StatusEntry {
 
   StatusEntry(this.path, this.expectations, this.issue);
 }
+
+/// Error thrown when a parse or validation error occurs in a [StatusFile].
+class SyntaxError implements Exception {
+  final String file;
+  final int lineNumber;
+  final String line;
+  final String message;
+  final List<String> errors;
+
+  SyntaxError(this.file, this.lineNumber, this.line, this.message, this.errors);
+
+  String toString() {
+    var buffer = new StringBuffer();
+    buffer.writeln('$message in "$file" line $lineNumber:');
+    buffer.writeln(line);
+
+    if (errors != null) {
+      for (var error in errors) {
+        buffer.writeln("- ${error.replaceAll('\n', '\n  ')}");
+      }
+    }
+
+    return buffer.toString().trimRight();
+  }
+}

pkg/status_file/pubspec.yaml

@@ -0,0 +1,12 @@
+name: status_file
+author: Dart Team <misc@dartlang.org>
+description: Parses status files.
+homepage: http://www.dartlang.org
+#environment:
+#  sdk: '>=1.0.0 <2.0.0'
+dependencies:
+  path:
+    path: ../../third_party/pkg/path
+dev_dependencies:
+  expect:
+    path: ../expect

pkg/status_file/test/repo_status_files_test.dart

@@ -0,0 +1,36 @@
+// Copyright (c) 2017, 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.
+
+/// Tests that every .status file in the Dart repository can be successfully
+/// parsed.
+import 'dart:io';
+
+import 'package:expect/expect.dart';
+import 'package:path/path.dart' as p;
+import 'package:status_file/status_file.dart';
+
+final repoRoot =
+    p.normalize(p.join(p.dirname(p.fromUri(Platform.script)), "../../../"));
+
+void main() {
+  // Parse every status file in the repository.
+  for (var directory in ["tests", p.join("runtime", "tests")]) {
+    for (var entry in new Directory(p.join(repoRoot, directory))
+        .listSync(recursive: true)) {
+      if (!entry.path.endsWith(".status")) continue;
+
+      // Inside the co19 repository, there is a status file that doesn't appear
+      // to be valid and looks more like some kind of template or help document.
+      // Ignore it.
+      if (entry.path.endsWith(p.join("co19", "src", "co19.status"))) continue;
+
+      try {
+        new StatusFile.read(entry.path);
+      } catch (err) {
+        var path = p.relative(entry.path, from: repoRoot);
+        Expect.fail("Could not parse '$path'.\n$err");
+      }
+    }
+  }
+}

pkg/status_file/test/status_expression_test.dart

@@ -1,17 +1,21 @@
-// Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
+// Copyright (c) 2017, 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 "package:expect/expect.dart";
 
-import "../../tools/testing/dart/environment.dart";
-import "../../tools/testing/dart/status_expression.dart";
+import "package:status_file/environment.dart";
+import "package:status_file/src/expression.dart";
 
 class TestEnvironment implements Environment {
   final Map<String, String> _values;
 
   TestEnvironment(this._values);
 
+  void validate(String name, String value, List<String> errors) {
+    throw new UnimplementedError();
+  }
+
   /// Looks up the value of the variable with [name].
   String lookUp(String name) => _values[name];
 

tests/standalone/io/status_file_parser_test.dart

@@ -1,40 +0,0 @@
-// Copyright (c) 2013, 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.
-
-library StatusFileParserTest;
-
-import "package:expect/expect.dart";
-import "dart:io";
-import "../../../tools/testing/dart/path.dart";
-import "../../../tools/testing/dart/status_file.dart";
-import "../../../tools/testing/dart/utils.dart";
-
-void main() {
-  testReadStatusFile("runtime/tests/vm/vm.status");
-  testReadStatusFile("samples/tests/samples/samples.status");
-  testReadStatusFile("tests/co19/co19-compiler.status");
-  testReadStatusFile("tests/co19/co19-runtime.status");
-  testReadStatusFile("tests/corelib/corelib.status");
-  testReadStatusFile("tests/dom/dom.status");
-  testReadStatusFile("tests/html/html.status");
-  testReadStatusFile("tests/isolate/isolate.status");
-  testReadStatusFile("tests/language/language.status");
-  testReadStatusFile("tests/standalone/standalone.status");
-}
-
-String fixFilePath(String filePath) {
-  if (new File(filePath).existsSync()) {
-    return filePath;
-  } else {
-    return "../${filePath}";
-  }
-}
-
-void testReadStatusFile(String filePath) {
-  var file = new File(fixFilePath(filePath));
-  if (!file.existsSync()) return;
-
-  var statusFile = new StatusFile.read(file.path);
-  Expect.isTrue(statusFile.sections.length > 0);
-}

tests/standalone/io/test_runner_test.dart

@@ -4,9 +4,11 @@
 
 import "dart:io";
 import "dart:async";
+
+import "package:status_file/expectation.dart";
+
 import "../../../tools/testing/dart/command.dart";
 import "../../../tools/testing/dart/configuration.dart";
-import "../../../tools/testing/dart/expectation.dart";
 import "../../../tools/testing/dart/options.dart";
 import "../../../tools/testing/dart/test_runner.dart";
 import "../../../tools/testing/dart/test_suite.dart";

tests/standalone/standalone.status

@@ -471,7 +471,5 @@ io/process_run_output_test: Fail # Unable to parse package files Flutter Issue 9
 
 io/dependency_graph_test: CompileTimeError # Imports dart:mirrors
 io/skipping_dart2js_compilations_test: CompileTimeError # Uses mirrors
-io/status_file_parser_test: CompileTimeError # Uses mirrors
 io/test_harness_analyzer_test: CompileTimeError # Uses mirrors
 io/test_runner_test: CompileTimeError # Uses mirrors
-status_expression_test: CompileTimeError # Uses mirrors

tools/testing/dart/command.dart

@@ -7,9 +7,10 @@ import 'dart:async';
 // CommandOutput.exitCode in subclasses of CommandOutput.
 import 'dart:io' as io;
 
+import 'package:status_file/expectation.dart';
+
 import 'command_output.dart';
 import 'configuration.dart';
-import 'expectation.dart';
 import 'path.dart';
 import 'utils.dart';
 

tools/testing/dart/command_output.dart

@@ -7,10 +7,11 @@ import 'dart:convert';
 // CommandOutput.exitCode in subclasses of CommandOutput.
 import 'dart:io' as io;
 
+import 'package:status_file/expectation.dart';
+
 import 'browser_controller.dart';
 import 'command.dart';
 import 'configuration.dart';
-import 'expectation.dart';
 import 'test_runner.dart';
 import 'utils.dart';
 

tools/testing/dart/environment.dart

@@ -2,6 +2,8 @@
 // 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 'package:status_file/environment.dart';
+
 import 'configuration.dart';
 
 typedef String _LookUpFunction(Configuration configuration);
@@ -51,12 +53,17 @@ String _runtimeName(Configuration configuration) {
 ///
 /// These mostly map to command line arguments with the same name, though this
 /// is only a subset of the full set of command line arguments.
-class Environment {
+class ConfigurationEnvironment implements Environment {
+  /// The configuration where variable data is found.
+  final Configuration _configuration;
+
+  ConfigurationEnvironment(this._configuration);
+
   /// Validates that the variable with [name] exists and can be compared
   /// against [value].
   ///
   /// If any errors are found, adds them to [errors].
-  static void validate(String name, String value, List<String> errors) {
+  void validate(String name, String value, List<String> errors) {
     var variable = _variables[name];
     if (variable == null) {
       errors.add('Unknown variable "$name".');
@@ -74,11 +81,6 @@ class Environment {
     }
   }
 
-  /// The configuration where variable data is found.
-  final Configuration _configuration;
-
-  Environment(this._configuration);
-
   /// Looks up the value of the variable with [name].
   String lookUp(String name) {
     var variable = _variables[name];

tools/testing/dart/expectation_set.dart

@@ -2,10 +2,13 @@
 // 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:io';
+
+import 'package:status_file/expectation.dart';
+import 'package:status_file/status_file.dart';
+
 import 'configuration.dart';
 import 'environment.dart';
-import 'expectation.dart';
-import 'status_file.dart';
 
 /// Tracks the [Expectation]s associated with a set of file paths.
 ///
@@ -18,20 +21,28 @@ class ExpectationSet {
   /// when in [configuration].
   static ExpectationSet read(
       List<String> statusFilePaths, Configuration configuration) {
-    var environment = new Environment(configuration);
-    var expectations = new ExpectationSet._();
-    for (var path in statusFilePaths) {
-      var file = new StatusFile.read(path);
-      for (var section in file.sections) {
-        if (section.isEnabled(environment)) {
-          for (var entry in section.entries) {
-            expectations.addEntry(entry);
+    try {
+      var environment = new ConfigurationEnvironment(configuration);
+      var expectations = new ExpectationSet._();
+      for (var path in statusFilePaths) {
+        var file = new StatusFile.read(path);
+        file.validate(environment);
+        for (var section in file.sections) {
+          if (section.isEnabled(environment)) {
+            for (var entry in section.entries) {
+              expectations.addEntry(entry);
+            }
           }
         }
       }
-    }
 
-    return expectations;
+      return expectations;
+    } on SyntaxError catch (error) {
+      stderr.writeln(error.toString());
+      exit(1);
+
+      throw "unreachable";
+    }
   }
 
   // Only create one copy of each Set<Expectation>.

tools/testing/dart/summary_report.dart

@@ -4,7 +4,8 @@
 
 library summary_report;
 
-import "expectation.dart";
+import "package:status_file/expectation.dart";
+
 import "test_runner.dart";
 
 final summaryReport = new SummaryReport();

tools/testing/dart/test_progress.dart

@@ -5,10 +5,11 @@
 import 'dart:convert' show JSON;
 import 'dart:io';
 
+import "package:status_file/expectation.dart";
+
 import 'command.dart';
 import 'command_output.dart';
 import 'configuration.dart';
-import 'expectation.dart';
 import 'path.dart';
 import 'summary_report.dart';
 import 'test_runner.dart';

tools/testing/dart/test_runner.dart

@@ -17,13 +17,14 @@ import 'dart:convert';
 import 'dart:io' as io;
 import 'dart:math' as math;
 
+import "package:status_file/expectation.dart";
+
 import 'android.dart';
 import 'browser_controller.dart';
 import 'command.dart';
 import 'command_output.dart';
 import 'configuration.dart';
 import 'dependency_graph.dart';
-import 'expectation.dart';
 import 'runtime_configuration.dart';
 import 'test_progress.dart';
 import 'test_suite.dart';

tools/testing/dart/test_suite.dart

@@ -15,11 +15,12 @@
 import 'dart:async';
 import 'dart:io';
 
+import "package:status_file/expectation.dart";
+
 import 'browser_test.dart';
 import 'command.dart';
 import 'compiler_configuration.dart';
 import 'configuration.dart';
-import 'expectation.dart';
 import 'expectation_set.dart';
 import 'html_test.dart' as html_test;
 import 'http_server.dart';