Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(137)

Unified Diff: pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart

Issue 2824393002: Introduce a testing framework for use with fasta type inference. (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart
diff --git a/pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart b/pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart
new file mode 100644
index 0000000000000000000000000000000000000000..159cc972d892cdee1c3353663146182aa29158fb
--- /dev/null
+++ b/pkg/front_end/lib/src/fasta/testing/validating_instrumentation.dart
@@ -0,0 +1,224 @@
+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 usin
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 readBy
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 cl
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 h
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 ScannerR
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
+ 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(); val
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 h
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);
+}

Powered by Google App Engine
This is Rietveld 408576698