Skip to content

Commit

Permalink
Add support for parsing simple nullable types
Browse files Browse the repository at this point in the history
... as part of adding NNBD as outlined in
dart-lang/language#110

This only supports parsing simple nullable types
such as int? and List<int>? while subsequent CLs
will add support for parsing more complex types.

Due to current parser limitations, it also only supports
nullable types in a limited number of statements such as

T? t;
if (x is String?) {}

In addition, address comments in
* https://dart-review.googlesource.com/c/sdk/+/87880
* https://dart-review.googlesource.com/c/sdk/+/87881

Change-Id: Ife4afadc0b72906fc0c4e9b1977ad838041c2a84
Reviewed-on: https://dart-review.googlesource.com/c/87920
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
  • Loading branch information
danrubel authored and commit-bot@chromium.org committed Dec 21, 2018
1 parent aeb8731 commit f0377b2
Show file tree
Hide file tree
Showing 8 changed files with 391 additions and 41 deletions.
3 changes: 2 additions & 1 deletion pkg/analyzer/lib/src/dart/analysis/experiments.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ class ExperimentStatus {

/// Initializes a newly created set of experiments based on optional
/// arguments.
ExperimentStatus({bool constant_update_2018, bool set_literals, non_nullable})
ExperimentStatus(
{bool constant_update_2018, bool set_literals, bool non_nullable})
: _enableFlags = <bool>[
constant_update_2018 ?? IsEnabledByDefault.constant_update_2018,
set_literals ?? IsEnabledByDefault.set_literals,
Expand Down
8 changes: 5 additions & 3 deletions pkg/analyzer/lib/src/fasta/ast_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ class AstBuilder extends StackListener {
@override
void handleType(Token beginToken, Token questionMark) {
debugEvent("Type");
if (!enableNonNullable || showNullableTypeErrors) {
if (showNullableTypeErrors) {
reportErrorIfNullableType(questionMark);
}

Expand Down Expand Up @@ -1128,7 +1128,7 @@ class AstBuilder extends StackListener {
void endFunctionType(Token functionToken, Token questionMark) {
assert(optional('Function', functionToken));
debugEvent("FunctionType");
if (!enableNonNullable || showNullableTypeErrors) {
if (showNullableTypeErrors) {
reportErrorIfNullableType(questionMark);
}

Expand Down Expand Up @@ -2093,14 +2093,16 @@ class AstBuilder extends StackListener {
List<SimpleIdentifier> libraryName = pop();
var name = ast.libraryIdentifier(libraryName);
List<Annotation> metadata = pop();
if (metadata != null) {
if (enableNonNullable && metadata != null) {
for (Annotation annotation in metadata) {
Identifier pragma = annotation.name;
if (pragma is SimpleIdentifier && pragma.name == 'pragma') {
NodeList<Expression> arguments = annotation.arguments.arguments;
if (arguments.length == 1) {
Expression tag = arguments[0];
if (tag is StringLiteral) {
// TODO(danrubel): handle other flags such as
// 'analyzer:non-nullable' without the trailing star.
if (tag.stringValue == 'analyzer:non-nullable*') {
showNullableTypeErrors = false;
break;
Expand Down
70 changes: 70 additions & 0 deletions pkg/analyzer/test/generated/parser_fasta_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'package:analyzer/src/fasta/ast_builder.dart';
import 'package:analyzer/src/generated/parser.dart' as analyzer;
import 'package:analyzer/src/generated/utilities_dart.dart';
import 'package:analyzer/src/string_source.dart';
import 'package:analyzer/src/test_utilities/ast_type_matchers.dart';
import 'package:front_end/src/fasta/parser/parser.dart' as fasta;
import 'package:front_end/src/fasta/parser/forwarding_listener.dart' as fasta;
import 'package:front_end/src/fasta/scanner.dart'
Expand All @@ -35,6 +36,7 @@ main() {
defineReflectiveTests(ErrorParserTest_Fasta);
defineReflectiveTests(ExpressionParserTest_Fasta);
defineReflectiveTests(FormalParameterParserTest_Fasta);
defineReflectiveTests(NNBDParserTest_Fasta);
defineReflectiveTests(RecoveryParserTest_Fasta);
defineReflectiveTests(SimpleParserTest_Fasta);
defineReflectiveTests(StatementParserTest_Fasta);
Expand Down Expand Up @@ -1420,22 +1422,90 @@ mixin A {
MixinDeclaration declaration = parseFullCompilationUnitMember();
expectCommentText(declaration.documentationComment, '/// Doc');
}
}

/**
* Tests of the fasta parser based on [ComplexParserTestMixin].
*/
@reflectiveTest
class NNBDParserTest_Fasta extends FastaParserTestCase {
parseNNBDCompilationUnit(String code, {List<ExpectedError> errors}) {
createParser('''
@pragma('analyzer:non-nullable*') library nnbd.parser.test;
$code
''');
_parserProxy.astBuilder.enableNonNullable = true;
CompilationUnit unit = _parserProxy.parseCompilationUnit2();
assertNoErrors();
return unit;
}

void test_is_nullable() {
CompilationUnit unit =
parseNNBDCompilationUnit('main() { x is String? ? (x + y) : z; }');
FunctionDeclaration function = unit.declarations[0];
BlockFunctionBody body = function.functionExpression.body;
ExpressionStatement statement = body.block.statements[0];
ConditionalExpression expression = statement.expression;

IsExpression condition = expression.condition;
expect((condition.type as NamedType).question, isNotNull);
Expression thenExpression = expression.thenExpression;
expect(thenExpression, isParenthesizedExpression);
Expression elseExpression = expression.elseExpression;
expect(elseExpression, isSimpleIdentifier);
}

void test_is_nullable_parenthesis() {
CompilationUnit unit =
parseNNBDCompilationUnit('main() { (x is String?) ? (x + y) : z; }');
FunctionDeclaration function = unit.declarations[0];
BlockFunctionBody body = function.functionExpression.body;
ExpressionStatement statement = body.block.statements[0];
ConditionalExpression expression = statement.expression;

ParenthesizedExpression condition = expression.condition;
IsExpression isExpression = condition.expression;
expect((isExpression.type as NamedType).question, isNotNull);
Expression thenExpression = expression.thenExpression;
expect(thenExpression, isParenthesizedExpression);
Expression elseExpression = expression.elseExpression;
expect(elseExpression, isSimpleIdentifier);
}

void test_simple_assignment() {
parseNNBDCompilationUnit('D? foo(X? x) { X? x1; X? x2 = x; }');
}

void test_pragma_missing() {
createParser("library foo;");
_parserProxy.astBuilder.enableNonNullable = true;
_parserProxy.parseCompilationUnit2();
expect(_parserProxy.astBuilder.showNullableTypeErrors, true);
}

void test_pragma_non_nullable() {
createParser("@pragma('analyzer:non-nullable*') library foo;");
_parserProxy.astBuilder.enableNonNullable = true;
_parserProxy.parseCompilationUnit2();
expect(_parserProxy.astBuilder.showNullableTypeErrors, false);
}

void test_pragma_non_nullable_not_enabled() {
createParser("@pragma('analyzer:non-nullable*') library foo;");
_parserProxy.parseCompilationUnit2();
expect(_parserProxy.astBuilder.showNullableTypeErrors, true);
}

void test_pragma_other() {
createParser("@pragma('analyzer:foo') library foo;");
_parserProxy.astBuilder.enableNonNullable = true;
_parserProxy.parseCompilationUnit2();
expect(_parserProxy.astBuilder.showNullableTypeErrors, true);
}

void test_without_pragma() {
parseCompilationUnit('main() { x is String? ? (x + y) : z; }',
errors: [expectedError(ParserErrorCode.UNEXPECTED_TOKEN, 20, 1)]);
}
}
2 changes: 1 addition & 1 deletion pkg/analyzer/test/generated/parser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1981,7 +1981,7 @@ mixin ComplexParserTestMixin implements AbstractParserTestCase {
expect(elseExpression, isSimpleIdentifier);
}

void test_conditionalExpression_precedence_nullableType_is() {
void test_conditionalExpression_precedence_is() {
ExpressionStatement statement =
parseStatement('x is String ? (x + y) : z;');
ConditionalExpression expression = statement.expression;
Expand Down
28 changes: 18 additions & 10 deletions pkg/front_end/lib/src/fasta/parser/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4500,11 +4500,7 @@ class Parser {
Token name = beforeName.next;
if (name.isIdentifier) {
TypeParamOrArgInfo typeParam = computeTypeParamOrArg(name);
Token next = name;
if (typeParam != noTypeParamOrArg) {
next = typeParam.skip(next);
}
next = next.next;
Token next = typeParam.skip(name).next;
if (optional('(', next)) {
if (looksLikeFunctionBody(next.endGroup.next)) {
return parseFunctionLiteral(
Expand Down Expand Up @@ -4886,8 +4882,10 @@ class Parser {
if (optional('!', token.next)) {
not = token = token.next;
}
// Ignore trailing `?` if there is one as it may be part of an expression
TypeInfo typeInfo = computeType(token, true).asNonNullableType();
TypeInfo typeInfo = computeType(token, true);
if (typeInfo.isConditionalExpressionStart(token, this)) {
typeInfo = typeInfo.asNonNullable;
}
token = typeInfo.ensureTypeNotVoid(token, this);
listener.handleIsOperator(operator, not);
return skipChainedAsIsOperators(token);
Expand All @@ -4901,8 +4899,10 @@ class Parser {
Token parseAsOperatorRest(Token token) {
Token operator = token = token.next;
assert(optional('as', operator));
// Ignore trailing `?` if there is one as it may be part of an expression
TypeInfo typeInfo = computeType(token, true).asNonNullableType();
TypeInfo typeInfo = computeType(token, true);
if (typeInfo.isConditionalExpressionStart(token, this)) {
typeInfo = typeInfo.asNonNullable;
}
token = typeInfo.ensureTypeNotVoid(token, this);
listener.handleAsOperator(operator);
return skipChainedAsIsOperators(token);
Expand All @@ -4921,7 +4921,11 @@ class Parser {
if (optional('!', next.next)) {
next = next.next;
}
token = computeType(next, true).skipType(next);
TypeInfo typeInfo = computeType(next, true);
if (typeInfo.isConditionalExpressionStart(next, this)) {
typeInfo = typeInfo.asNonNullable;
}
token = typeInfo.skipType(next);
next = token.next;
value = next.stringValue;
}
Expand Down Expand Up @@ -5042,6 +5046,10 @@ class Parser {
TypeInfo typeInfo,
bool onlyParseVariableDeclarationStart = false]) {
typeInfo ??= computeType(beforeType, false);
if (typeInfo.isConditionalExpressionStart(beforeType, this)) {
typeInfo = noType;
}

Token token = typeInfo.skipType(beforeType);
Token next = token.next;

Expand Down
45 changes: 36 additions & 9 deletions pkg/front_end/lib/src/fasta/parser/type_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import 'util.dart' show isOneOf, optional;
abstract class TypeInfo {
/// Return type info representing the receiver without the trailing `?`
/// or the receiver if the receiver does not represent a nullable type.
TypeInfo asNonNullableType();
TypeInfo get asNonNullable;

/// Return `true` if the tokens comprising the type represented by the
/// receiver could be interpreted as a valid standalone expression.
/// For example, `A` or `A.b` could be interpreted as a type references
/// or as expressions, while `A<T>` only looks like a type reference.
/// For example, `A` or `A.b` could be interpreted as type references
/// or expressions, while `A<T>` only looks like a type reference.
bool get couldBeExpression;

/// Call this function when the token after [token] must be a type (not void).
Expand All @@ -41,6 +41,13 @@ abstract class TypeInfo {
/// in valid code or during recovery.
Token ensureTypeOrVoid(Token token, Parser parser);

/// Return `true` if the tokens comprising the type represented by the
/// receiver are the start of a conditional expression.
/// For example, `A?` or `A.b?` could be the start of a conditional expression
/// and require arbitrary look ahead to determine if it is,
/// while `A<T>?` cannot be the start of a conditional expression.
bool isConditionalExpressionStart(Token token, Parser parser);

/// Call this function to parse an optional type (not void) after [token].
/// This function will call the appropriate event methods on the [Parser]'s
/// listener to handle the type. This may modify the token stream
Expand Down Expand Up @@ -199,13 +206,19 @@ TypeInfo computeType(final Token token, bool required,
// We've seen identifier `<` identifier `>`
next = typeParamOrArg.skip(next).next;
if (!isGeneralizedFunctionType(next)) {
if (required || looksLikeName(next)) {
// identifier `<` identifier `>` identifier
return typeParamOrArg.typeInfo;
if (optional('?', next) && typeParamOrArg == simpleTypeArgument1) {
if (required || looksLikeName(next.next)) {
// identifier `<` identifier `>` `?` identifier
return simpleNullableTypeWith1Argument;
}
} else {
// identifier `<` identifier `>` non-identifier
return noType;
if (required || looksLikeName(next)) {
// identifier `<` identifier `>` identifier
return typeParamOrArg.typeInfo;
}
}
// identifier `<` identifier `>` non-identifier
return noType;
}
}
// TODO(danrubel): Consider adding a const for
Expand Down Expand Up @@ -255,7 +268,21 @@ TypeInfo computeType(final Token token, bool required,
.computeIdentifierGFT(required);
}

if (required || looksLikeName(next)) {
if (optional('?', next)) {
if (required) {
// identifier `?`
return simpleNullableType;
} else {
next = next.next;
if (isGeneralizedFunctionType(next)) {
// identifier `?` Function `(`
return simpleNullableType;
} else if (looksLikeName(next)) {
// identifier `?` identifier `=`
return simpleNullableType;
}
}
} else if (required || looksLikeName(next)) {
// identifier identifier
return simpleType;
}
Expand Down
Loading

0 comments on commit f0377b2

Please sign in to comment.