Update AstBuilder unified collection parsing to reduce error messages

Change-Id: Ib1af352a7f957c775064eeaa946a401e1024c90d
Reviewed-on: https://dart-review.googlesource.com/c/93847
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
This commit is contained in:
Dan Rubel 2019-02-21 18:14:09 +00:00 committed by commit-bot@chromium.org
parent e3b8065625
commit bc53c4dcda
2 changed files with 75 additions and 56 deletions

View file

@ -55,6 +55,8 @@ import 'package:front_end/src/fasta/source/stack_listener.dart'
show NullValue, StackListener;
import 'package:kernel/ast.dart' show AsyncMarker;
const _invalidCollectionElement = const _InvalidCollectionElement._();
/// A parser listener that builds the analyzer's AST structure.
class AstBuilder extends StackListener {
final AstFactory ast = standard.astFactory;
@ -312,7 +314,10 @@ class AstBuilder extends StackListener {
CollectionElement thenElement,
Token elseToken,
CollectionElement elseElement) {
if (enableControlFlowCollections) {
if (thenElement == _invalidCollectionElement ||
elseElement == _invalidCollectionElement) {
push(_invalidCollectionElement);
} else if (enableControlFlowCollections) {
push(ast.ifElement(
ifKeyword: ifToken,
leftParenthesis: condition.leftParenthesis,
@ -323,19 +328,24 @@ class AstBuilder extends StackListener {
elseElement: elseElement,
));
} else {
// TODO(danrubel): Improve error message by indicating to the user
// that they need to enable this experiment.
handleRecoverableError(
templateUnexpectedToken.withArguments(ifToken), ifToken, ifToken);
push(thenElement);
push(_invalidCollectionElement);
}
}
@override
void handleSpreadExpression(Token spreadToken) {
var expression = pop();
if (enableSpreadCollections) {
push(ast.spreadElement(spreadOperator: spreadToken, expression: pop()));
push(ast.spreadElement(
spreadOperator: spreadToken, expression: expression));
} else {
handleRecoverableError(templateUnexpectedToken.withArguments(spreadToken),
spreadToken, spreadToken);
push(_invalidCollectionElement);
}
}
@ -928,7 +938,9 @@ class AstBuilder extends StackListener {
void pushForControlFlowInfo(Token awaitToken, Token forToken,
Token leftParenthesis, ForLoopParts forLoopParts, Object entry) {
if (enableControlFlowCollections) {
if (entry == _invalidCollectionElement) {
push(_invalidCollectionElement);
} else if (enableControlFlowCollections) {
push(ast.forElement(
awaitKeyword: awaitToken,
forKeyword: forToken,
@ -940,7 +952,7 @@ class AstBuilder extends StackListener {
} else {
handleRecoverableError(
templateUnexpectedToken.withArguments(forToken), forToken, forToken);
push(entry);
push(_invalidCollectionElement);
}
}
@ -991,13 +1003,27 @@ class AstBuilder extends StackListener {
if (enableControlFlowCollections || enableSpreadCollections) {
List<CollectionElement> elements = popCollectionElements(count);
TypeArgumentList typeArguments = pop();
// TODO(danrubel): Remove this and _InvalidCollectionElement
// once control flow and spread collection support is enabled by default
elements.removeWhere((e) => e == _invalidCollectionElement);
push(ast.listLiteral(
constKeyword, typeArguments, leftBracket, elements, rightBracket));
} else {
List<Expression> expressions = popTypedList(count);
List<dynamic> elements = popTypedList(count);
TypeArgumentList typeArguments = pop();
List<Expression> expressions = <Expression>[];
if (elements != null) {
for (var elem in elements) {
if (elem is Expression) {
expressions.add(elem);
}
}
}
push(ast.listLiteral(
constKeyword, typeArguments, leftBracket, expressions, rightBracket));
}
@ -1048,6 +1074,11 @@ class AstBuilder extends StackListener {
int count, Token leftBrace, Token constKeyword, Token rightBrace) {
if (enableControlFlowCollections || enableSpreadCollections) {
List<CollectionElement> elements = popCollectionElements(count);
// TODO(danrubel): Remove this and _InvalidCollectionElement
// once control flow and spread collection support is enabled by default
elements.removeWhere((e) => e == _invalidCollectionElement);
TypeArgumentList typeArguments = pop();
push(ast.setOrMapLiteral(
constKeyword: constKeyword,
@ -1057,7 +1088,7 @@ class AstBuilder extends StackListener {
rightBracket: rightBrace,
));
} else {
List elements = popTypedList(count);
List<dynamic> elements = popTypedList(count);
TypeArgumentList typeArguments = pop();
// Replicate existing behavior that has been removed from the parser.
@ -1072,7 +1103,7 @@ class AstBuilder extends StackListener {
if (elem is MapLiteralEntry) {
isSet = false;
break;
} else {
} else if (elem is Expression) {
isSet = true;
break;
}
@ -1090,7 +1121,7 @@ class AstBuilder extends StackListener {
templateUnexpectedToken.withArguments(elem.separator),
elem.separator,
elem.separator);
} else {
} else if (elem is Expression) {
setEntries.add(elem);
}
}
@ -1103,8 +1134,8 @@ class AstBuilder extends StackListener {
for (var elem in elements) {
if (elem is MapLiteralEntry) {
mapEntries.add(elem);
} else {
Token next = (elem as Expression).endToken.next;
} else if (elem is Expression) {
Token next = elem.endToken.next;
int offset = next.offset;
handleRecoverableError(
templateExpectedButGot.withArguments(':'), next, next);
@ -3189,6 +3220,18 @@ class AstBuilder extends StackListener {
}
}
/// When [enableSpreadCollections] and/or [enableControlFlowCollections]
/// are false, this class is pushed on the stack when a disabled
/// [CollectionElement] has been parsed.
class _InvalidCollectionElement implements CollectionElement {
// TODO(danrubel): Remove this once control flow and spread collections
// have been enabled by default.
const _InvalidCollectionElement._();
noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
/// Data structure placed on the stack to represent the default parameter
/// value with the separator token.
class _ParameterDefaultValue {

View file

@ -871,11 +871,9 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
ListLiteral list = parseExpression('[1, ...[2]]', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 4, 3),
]);
expect(list.elements2, hasLength(2));
expect(list.elements2, hasLength(1));
IntegerLiteral first = list.elements2[0];
expect(first.value, 1);
ListLiteral second = list.elements2[1];
expect(second.elements2, hasLength(1));
}
void test_listLiteral_spreadQ() {
@ -883,11 +881,9 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
ListLiteral list = parseExpression('[1, ...?[2]]', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 4, 4),
]);
expect(list.elements2, hasLength(2));
expect(list.elements2, hasLength(1));
IntegerLiteral first = list.elements2[0];
expect(first.value, 1);
ListLiteral second = list.elements2[1];
expect(second.elements2, hasLength(1));
}
void test_mapLiteral() {
@ -958,72 +954,60 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('{1: 2, ...{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 3),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 16, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 16, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments, isNull);
expect(map.entries, hasLength(2));
expect(map.entries, hasLength(1));
}
void test_mapLiteral_spread2_typed() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('<int, int>{1: 2, ...{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 17, 3),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 26, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 26, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments.arguments, hasLength(2));
expect(map.entries, hasLength(2));
expect(map.entries, hasLength(1));
}
void test_mapLiteral_spread_typed() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('<int, int>{...{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 11, 3),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 20, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 20, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments.arguments, hasLength(2));
expect(map.entries, hasLength(1));
expect(map.entries, hasLength(0));
}
void test_mapLiteral_spreadQ() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('{1: 2, ...?{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 7, 4),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 17, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 17, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments, isNull);
expect(map.entries, hasLength(2));
expect(map.entries, hasLength(1));
}
void test_mapLiteral_spreadQ2_typed() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('<int, int>{1: 2, ...?{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 17, 4),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 27, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 27, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments.arguments, hasLength(2));
expect(map.entries, hasLength(2));
expect(map.entries, hasLength(1));
}
void test_mapLiteral_spreadQ_typed() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
MapLiteral map = parseExpression('<int, int>{...?{3: 4}}', errors: [
expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 11, 4),
expectedError(ParserErrorCode.EXPECTED_TOKEN, 21, 1),
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 21, 1),
]);
expect(map.constKeyword, isNull);
expect(map.typeArguments.arguments, hasLength(2));
expect(map.entries, hasLength(1));
expect(map.entries, hasLength(0));
}
@override
@ -1108,11 +1092,9 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 4, 3)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNull);
expect(set.elements, hasLength(2));
expect(set.elements, hasLength(1));
IntegerLiteral value = set.elements[0];
expect(value.value, 3);
ListLiteral list = set.elements[1];
expect(list.elements2, hasLength(1));
}
void test_setLiteral_spread2Q() {
@ -1121,11 +1103,9 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 4, 4)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNull);
expect(set.elements, hasLength(2));
expect(set.elements, hasLength(1));
IntegerLiteral value = set.elements[0];
expect(value.value, 3);
ListLiteral list = set.elements[1];
expect(list.elements2, hasLength(1));
}
void test_setLiteral_spread_typed() {
@ -1134,9 +1114,7 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 6, 3)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNotNull);
expect(set.elements, hasLength(1));
ListLiteral list = set.elements[0];
expect(list.elements2, hasLength(1));
expect(set.elements, hasLength(0));
}
void test_setLiteral_spreadQ_typed() {
@ -1145,9 +1123,7 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 6, 4)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNotNull);
expect(set.elements, hasLength(1));
ListLiteral list = set.elements[0];
expect(list.elements2, hasLength(1));
expect(set.elements, hasLength(0));
}
void test_setLiteral_typed() {
@ -1163,20 +1139,20 @@ class ExpressionParserTest_Fasta extends FastaParserTestCase
void test_setOrMapLiteral_spread() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
SetLiteral set = parseExpression('{...{3: 4}}',
MapLiteral map = parseExpression('{...{3: 4}}',
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 1, 3)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNull);
expect(set.elements, hasLength(1));
expect(map.constKeyword, isNull);
expect(map.typeArguments, isNull);
expect(map.entries, hasLength(0));
}
void test_setOrMapLiteral_spreadQ() {
// TODO(danrubel): Remove this once spread_collections is enabled by default
SetLiteral set = parseExpression('{...?{3: 4}}',
MapLiteral map = parseExpression('{...?{3: 4}}',
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 1, 4)]);
expect(set.constKeyword, isNull);
expect(set.typeArguments, isNull);
expect(set.elements, hasLength(1));
expect(map.constKeyword, isNull);
expect(map.typeArguments, isNull);
expect(map.entries, hasLength(0));
}
}