Introduce a testing framework for use with fasta type inference.

pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart

+import 'dart:async';

[ahe, 2017/04/19 11:38:29]

Missing copyright.

[Paul Berry, 2017/04/19 13:20:10]

Done.

+import 'dart:convert';
+import 'dart:io';
+
+import 'package:front_end/src/base/instrumentation.dart';
+import 'package:front_end/src/fasta/scanner.dart';
+import 'package:front_end/src/fasta/scanner/io.dart';
+
+/// Implementation of [Instrumentation] which checks property/value pairs
+/// against expectations encoded in source files using "/*@...*/" comments.
+class ValidatingInstrumentation implements Instrumentation {
+  static final _ESCAPE_SEQUENCE = new RegExp(r'\\(.)');
+
+  /// Map from category names to the property names they are short for.

[ahe, 2017/04/19 11:38:29]

Here and several other places below: consider using the words "feature" and
"features" instead of "category" and "categories".

[Paul Berry, 2017/04/19 13:20:09]

Done.

+  static const _CATEGORIES = const {
+    'inference': const [
+      'topType',
+      'typeArg',
+      'promotedType',
+      'type',
+      'returnType'
+    ],
+  };
+
+  /// Map from file URI to the as-yet unsatisfied expectations from that file,
+  /// organized by file offset.
+  final _expectations = <Uri, Map<int, List<_Expectation>>>{};

[ahe, 2017/04/19 11:38:29]

Consider renaming to unsatisfiedExpecations.

[Paul Berry, 2017/04/19 13:20:10]

Done.

+
+  /// Information about "testedFeatures" annotations, organized by file URI and
+  /// file offset.  The inner map is guaranteed to be in ascending order of
+  /// file offset.
+  final _testedFeaturesState = <Uri, Map<int, Set<String>>>{};
+
+  /// String descriptions of the expectation mismatches found so far.
+  final _problems = <String>[];
+
+  /// Fixes that would need to be performed on source files in order for all
+  /// expectations to be met, organized by file URI.  The inner map is not
+  /// guaranteed to be in ascending order of file offset.
+  final _fixes = <Uri, List<_Fix>>{};
+
+  /// Indicates whether any expectation mismatches were found.
+  ///
+  /// Should be called after [finish].
+  bool get hasProblems => _problems.isNotEmpty;
+
+  /// Gets a description of all expectation mismatches that were found, in a
+  /// form suitable for printing to the console.
+  ///
+  /// Should be called after [finish].
+  get problemsAsString => _problems.join('\n');
+
+  /// Checks whether the property/value pairs passed to [record] match the
+  /// expectations loaded by [loadExpectations].
+  void finish() {
+    _expectations.forEach((uri, expectationsForUri) {
+      expectationsForUri.forEach((offset, expectationsAtOffset) {
+        for (var expectation in expectationsAtOffset) {
+          _problem(
+              uri,
+              offset,
+              'expected ${expectation.property}=${expectation.value}, '
+              'got nothing',
+              new _Fix(
+                  expectation.commentOffset, expectation.commentLength, ''));
+        }
+      });
+    });
+  }
+
+  /// Updates the source file at [uri] based on the actual property/value
+  /// pairs that were observed.
+  Future<Null> fixSource(Uri uri) async {
+    var fixes = _fixes[uri];
+    if (fixes == null) return;
+    var bytes = (await readBytesFromFile(uri)).toList();
+    // Remove the trailing \0 that's added by readBytesFromFile.

[ahe, 2017/04/19 11:38:29]

We probably need to add boolean argument to readBytesFromFile to disable the
trailing \0. This is the second time we've had to do this recently.

[ahe, 2017/04/19 13:10:59]

I've done that in CL 2827543006.

[Paul Berry, 2017/04/19 13:20:09]

Thanks!  I'll wait until you land that and then clean this up.

[Paul Berry, 2017/04/19 13:53:55]

Done.

+    bytes.removeLast();
+    fixes.sort((a, b) => b.offset.compareTo(a.offset));

[ahe, 2017/04/19 11:38:29]

I assume you sort them in reverse order to avoid having to adjust offsets below.
Perhaps add a comment about that?

[Paul Berry, 2017/04/19 13:20:10]

Done.

+    for (var fix in fixes) {
+      bytes.replaceRange(
+          fix.offset, fix.offset + fix.length, UTF8.encode(fix.replacement));
+    }
+    await new File.fromUri(uri).writeAsBytes(bytes);
+  }
+
+  /// Loads expectations from the source file located at [uri].
+  ///
+  /// Should be called before [finish].
+  Future<Null> loadExpectations(Uri uri) async {
+    var bytes = await readBytesFromFile(uri);
+    var expectations = _expectations.putIfAbsent(uri, () => {});
+    var testedFeaturesState = _testedFeaturesState.putIfAbsent(uri, () => {});
+    ScannerResult result = scan(bytes, includeComments: true);
+    for (Token token = result.tokens; !token.isEof; token = token.next) {
+      for (Token commentToken = token.precedingCommentTokens;
+          commentToken != null;
+          commentToken = commentToken.next) {

[ahe, 2017/04/19 11:38:29]

Perhaps we should add a forEachComment to ScannerResult?

[Paul Berry, 2017/04/19 13:20:10]

Fair enough.  I will do that as a follow-up CL.

[Paul Berry, 2017/04/19 15:31:20]

I looked into this and it's uglier than I expected, because we actually need to
know both the comment token and the non-comment token it's attached to.  Which
means the signature of ScannerResult.forEachComment would have to be something
like:

void forEachComment(void callback(Token commentToken, Token anchorToken));

IMHO it's not worth it so I'm going to leave it as is.

+        String lexeme = commentToken.lexeme;
+        if (lexeme.startsWith('/*@') && lexeme.endsWith('*/')) {
+          var expectation = lexeme.substring(3, lexeme.length - 2);
+          var equals = expectation.indexOf('=');
+          String property;
+          String value;
+          if (equals == -1) {
+            property = expectation;
+            value = '';
+          } else {
+            property = expectation.substring(0, equals);
+            value = expectation
+                .substring(equals + 1)
+                .replaceAllMapped(_ESCAPE_SEQUENCE, (m) => m.group(1));
+          }

[ahe, 2017/04/19 11:38:29]

I suggest adding:

property = property.trim();
value = value.trim();

[Paul Berry, 2017/04/19 13:20:10]

Done.

+          if (property == 'testedFeatures') {
+            Set<String> state = new Set<String>();
+            for (String category in value.split(',')) {

[ahe, 2017/04/19 11:38:29]

And here:

category = category.trim();

[Paul Berry, 2017/04/19 13:20:10]

Done.

+              // If an unrecognized category name is found, it is assumed to be
+              // just a property name.
+              state.addAll(_CATEGORIES[category] ?? [category]);
+            }
+            testedFeaturesState[commentToken.offset] = state;
+          } else {
+            var offset = token.charOffset;
+            var expectationsAtOffset =
+                expectations.putIfAbsent(offset, () => []);
+            expectationsAtOffset.add(new _Expectation(
+                property, value, commentToken.offset, commentToken.length));
+          }
+        }
+      }
+    }
+  }
+
+  @override
+  void record(
+      String property, Uri uri, int offset, InstrumentationValue value) {
+    var expectationsForUri = _expectations[uri];
+    if (expectationsForUri == null) return;
+    var expectationsAtOffset = expectationsForUri[offset];
+    if (expectationsAtOffset != null) {
+      for (int i = 0; i < expectationsAtOffset.length; i++) {
+        var expectation = expectationsAtOffset[i];
+        if (expectation.property == property) {
+          if (!value.matches(expectation.value)) {
+            _problemWithStack(
+                uri,
+                offset,
+                'expected $property=${expectation.value}, got '
+                '$property=${value.canonicalize()}',
+                new _Fix(expectation.commentOffset, expectation.commentLength,
+                    _makeExpectationComment(property, value)));
+          }
+          expectationsAtOffset.removeAt(i);
+          return;
+        }
+      }
+    }
+    // Unexpected property/value pair.  See if we should report.
+    if (_shouldCheck(property, uri, offset)) {
+      _problemWithStack(
+          uri,
+          offset,
+          'expected nothing, got $property=${value.canonicalize()}',
+          new _Fix(offset, 0, _makeExpectationComment(property, value)));
+    }
+  }
+
+  String _escape(String s) {
+    return s.replaceAll(r'\', r'\\').replaceAll('*/', r'*\/');
+  }
+
+  String _formatProblem(
+      Uri uri, int offset, String desc, StackTrace stackTrace) {
+    return '$uri:$offset: $desc${stackTrace == null ? '' : '\n$stackTrace'}';

[ahe, 2017/04/19 11:38:29]

Perhaps you can use format from ../messages.dart here.

[Paul Berry, 2017/04/19 13:20:10]

Done.

+  }
+
+  String _makeExpectationComment(String property, InstrumentationValue value) {
+    return '/*@$property=${_escape(value.canonicalize())}*/';
+  }
+
+  void _problem(Uri uri, int offset, String desc, _Fix fix) {
+    _problems.add(_formatProblem(uri, offset, desc, null));
+    _fixes.putIfAbsent(uri, () => []).add(fix);
+  }
+
+  void _problemWithStack(Uri uri, int offset, String desc, _Fix fix) {
+    try {
+      throw null;

[ahe, 2017/04/19 11:38:29]

We now have StackTrace.current.

[Paul Berry, 2017/04/19 13:20:10]

Done.

+    } catch (_, stackTrace) {
+      _problems.add(_formatProblem(uri, offset, desc, stackTrace));
+      _fixes.putIfAbsent(uri, () => []).add(fix);
+    }
+  }
+
+  bool _shouldCheck(String property, Uri uri, int offset) {
+    var state = false;
+    var testedFeaturesStateForUri = _testedFeaturesState[uri];
+    if (testedFeaturesStateForUri == null) return false;
+    for (int i in testedFeaturesStateForUri.keys) {
+      if (i > offset) break;
+      var testedFeaturesStateAtOffset = testedFeaturesStateForUri[i];
+      state = testedFeaturesStateAtOffset.contains(property);
+    }
+    return state;
+  }
+}
+
+class _Expectation {
+  final String property;
+  final String value;
+  final int commentOffset;
+  final int commentLength;
+
+  _Expectation(
+      this.property, this.value, this.commentOffset, this.commentLength);
+}
+
+class _Fix {
+  final int offset;
+  final int length;
+  final String replacement;
+
+  _Fix(this.offset, this.length, this.replacement);
+}