Skip to content

Commit

Permalink
Revert support for simple nullable type return value in generalized f…
Browse files Browse the repository at this point in the history
…unction type

This reverts commit 11d081d

Reason for revert: Breaks parsing less common conditionals (e.g. b ? c = true : g();)

Original change's description:
> Add support for simple nullable type return value in generalized function type
>
> This only supports nullable return values of the form
>
> <identifier> '?' 'Function' '(' ...
>
> This is an increment CL in the ongoing effort to add nullable type support
> as outlined in dart-lang/language#110

Change-Id: I99bce29619d4e448193e3c81fa86a982791b1f77
Reviewed-on: https://dart-review.googlesource.com/c/87283
Reviewed-by: Dan Rubel <danrubel@google.com>
  • Loading branch information
danrubel committed Dec 14, 2018
1 parent 4949f89 commit e200712
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 115 deletions.
40 changes: 0 additions & 40 deletions pkg/analyzer/test/generated/parser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2013,46 +2013,6 @@ mixin ComplexParserTestMixin implements AbstractParserTestCase {
expect(elseExpression, new TypeMatcher<SimpleIdentifier>());
}

void test_conditionalExpression_precedence_nullableTypeWithTypeArg1_is() {
Expression expression = parseExpression('x is String<S> ? (x + y) : z');
expect(expression, isNotNull);
expect(expression, new TypeMatcher<ConditionalExpression>());
ConditionalExpression conditional = expression;
Expression condition = conditional.condition;
expect(condition, new TypeMatcher<IsExpression>());
Expression thenExpression = conditional.thenExpression;
expect(thenExpression, new TypeMatcher<ParenthesizedExpression>());
Expression elseExpression = conditional.elseExpression;
expect(elseExpression, new TypeMatcher<SimpleIdentifier>());
}

void test_conditionalExpression_precedence_nullableTypeWithTypeArg1GFT_is() {
Expression expression =
parseExpression('x is String<S> Function() ? (x + y) : z');
expect(expression, isNotNull);
expect(expression, new TypeMatcher<ConditionalExpression>());
ConditionalExpression conditional = expression;
Expression condition = conditional.condition;
expect(condition, new TypeMatcher<IsExpression>());
Expression thenExpression = conditional.thenExpression;
expect(thenExpression, new TypeMatcher<ParenthesizedExpression>());
Expression elseExpression = conditional.elseExpression;
expect(elseExpression, new TypeMatcher<SimpleIdentifier>());
}

void test_conditionalExpression_precedence_nullableTypeWithTypeArg2_is() {
Expression expression = parseExpression('x is String<S,T> ? (x + y) : z');
expect(expression, isNotNull);
expect(expression, new TypeMatcher<ConditionalExpression>());
ConditionalExpression conditional = expression;
Expression condition = conditional.condition;
expect(condition, new TypeMatcher<IsExpression>());
Expression thenExpression = conditional.thenExpression;
expect(thenExpression, new TypeMatcher<ParenthesizedExpression>());
Expression elseExpression = conditional.elseExpression;
expect(elseExpression, new TypeMatcher<SimpleIdentifier>());
}

