From 230cfc209cdb0302b18a3e16127295da4d6b0329 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Mon, 20 Apr 2015 17:38:29 -0700
Subject: [PATCH] Refactor parseMetadata.
This moves most of the functionality into reusable helper methods, to make it
easier to add new metadata in the future.
R=kevmoo@google.com
Review URL: https://codereview.chromium.org//1093313002
---
lib/src/runner/parse_metadata.dart | 527 +++++++++++++++--------------
1 file changed, 274 insertions(+), 253 deletions(-)
diff --git a/lib/src/runner/parse_metadata.dart b/lib/src/runner/parse_metadata.dart
index 21ed6e2c..e4e40796 100644
--- a/lib/src/runner/parse_metadata.dart
+++ b/lib/src/runner/parse_metadata.dart
@@ -15,310 +15,331 @@ import '../backend/metadata.dart';
import '../frontend/timeout.dart';
import '../util/dart.dart';
-/// The valid argument names for [new Duration].
-const _durationArgs = const [
- "days",
- "hours",
- "minutes",
- "seconds",
- "milliseconds",
- "microseconds"
-];
-
/// Parse the test metadata for the test file at [path].
///
/// Throws an [AnalysisError] if parsing fails or a [FormatException] if the
/// test annotations are incorrect.
-Metadata parseMetadata(String path) {
- var timeout;
- var testOn;
- var skip;
-
- var contents = new File(path).readAsStringSync();
- var directives = parseDirectives(contents, name: path).directives;
- var annotations = directives.isEmpty ? [] : directives.first.metadata;
-
- // We explicitly *don't* just look for "package:test" imports here,
- // because it could be re-exported from another library.
- var prefixes = directives.map((directive) {
- if (directive is! ImportDirective) return null;
- if (directive.prefix == null) return null;
- return directive.prefix.name;
- }).where((prefix) => prefix != null).toSet();
-
- for (var annotation in annotations) {
- // The annotation syntax is ambiguous between named constructors and
- // prefixed annotations, so we need to resolve that ambiguity using the
- // known prefixes. The analyzer parses "@x.y()" as prefix "x", annotation
- // "y", and named constructor null. It parses "@x.y.z()" as prefix "x",
- // annotation "y", and named constructor "z".
- var name;
- var constructorName;
- var identifier = annotation.name;
- if (identifier is PrefixedIdentifier &&
- !prefixes.contains(identifier.prefix.name) &&
- annotation.constructorName == null) {
- name = identifier.prefix.name;
- constructorName = identifier.identifier.name;
- } else {
- name = identifier is PrefixedIdentifier
- ? identifier.identifier.name
- : identifier.name;
- if (annotation.constructorName != null) {
- constructorName = annotation.constructorName.name;
- }
- }
+Metadata parseMetadata(String path) => new _Parser(path).parse();
+
+/// A parser for test suite metadata.
+class _Parser {
+ /// The path to the test suite.
+ final String _path;
+
+ /// All annotations at the top of the file.
+ List<Annotation> _annotations;
+
+ /// All prefixes defined by imports in this file.
+ Set<String> _prefixes;
+
+ _Parser(String path)
+ : _path = path {
+ var contents = new File(path).readAsStringSync();
+ var directives = parseDirectives(contents, name: path).directives;
+ _annotations = directives.isEmpty ? [] : directives.first.metadata;
+
+ // We explicitly *don't* just look for "package:test" imports here,
+ // because it could be re-exported from another library.
+ _prefixes = directives.map((directive) {
+ if (directive is! ImportDirective) return null;
+ if (directive.prefix == null) return null;
+ return directive.prefix.name;
+ }).where((prefix) => prefix != null).toSet();
+ }
- if (name == 'TestOn') {
- if (testOn != null) {
- throw new SourceSpanFormatException(
- "Only a single TestOn annotation may be used for a given test file.",
- _spanFor(annotation, path));
+ /// Parses the metadata.
+ Metadata parse() {
+ var timeout;
+ var testOn;
+ var skip;
+
+ for (var annotation in _annotations) {
+ // The annotation syntax is ambiguous between named constructors and
+ // prefixed annotations, so we need to resolve that ambiguity using the
+ // known prefixes. The analyzer parses "@x.y()" as prefix "x", annotation
+ // "y", and named constructor null. It parses "@x.y.z()" as prefix "x",
+ // annotation "y", and named constructor "z".
+ var name;
+ var constructorName;
+ var identifier = annotation.name;
+ if (identifier is PrefixedIdentifier &&
+ !_prefixes.contains(identifier.prefix.name) &&
+ annotation.constructorName == null) {
+ name = identifier.prefix.name;
+ constructorName = identifier.identifier.name;
+ } else {
+ name = identifier is PrefixedIdentifier
+ ? identifier.identifier.name
+ : identifier.name;
+ if (annotation.constructorName != null) {
+ constructorName = annotation.constructorName.name;
+ }
}
- testOn = _parseTestOn(annotation, constructorName, path);
- } else if (name == 'Timeout') {
- if (timeout != null) {
- throw new SourceSpanFormatException(
- "Only a single Timeout annotation may be used for a given test file.",
- _spanFor(annotation, path));
- }
- timeout = _parseTimeout(annotation, constructorName, path);
- } else if (name == 'Skip') {
- if (skip != null) {
- throw new SourceSpanFormatException(
- "Only a single Skip annotation may be used for a given test file.",
- _spanFor(annotation, path));
+
+ if (name == 'TestOn') {
+ _assertSingleAnnotation(testOn, 'TestOn', annotation);
+ testOn = _parseTestOn(annotation, constructorName);
+ } else if (name == 'Timeout') {
+ _assertSingleAnnotation(timeout, 'Timeout', annotation);
+ timeout = _parseTimeout(annotation, constructorName);
+ } else if (name == 'Skip') {
+ _assertSingleAnnotation(skip, 'Skip', annotation);
+ skip = _parseSkip(annotation, constructorName);
}
- skip = _parseSkip(annotation, constructorName, path);
}
- }
- try {
- return new Metadata.parse(
- testOn: testOn == null ? null : testOn.stringValue,
- timeout: timeout,
- skip: skip);
- } on SourceSpanFormatException catch (error) {
- var file = new SourceFile(new File(path).readAsStringSync(),
- url: p.toUri(path));
- var span = contextualizeSpan(error.span, testOn, file);
- if (span == null) rethrow;
- throw new SourceSpanFormatException(error.message, span);
+ try {
+ return new Metadata.parse(
+ testOn: testOn == null ? null : testOn.stringValue,
+ timeout: timeout,
+ skip: skip);
+ } on SourceSpanFormatException catch (error) {
+ var file = new SourceFile(new File(_path).readAsStringSync(),
+ url: p.toUri(_path));
+ var span = contextualizeSpan(error.span, testOn, file);
+ if (span == null) rethrow;
+ throw new SourceSpanFormatException(error.message, span);
+ }
}
-}
-/// Parses a `@TestOn` annotation.
-///
-/// [annotation] is the annotation. [constructorName] is the name of the named
-/// constructor for the annotation, if any. [path] is the path to the file from
-/// which the annotation was parsed.
-StringLiteral _parseTestOn(Annotation annotation, String constructorName,
- String path) {
- if (constructorName != null) {
- throw new SourceSpanFormatException(
- 'TestOn doesn\'t have a constructor named "$constructorName".',
- _spanFor(annotation, path));
+ /// Parses a `@TestOn` annotation.
+ ///
+ /// [annotation] is the annotation. [constructorName] is the name of the named
+ /// constructor for the annotation, if any.
+ StringLiteral _parseTestOn(Annotation annotation, String constructorName) {
+ _assertConstructorName(constructorName, 'TestOn', annotation);
+ _assertArguments(annotation.arguments, 'TestOn', annotation, positional: 1);
+ return _parseString(annotation.arguments.arguments.first);
}
- if (annotation.arguments == null) {
- throw new SourceSpanFormatException(
- 'TestOn takes one argument.', _spanFor(annotation, path));
- }
+ /// Parses a `@Timeout` annotation.
+ ///
+ /// [annotation] is the annotation. [constructorName] is the name of the named
+ /// constructor for the annotation, if any.
+ Timeout _parseTimeout(Annotation annotation, String constructorName) {
+ _assertConstructorName(constructorName, 'Timeout', annotation,
+ validNames: [null, 'factor']);
- var args = annotation.arguments.arguments;
- if (args.isEmpty) {
- throw new SourceSpanFormatException(
- 'TestOn takes one argument.', _spanFor(annotation.arguments, path));
- }
+ var description = 'Timeout';
+ if (constructorName != null) description += '.$constructorName';
- if (args.first is NamedExpression) {
- throw new SourceSpanFormatException(
- "TestOn doesn't take named parameters.", _spanFor(args.first, path));
- }
+ _assertArguments(annotation.arguments, description, annotation,
+ positional: 1);
- if (args.length > 1) {
- throw new SourceSpanFormatException(
- "TestOn takes only one argument.",
- _spanFor(annotation.arguments, path));
+ var args = annotation.arguments.arguments;
+ if (constructorName == null) return new Timeout(_parseDuration(args.first));
+ return new Timeout.factor(_parseNum(args.first));
}
- if (args.first is! StringLiteral) {
- throw new SourceSpanFormatException(
- "TestOn takes a String.", _spanFor(args.first, path));
+ /// Parses a `@Skip` annotation.
+ ///
+ /// [annotation] is the annotation. [constructorName] is the name of the named
+ /// constructor for the annotation, if any.
+ ///
+ /// Returns either `true` or a reason string.
+ _parseSkip(Annotation annotation, String constructorName) {
+ _assertConstructorName(constructorName, 'Skip', annotation);
+ _assertArguments(annotation.arguments, 'Skip', annotation, optional: 1);
+
+ var args = annotation.arguments.arguments;
+ return args.isEmpty ? true : _parseString(args.first).stringValue;
}
- return args.first;
-}
+ /// Parses a `const Duration` expression.
+ Duration _parseDuration(Expression expression) {
+ _parseConstructor(expression, 'Duration');
-/// Parses a `@Timeout` annotation.
-///
-/// [annotation] is the annotation. [constructorName] is the name of the named
-/// constructor for the annotation, if any. [path] is the path to the file from
-/// which the annotation was parsed.
-Timeout _parseTimeout(Annotation annotation, String constructorName,
- String path) {
- if (constructorName != null && constructorName != 'factor') {
- throw new SourceSpanFormatException(
- 'Timeout doesn\'t have a constructor named "$constructorName".',
- _spanFor(annotation, path));
- }
+ var constructor = expression as InstanceCreationExpression;
+ var values = _assertArguments(
+ constructor.argumentList, 'Duration', constructor, named: [
+ 'days', 'hours', 'minutes', 'seconds', 'milliseconds', 'microseconds'
+ ]);
- var description = 'Timeout';
- if (constructorName != null) description += '.$constructorName';
+ for (var key in values.keys.toList()) {
+ if (values.containsKey(key)) values[key] = _parseInt(values[key]);
+ }
- if (annotation.arguments == null) {
- throw new SourceSpanFormatException(
- '$description takes one argument.', _spanFor(annotation, path));
+ return new Duration(
+ days: values["days"] == null ? 0 : values["days"],
+ hours: values["hours"] == null ? 0 : values["hours"],
+ minutes: values["minutes"] == null ? 0 : values["minutes"],
+ seconds: values["seconds"] == null ? 0 : values["seconds"],
+ milliseconds: values["milliseconds"] == null ? 0 : values["milliseconds"],
+ microseconds:
+ values["microseconds"] == null ? 0 : values["microseconds"]);
}
- var args = annotation.arguments.arguments;
- if (args.isEmpty) {
+ /// Asserts that [existing] is null.
+ ///
+ /// [name] is the name of the annotation and [node] is its location, used for
+ /// error reporting.
+ void _assertSingleAnnotation(Object existing, String name, AstNode node) {
+ if (existing == null) return;
throw new SourceSpanFormatException(
- '$description takes one argument.',
- _spanFor(annotation.arguments, path));
+ "Only a single $name annotation may be used for a given test file.",
+ _spanFor(node));
}
- if (args.first is NamedExpression) {
- throw new SourceSpanFormatException(
- "$description doesn't take named parameters.",
- _spanFor(args.first, path));
+ /// Asserts that [constructorName] is a valid constructor name for an AST
+ /// node.
+ ///
+ /// [nodeName] is the name of the class being constructed, and [node] is the
+ /// AST node for that class. [validNames], if passed, is the set of valid
+ /// constructor names; if an unnamed constructor is valid, it should include
+ /// `null`. By default, only an unnamed constructor is allowed.
+ void _assertConstructorName(String constructorName, String nodeName,
+ AstNode node, {Iterable<String> validNames}) {
+ if (validNames == null) validNames = [null];
+ if (validNames.contains(constructorName)) return;
+
+ if (constructorName == null) {
+ throw new SourceSpanFormatException(
+ "$nodeName doesn't have an unnamed constructor.",
+ _spanFor(node));
+ } else {
+ throw new SourceSpanFormatException(
+ '$nodeName doesn\'t have a constructor named "$constructorName".',
+ _spanFor(node));
+ }
}
- if (args.length > 1) {
- throw new SourceSpanFormatException(
- "$description takes only one argument.",
- _spanFor(annotation.arguments, path));
- }
+ /// Parses a constructor invocation for [className].
+ ///
+ /// [validNames], if passed, is the set of valid constructor names; if an
+ /// unnamed constructor is valid, it should include `null`. By default, only
+ /// an unnamed constructor is allowed.
+ ///
+ /// Returns the name of the named constructor, if any.
+ String _parseConstructor(Expression expression, String className,
+ {Iterable<String> validNames}) {
+ if (validNames == null) validNames = [null];
+
+ if (expression is! InstanceCreationExpression) {
+ throw new SourceSpanFormatException(
+ "Expected a $className.", _spanFor(expression));
+ }
- if (constructorName == null) {
- return new Timeout(_parseDuration(args.first, path));
- } else {
- return new Timeout.factor(_parseNum(args.first, path));
- }
-}
+ var constructor = expression as InstanceCreationExpression;
+ if (constructor.constructorName.type.name.name != className) {
+ throw new SourceSpanFormatException(
+ "Expected a $className.", _spanFor(constructor));
+ }
-/// Parses a `@Skip` annotation.
-///
-/// [annotation] is the annotation. [constructorName] is the name of the named
-/// constructor for the annotation, if any. [path] is the path to the file from
-/// which the annotation was parsed.
-///
-/// Returns either `true` or a reason string.
-_parseSkip(Annotation annotation, String constructorName, String path) {
- if (constructorName != null) {
- throw new SourceSpanFormatException(
- 'Skip doesn\'t have a constructor named "$constructorName".',
- _spanFor(annotation, path));
- }
+ if (constructor.keyword.lexeme != "const") {
+ throw new SourceSpanFormatException(
+ "$className must use a const constructor.", _spanFor(constructor));
+ }
- if (annotation.arguments == null) {
- throw new SourceSpanFormatException(
- 'Skip must have parentheses.', _spanFor(annotation, path));
+ var name = constructor.constructorName == null
+ ? null
+ : constructor.constructorName.name;
+ _assertConstructorName(name, className, expression,
+ validNames: validNames);
+ return name;
}
- var args = annotation.arguments.arguments;
- if (args.length > 1) {
- throw new SourceSpanFormatException(
- 'Skip takes zero arguments or one argument.',
- _spanFor(annotation.arguments, path));
- }
+ /// Assert that [arguments] is a valid argument list.
+ ///
+ /// [name] describes the function and [node] is its AST node. [positional] is
+ /// the number of required positional arguments, [optional] the number of
+ /// optional positional arguments, and [named] the set of valid argument
+ /// names.
+ ///
+ /// The set of parsed named arguments is returned.
+ Map<String, Expression> _assertArguments(ArgumentList arguments, String name,
+ AstNode node, {int positional, int optional, Iterable<String> named}) {
+ if (positional == null) positional = 0;
+ if (optional == null) optional = 0;
+ if (named == null) named = new Set();
+
+ if (arguments == null) {
+ throw new SourceSpanFormatException(
+ '$name takes arguments.', _spanFor(node));
+ }
- if (args.isEmpty) return true;
+ var actualNamed = arguments.arguments
+ .where((arg) => arg is NamedExpression).toList();
+ if (!actualNamed.isEmpty && named.isEmpty) {
+ throw new SourceSpanFormatException(
+ "$name doesn't take named arguments.", _spanFor(actualNamed.first));
+ }
- if (args.first is NamedExpression) {
- throw new SourceSpanFormatException(
- "Skip doesn't take named parameters.", _spanFor(args.first, path));
- }
+ var namedValues = {};
+ for (var argument in actualNamed) {
+ var argumentName = argument.name.label.name;
+ if (!named.contains(argumentName)) {
+ throw new SourceSpanFormatException(
+ '$name doesn\'t take an argument named "$argumentName".',
+ _spanFor(argument));
+ } else if (namedValues.containsKey(argumentName)) {
+ throw new SourceSpanFormatException(
+ 'An argument named "$argumentName" was already passed.',
+ _spanFor(argument));
+ } else {
+ namedValues[argumentName] = argument.expression;
+ }
+ }
- if (args.first is! StringLiteral) {
- throw new SourceSpanFormatException(
- "Skip takes a String.", _spanFor(args.first, path));
- }
+ var actualPositional = arguments.arguments.length - actualNamed.length;
+ if (actualPositional < positional) {
+ var buffer = new StringBuffer("$name takes ");
+ if (optional != 0) buffer.write("at least ");
+ buffer.write("$positional argument");
+ if (positional > 1) buffer.write("s");
+ buffer.write(".");
+ throw new SourceSpanFormatException(
+ buffer.toString(), _spanFor(arguments));
+ }
- return args.first.stringValue;
-}
+ if (actualPositional > positional + optional) {
+ if (optional + positional == 0) {
+ var buffer = new StringBuffer("$name doesn't take ");
+ if (!named.isEmpty) buffer.write("positional ");
+ buffer.write("arguments.");
+ throw new SourceSpanFormatException(
+ buffer.toString(), _spanFor(arguments));
+ }
-/// Parses a `const Duration` expression.
-Duration _parseDuration(Expression expression, String path) {
- if (expression is! InstanceCreationExpression) {
- throw new SourceSpanFormatException(
- "Expected a Duration.",
- _spanFor(expression, path));
- }
+ var buffer = new StringBuffer("$name takes ");
+ if (optional != 0) buffer.write("at most ");
+ buffer.write("${positional + optional} argument");
+ if (positional > 1) buffer.write("s");
+ buffer.write(".");
+ throw new SourceSpanFormatException(
+ buffer.toString(), _spanFor(arguments));
+ }
- var constructor = expression as InstanceCreationExpression;
- if (constructor.constructorName.type.name.name != 'Duration') {
- throw new SourceSpanFormatException(
- "Expected a Duration.",
- _spanFor(constructor, path));
+ return namedValues;
}
- if (constructor.keyword.lexeme != "const") {
+ /// Parses a constant number literal.
+ num _parseNum(Expression expression) {
+ if (expression is IntegerLiteral) return expression.value;
+ if (expression is DoubleLiteral) return expression.value;
throw new SourceSpanFormatException(
- "Duration must use a const constructor.",
- _spanFor(constructor, path));
+ "Expected a number.", _spanFor(expression));
}
- if (constructor.constructorName.name != null) {
+ /// Parses a constant int literal.
+ int _parseInt(Expression expression) {
+ if (expression is IntegerLiteral) return expression.value;
throw new SourceSpanFormatException(
- "Duration doesn't have a constructor named "
- '"${constructor.constructorName}".',
- _spanFor(constructor.constructorName, path));
+ "Expected an integer.", _spanFor(expression));
}
- var values = {};
- var args = constructor.argumentList.arguments;
- for (var argument in args) {
- if (argument is! NamedExpression) {
- throw new SourceSpanFormatException(
- "Duration doesn't take positional arguments.",
- _spanFor(argument, path));
- }
-
- var name = argument.name.label.name;
- if (!_durationArgs.contains(name)) {
- throw new SourceSpanFormatException(
- 'Duration doesn\'t take an argument named "$name".',
- _spanFor(argument, path));
- }
-
- if (values.containsKey(name)) {
- throw new SourceSpanFormatException(
- 'An argument named "$name" was already passed.',
- _spanFor(argument, path));
- }
-
- values[name] = _parseInt(argument.expression, path);
+ /// Parses a constant String literal.
+ StringLiteral _parseString(Expression expression) {
+ if (expression is StringLiteral) return expression;
+ throw new SourceSpanFormatException(
+ "Expected a String.", _spanFor(expression));
}
- return new Duration(
- days: values["days"] == null ? 0 : values["days"],
- hours: values["hours"] == null ? 0 : values["hours"],
- minutes: values["minutes"] == null ? 0 : values["minutes"],
- seconds: values["seconds"] == null ? 0 : values["seconds"],
- milliseconds: values["milliseconds"] == null ? 0 : values["milliseconds"],
- microseconds:
- values["microseconds"] == null ? 0 : values["microseconds"]);
-}
-
-/// Parses a constant number literal.
-num _parseNum(Expression expression, String path) {
- if (expression is IntegerLiteral) return expression.value;
- if (expression is DoubleLiteral) return expression.value;
- throw new SourceSpanFormatException(
- "Expected a number.", _spanFor(expression, path));
-}
-
-/// Parses a constant int literal.
-int _parseInt(Expression expression, String path) {
- if (expression is IntegerLiteral) return expression.value;
- throw new SourceSpanFormatException(
- "Expected an integer.", _spanFor(expression, path));
-}
-
-/// Creates a [SourceSpan] for [node].
-SourceSpan _spanFor(AstNode node, String path) =>
+ /// Creates a [SourceSpan] for [node].
+ SourceSpan _spanFor(AstNode node) {
// Load a SourceFile from scratch here since we're only ever going to emit
// one error per file anyway.
- new SourceFile(new File(path).readAsStringSync(), url: p.toUri(path))
+ var contents = new File(_path).readAsStringSync();
+ return new SourceFile(contents, url: p.toUri(_path))
.span(node.offset, node.end);
+ }
+}
--
GitLab