Add negation to single-identifier tests in status files.

tools/testing/dart/status_expression.dart

Index: tools/testing/dart/status_expression.dart
diff --git a/tools/testing/dart/status_expression.dart b/tools/testing/dart/status_expression.dart
index 10b30953096af0f6f634f6f8c94c6e36e9b3c645..f58884d7dba7f53e36574281bb3faa88e39c11d7 100644
--- a/tools/testing/dart/status_expression.dart
+++ b/tools/testing/dart/status_expression.dart
@@ -13,7 +13,8 @@ abstract class Expression {
   ///     expression := or
   ///     or         := and ( "||" and )*
   ///     and        := primary ( "&&" primary )*
-  ///     primary    := "$" identifier ( ( "==" | "!=" ) identifier )? |
+  ///     primary    := "$" identifier ( "==" | "!=" ) identifier |
+  ///                   "!"? "$" identifier |
   ///                   "(" expression ")"
   ///     identifier := regex "\w+"
   ///
@@ -44,6 +45,7 @@ class _Token {
   static const notEqual = "!=";
   static const and = "&&";
   static const or = "||";
+  static const not = "!";
 }
 
 /// A reference to a variable.
@@ -70,8 +72,10 @@ class _Variable {
 }
 
 /// Tests whether a given variable is or is not equal some literal value, as in:
-///
-///     $variable == someValue
+/// ```
+/// $variable == someValue
+/// ```
+/// Negate the result if [negate] is true.
 class _ComparisonExpression implements Expression {
   final _Variable left;
   final String right;
@@ -92,22 +96,36 @@ class _ComparisonExpression implements Expression {
 
 /// A reference to a variable defined in the environment. The expression
 /// evaluates to true if the variable's stringified value is "true".
-///
+/// ```
 ///     $variable
+/// ```
+/// is equivalent to
+/// ```
+///     $variable == true
+/// ```
+/// Negates result if [negate] is true, so
+/// ```
+///     !$variable
+/// ```
+/// is equivalent to
+/// ```
+///     $variable != true
+/// ```
 class _VariableExpression implements Expression {
   final _Variable variable;
+  final bool negate;
 
-  _VariableExpression(this.variable);
+  _VariableExpression(this.variable, {this.negate = false});
 
   void validate(List<String> errors) {
     // It must be a Boolean, so it should allow either Boolean value.
     Environment.validate(variable.name, "true", errors);
   }
 
-  bool evaluate(Environment environment) =>
-      variable.lookup(environment) == "true";
+  bool evaluate(Map<String, dynamic> environment) =>
+      negate != (variable.lookup(environment) == "true");
 
-  String toString() => "(bool \$${variable.name})";
+  String toString() => "(bool ${negate ? "! " : ""}\$${variable.name})";
 }
 
 /// A logical `||` or `&&` expression.
@@ -179,6 +197,11 @@ class _ExpressionParser {
       return value;
     }
 
+    bool negate = false;

[Bob Nystrom, 2017/06/01 23:07:46]

Nit: "bool" -> "var".

I'm moving test.dart to consistently use var for locals. (Previously, it was
simply inconsistent.)

[Lasse Reichstein Nielsen, 2017/06/07 08:51:49]

Done.

+    if (_scanner.match(_Token.not)) {
+      negate = true;
+    }
+
     // The only atomic booleans are of the form $variable == value or
     // of the form $variable.
     if (!_scanner.match(_Token.dollar)) {
@@ -194,8 +217,9 @@ class _ExpressionParser {
     var left = new _Variable(_scanner.current);
     _scanner.advance();
 
-    if (_scanner.current == _Token.equals ||
-        _scanner.current == _Token.notEqual) {
+    if (!negate &&
+        (_scanner.current == _Token.equals ||
+            _scanner.current == _Token.notEqual)) {
       var negate = _scanner.advance() == _Token.notEqual;

[Bob Nystrom, 2017/06/01 23:07:46]

How about renaming this local to "isNotEqual" to avoid confusing shadowing?

[Lasse Reichstein Nielsen, 2017/06/07 08:51:49]

Done.

 
       if (!_scanner.isIdentifier) {
@@ -206,20 +230,22 @@ class _ExpressionParser {
       var right = _scanner.advance();
       return new _ComparisonExpression(left, right, negate);
     } else {
-      return new _VariableExpression(left);
+      return new _VariableExpression(left, negate: negate);
     }
   }
 }
 
 /// An iterator that allows peeking at the current token.
 class _Scanner {
-  /// Tokens are "(", ")", "$", "&&", "||", "==", "!=", and (maximal) \w+.
-  static final _testPattern =
-      new RegExp(r"^([()$\w\s]|(\&\&)|(\|\|)|(\=\=)|(\!\=))+$");
-  static final _tokenPattern =
-      new RegExp(r"[()$]|(\&\&)|(\|\|)|(\=\=)|(\!\=)|\w+");
+  /// Tokens are "(", ")", "$", "&&", "||", "!", ==", "!=", and (maximal) \w+.
+  static final _testPattern = new RegExp(r"^(?:[()$\w\s]|&&|\|\||==|!=?)+$");
+  static final _tokenPattern = new RegExp(r"[()$]|&&|\|\||==|!=?|\w+");

[Bob Nystrom, 2017/06/01 23:07:46]

Much nicer!

[Lasse Reichstein Nielsen, 2017/06/07 08:51:49]

Occupational hazard - I can't pass by a RegExp without needing to tweak it :)

 
-  static final _identifierPattern = new RegExp(r"^\w+$");
+  /// Pattern that recognizes identifier tokens.
+  ///
+  /// Only checks the first character, since no non-identifier token can start
+  /// with a word character.
+  static final _identifierPattern = new RegExp(r"^\w");
 
   /// The token strings being iterated.
   final Iterator<String> tokenIterator;
@@ -244,7 +270,10 @@ class _Scanner {
   bool get hasMore => current != null;
 
   /// Returns `true` if the current token is an identifier.
-  bool get isIdentifier => _identifierPattern.hasMatch(current);
+  // All non-identifier tokens are one or two characters,
+  // so a longer token must be an identifier.
+  bool get isIdentifier =>
+      current.length > 2 || _identifierPattern.hasMatch(current);

[Bob Nystrom, 2017/06/01 23:07:46]

Is this an optimization? It seems unnecessary to me, unless this really does
address a visible (benchmarked?) performance problem.

[Lasse Reichstein Nielsen, 2017/06/07 08:51:49]

Yes, optimization.
Necessary? Maybe not, but it seems ridiculously expensive to do full "^\w+$"
RegExp run on every identifier (which will, in almost all cases, match anyway)
when checking just the first character or even the length is sufficient.

I guess my approach to performance comes from library work - they need to be as
efficient as absolutely possible because they affect code that can't fix them. 
Applications can probably stop when they are "fast enough" because they can
predict all their uses.

 
   /// If the current token is [token], consumes it and returns `true`.
   bool match(String token) {