void test_constructor_initializer_withParenthesizedExpression() {
CompilationUnit unit = parseCompilationUnit(r'''
class C {
Expand Down
23 changes: 13 additions & 10 deletions pkg/front_end/lib/src/fasta/parser/type_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ TypeInfo computeType(final Token token, bool required,
if (isGeneralizedFunctionType(next)) {
// `Function` ...
return new ComplexTypeInfo(token, noTypeParamOrArg)
.computeNoTypeGFT(token, required);
.computeNoTypeGFT(required);
}

// We've seen an identifier.
Expand Down Expand Up @@ -262,17 +262,20 @@ TypeInfo computeType(final Token token, bool required,
}

if (optional('?', next)) {
next = next.next;
if (isGeneralizedFunctionType(next)) {
// identifier `?` Function `(`
return new ComplexTypeInfo(token, noTypeParamOrArg)
.computeIdentifierQuestionGFT(required);
} else if (required ||
(looksLikeName(next) &&
isOneOfOrEof(
next.next, const [';', ',', '=', '>', '>=', '>>', '>>>']))) {
if (required) {
// identifier `?`
return simpleNullableType;
} else {
next = next.next;
if (isGeneralizedFunctionType(next)) {
// identifier `?` Function `(`
return simpleNullableType;
} else if (looksLikeName(next) &&
isOneOfOrEof(
next.next, const [';', ',', '=', '>', '>=', '>>', '>>>'])) {
// identifier `?` identifier `=`
return simpleNullableType;
}
}
} else if (required || looksLikeName(next)) {
// identifier identifier
Expand Down
61 changes: 9 additions & 52 deletions pkg/front_end/lib/src/fasta/parser/type_info_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,6 @@ class ComplexTypeInfo implements TypeInfo {
/// Type arguments were seen during analysis.
final TypeParamOrArgInfo typeArguments;

/// The token before the trailing question mark or `null` if none.
Token beforeQuestionMark;

/// The last token in the type reference.
Token end;

Expand All @@ -393,18 +390,12 @@ class ComplexTypeInfo implements TypeInfo {
ComplexTypeInfo(Token beforeStart, this.typeArguments)
: this.start = beforeStart.next;

ComplexTypeInfo._nonNullable(this.start, this.typeArguments, this.end,
this.typeVariableStarters, this.gftHasReturnType);

@override
bool get couldBeExpression => false;

@override
TypeInfo asNonNullableType() {
return beforeQuestionMark == null
? this
: new ComplexTypeInfo._nonNullable(start, typeArguments,
beforeQuestionMark, typeVariableStarters, gftHasReturnType);
return this;
}

@override
Expand Down Expand Up @@ -461,15 +452,7 @@ class ComplexTypeInfo implements TypeInfo {
}
}
token = typeArguments.parseArguments(token, parser);
Token questionMark = token.next;
if (optional('?', questionMark) &&
(typeVariableEndGroups.isNotEmpty || beforeQuestionMark != null)) {
// Only consume the `?` if it is part of the complex type
token = questionMark;
} else {
questionMark = null;
}
parser.listener.handleType(typeRefOrPrefix, questionMark);
parser.listener.handleType(typeRefOrPrefix, null);
}
}

Expand Down Expand Up @@ -509,11 +492,10 @@ class ComplexTypeInfo implements TypeInfo {

/// Given `Function` non-identifier, compute the type
/// and return the receiver or one of the [TypeInfo] constants.
TypeInfo computeNoTypeGFT(Token beforeStart, bool required) {
TypeInfo computeNoTypeGFT(bool required) {
assert(optional('Function', start));
assert(beforeStart.next == start);

computeRest(beforeStart, required);
computeRest(start, required);
if (gftHasReturnType == null) {
return required ? simpleType : noType;
}
Expand All @@ -527,7 +509,7 @@ class ComplexTypeInfo implements TypeInfo {
assert(optional('void', start));
assert(optional('Function', start.next));

computeRest(start, required);
computeRest(start.next, required);
if (gftHasReturnType == null) {
return voidType;
}
Expand All @@ -541,36 +523,21 @@ class ComplexTypeInfo implements TypeInfo {
assert(isValidTypeReference(start));
assert(optional('Function', start.next));

computeRest(start, required);
computeRest(start.next, required);
if (gftHasReturnType == null) {
return simpleType;
}
assert(end != null);
return this;
}

/// Given identifier `?` `Function` non-identifier, compute the type
/// and return the receiver or one of the [TypeInfo] constants.
TypeInfo computeIdentifierQuestionGFT(bool required) {
assert(isValidTypeReference(start));
assert(optional('?', start.next));
assert(optional('Function', start.next.next));

computeRest(start, required);
if (gftHasReturnType == null) {
return simpleNullableType;
}
assert(end != null);
return this;
}

/// Given a builtin, return the receiver so that parseType will report
/// an error for the builtin used as a type.
TypeInfo computeBuiltinOrVarAsType(bool required) {
assert(start.type.isBuiltIn || optional('var', start));

end = typeArguments.skip(start);
computeRest(end, required);
computeRest(end.next, required);
assert(end != null);
return this;
}
Expand All @@ -583,7 +550,7 @@ class ComplexTypeInfo implements TypeInfo {
assert(typeArguments != noTypeParamOrArg);

end = typeArguments.skip(start);
computeRest(end, required);
computeRest(end.next, required);

if (!required && !looksLikeName(end.next) && gftHasReturnType == null) {
return noType;
Expand All @@ -607,7 +574,7 @@ class ComplexTypeInfo implements TypeInfo {
}

end = typeArguments.skip(token);
computeRest(end, required);
computeRest(end.next, required);
if (!required && !looksLikeName(end.next) && gftHasReturnType == null) {
return noType;
}
Expand All @@ -616,11 +583,6 @@ class ComplexTypeInfo implements TypeInfo {
}

void computeRest(Token token, bool required) {
if (optional('?', token.next)) {
beforeQuestionMark = token;
end = token = token.next;
}
token = token.next;
while (optional('Function', token)) {
Token typeVariableStart = token;
// TODO(danrubel): Consider caching TypeParamOrArgInfo
Expand All @@ -641,14 +603,9 @@ class ComplexTypeInfo implements TypeInfo {
assert(optional(')', token));
gftHasReturnType ??= typeVariableStart != start;
typeVariableStarters = typeVariableStarters.prepend(typeVariableStart);
beforeQuestionMark = null;
end = token;
token = token.next;
}
if (optional('?', token)) {
beforeQuestionMark = end;
end = token;
}
}
}

Expand Down
13 changes: 0 additions & 13 deletions pkg/front_end/test/fasta/parser/type_info_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -800,19 +800,6 @@ class TypeInfoTest {
]);
}

void test_computeType_identifierComplex_questionMark() {
expectComplexInfo('C? Function()', required: true, expectedCalls: [
'handleNoTypeVariables (',
'beginFunctionType C',
'handleIdentifier C typeReference',
'handleNoTypeArguments ?',
'handleType C ?',
'beginFormalParameters ( MemberKind.GeneralizedFunctionType',
'endFormalParameters 0 ( ) MemberKind.GeneralizedFunctionType',
'endFunctionType Function null',
]);
}

void test_computeType_identifierTypeArg() {
expectComplexInfo('C<void>', required: true, expectedCalls: [
'handleIdentifier C typeReference',
Expand Down

0 comments on commit e200712

Please sign in to comment.