Skip to content

Commit 4bb9fdc

Browse files
vsmenonvsmenon
committed
Fixes #151
Type check async-return, async*-yield, and sync*-yield. R=leafp@google.com Review URL: https://codereview.chromium.org/1190413005.
1 parent 9b1ad9c commit 4bb9fdc

File tree

3 files changed

+190
-115
lines changed

3 files changed

+190
-115
lines changed

‎pkg/dev_compiler/lib/src/checker/checker.dart

+129-111
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,6 @@ class CodeChecker extends RecursiveAstVisitor {
337337
final TypeRules _rules;
338338
final CheckerReporter _reporter;
339339
final _OverrideChecker _overrideChecker;
340-
bool _constantContext = false;
341340
bool _failure = false;
342341
bool get failure => _failure || _overrideChecker._failure;
343342

@@ -358,27 +357,6 @@ class CodeChecker extends RecursiveAstVisitor {
358357
_rules.reportMissingType = callback;
359358
}
360359

361-
_visitMaybeConst(AstNode n, visitNode(AstNode n)) {
362-
var o = _constantContext;
363-
if (!o) {
364-
if (n is VariableDeclarationList) {
365-
_constantContext = o || n.isConst;
366-
} else if (n is VariableDeclaration) {
367-
_constantContext = o || n.isConst;
368-
} else if (n is DefaultFormalParameter) {
369-
_constantContext = true;
370-
} else if (n is FormalParameter) {
371-
_constantContext = o || n.isConst;
372-
} else if (n is InstanceCreationExpression) {
373-
_constantContext = o || n.isConst;
374-
} else if (n is ConstructorDeclaration) {
375-
_constantContext = o || n.element.isConst;
376-
}
377-
}
378-
visitNode(n);
379-
_constantContext = o;
380-
}
381-
382360
@override
383361
void visitComment(Comment node) {
384362
// skip, no need to do typechecking inside comments (they may contain
@@ -406,17 +384,15 @@ class CodeChecker extends RecursiveAstVisitor {
406384
/// Check constructor declaration to ensure correct super call placement.
407385
@override
408386
void visitConstructorDeclaration(ConstructorDeclaration node) {
409-
_visitMaybeConst(node, (node) {
410-
node.visitChildren(this);
411-
412-
final init = node.initializers;
413-
for (int i = 0, last = init.length - 1; i < last; i++) {
414-
final node = init[i];
415-
if (node is SuperConstructorInvocation) {
416-
_recordMessage(new InvalidSuperInvocation(node));
417-
}
387+
node.visitChildren(this);
388+
389+
final init = node.initializers;
390+
for (int i = 0, last = init.length - 1; i < last; i++) {
391+
final node = init[i];
392+
if (node is SuperConstructorInvocation) {
393+
_recordMessage(new InvalidSuperInvocation(node));
418394
}
419-
});
395+
}
420396
}
421397

422398
@override
@@ -603,40 +579,94 @@ class CodeChecker extends RecursiveAstVisitor {
603579
node.visitChildren(this);
604580
}
605581

606-
AstNode _getOwnerFunction(AstNode node) {
607-
var parent = node.parent;
608-
while (parent is! FunctionExpression &&
609-
parent is! MethodDeclaration &&
610-
parent is! ConstructorDeclaration) {
611-
parent = parent.parent;
582+
DartType _getExpectedReturnType(FunctionBody body, bool yieldStar) {
583+
FunctionType functionType;
584+
var parent = body.parent;
585+
if (parent is Declaration) {
586+
functionType = _rules.elementType(parent.element);
587+
} else {
588+
assert(parent is FunctionExpression);
589+
functionType = _rules.getStaticType(parent);
612590
}
613-
return parent;
614-
}
615591

616-
FunctionType _getFunctionType(AstNode node) {
617-
if (node is Declaration) {
618-
return _rules.elementType(node.element);
592+
var type = functionType.returnType;
593+
var provider = _rules.provider;
594+
595+
InterfaceType expectedType = null;
596+
if (body.isAsynchronous) {
597+
if (body.isGenerator) {
598+
// Stream<T> -> T
599+
expectedType = provider.streamType;
600+
} else {
601+
// Future<T> -> T
602+
// TODO(vsm): Revisit with issue #228.
603+
expectedType = provider.futureType;
604+
}
619605
} else {
620-
assert(node is FunctionExpression);
621-
return _rules.getStaticType(node);
606+
if (body.isGenerator) {
607+
// Iterable<T> -> T
608+
expectedType = provider.iterableType;
609+
} else {
610+
// T -> T
611+
return type;
612+
}
613+
}
614+
if (yieldStar) {
615+
if (type.isDynamic) {
616+
// Ensure it's at least a Stream / Iterable.
617+
return expectedType.substitute4([provider.dynamicType]);
618+
} else {
619+
// Analyzer will provide a separate error if expected type
620+
// is not compatible with type.
621+
return type;
622+
}
623+
}
624+
if (type.isDynamic) {
625+
return type;
626+
} else if (type is InterfaceType && type.element == expectedType.element) {
627+
return type.typeArguments[0];
628+
} else {
629+
// Malformed type - fallback on analyzer error.
630+
return null;
631+
}
632+
}
633+
634+
FunctionBody _getFunctionBody(AstNode node) {
635+
while (node is! FunctionBody) {
636+
node = node.parent;
622637
}
638+
return node as FunctionBody;
623639
}
624640

625-
void _checkReturn(Expression expression, AstNode node) {
626-
var type = _getFunctionType(_getOwnerFunction(node)).returnType;
641+
void _checkReturnOrYield(Expression expression, AstNode node,
642+
[bool yieldStar = false]) {
643+
var body = _getFunctionBody(node);
644+
var type = _getExpectedReturnType(body, yieldStar);
645+
if (type == null) {
646+
// We have a type mismatch: the async/async*/sync* modifier does
647+
// not match the return or yield type. We should have already gotten an
648+
// analyzer error in this case.
649+
return;
650+
}
627651
// TODO(vsm): Enforce void or dynamic (to void?) when expression is null.
628652
if (expression != null) checkAssignment(expression, type);
629653
}
630654

631655
@override
632656
void visitExpressionFunctionBody(ExpressionFunctionBody node) {
633-
_checkReturn(node.expression, node);
657+
_checkReturnOrYield(node.expression, node);
634658
node.visitChildren(this);
635659
}
636660

637661
@override
638662
void visitReturnStatement(ReturnStatement node) {
639-
_checkReturn(node.expression, node);
663+
_checkReturnOrYield(node.expression, node);
664+
node.visitChildren(this);
665+
}
666+
667+
@override
668+
void visitYieldStatement(YieldStatement node) {
669+
_checkReturnOrYield(node.expression, node, node.star != null);
640670
node.visitChildren(this);
641671
}
642672

@@ -660,24 +690,22 @@ class CodeChecker extends RecursiveAstVisitor {
660690

661691
@override
662692
void visitDefaultFormalParameter(DefaultFormalParameter node) {
663-
_visitMaybeConst(node, (node) {
664-
// Check that defaults have the proper subtype.
665-
var parameter = node.parameter;
666-
var parameterType = _rules.elementType(parameter.element);
667-
assert(parameterType != null);
668-
var defaultValue = node.defaultValue;
669-
if (defaultValue == null) {
670-
if (_rules.maybeNonNullableType(parameterType)) {
671-
var staticInfo = new InvalidVariableDeclaration(
672-
_rules, node.identifier, parameterType);
673-
_recordMessage(staticInfo);
674-
}
675-
} else {
676-
checkAssignment(defaultValue, parameterType);
693+
// Check that defaults have the proper subtype.
694+
var parameter = node.parameter;
695+
var parameterType = _rules.elementType(parameter.element);
696+
assert(parameterType != null);
697+
var defaultValue = node.defaultValue;
698+
if (defaultValue == null) {
699+
if (_rules.maybeNonNullableType(parameterType)) {
700+
var staticInfo = new InvalidVariableDeclaration(
701+
_rules, node.identifier, parameterType);
702+
_recordMessage(staticInfo);
677703
}
704+
} else {
705+
checkAssignment(defaultValue, parameterType);
706+
}
678707

679-
node.visitChildren(this);
680-
});
708+
node.visitChildren(this);
681709
}
682710

683711
@override
@@ -700,56 +728,47 @@ class CodeChecker extends RecursiveAstVisitor {
700728

701729
@override
702730
void visitInstanceCreationExpression(InstanceCreationExpression node) {
703-
_visitMaybeConst(node, (node) {
704-
var arguments = node.argumentList;
705-
var element = node.staticElement;
706-
if (element != null) {
707-
var type = _rules.elementType(node.staticElement);
708-
checkArgumentList(arguments, type);
709-
} else {
710-
_recordMessage(new MissingTypeError(node));
711-
}
712-
node.visitChildren(this);
713-
});
731+
var arguments = node.argumentList;
732+
var element = node.staticElement;
733+
if (element != null) {
734+
var type = _rules.elementType(node.staticElement);
735+
checkArgumentList(arguments, type);
736+
} else {
737+
_recordMessage(new MissingTypeError(node));
738+
}
739+
node.visitChildren(this);
714740
}
715741

716742
@override
717743
void visitVariableDeclarationList(VariableDeclarationList node) {
718-
_visitMaybeConst(node, (node) {
719-
TypeName type = node.type;
720-
if (type == null) {
721-
// No checks are needed when the type is var. Although internally the
722-
// typing rules may have inferred a more precise type for the variable
723-
// based on the initializer.
724-
} else {
725-
var dartType = getType(type);
726-
for (VariableDeclaration variable in node.variables) {
727-
var initializer = variable.initializer;
728-
if (initializer != null) {
729-
checkAssignment(initializer, dartType);
730-
} else if (_rules.maybeNonNullableType(dartType)) {
731-
var element = variable.element;
732-
if (element is FieldElement && !element.isStatic) {
733-
// Initialized - possibly implicitly - during construction.
734-
// Handle this via a runtime check during code generation.
735-
736-
// TODO(vsm): Detect statically whether this can fail and
737-
// report a static error (must fail) or warning (can fail).
738-
} else {
739-
var staticInfo =
740-
new InvalidVariableDeclaration(_rules, variable, dartType);
741-
_recordMessage(staticInfo);
742-
}
744+
TypeName type = node.type;
745+
if (type == null) {
746+
// No checks are needed when the type is var. Although internally the
747+
// typing rules may have inferred a more precise type for the variable
748+
// based on the initializer.
749+
} else {
750+
var dartType = getType(type);
751+
for (VariableDeclaration variable in node.variables) {
752+
var initializer = variable.initializer;
753+
if (initializer != null) {
754+
checkAssignment(initializer, dartType);
755+
} else if (_rules.maybeNonNullableType(dartType)) {
756+
var element = variable.element;
757+
if (element is FieldElement && !element.isStatic) {
758+
// Initialized - possibly implicitly - during construction.
759+
// Handle this via a runtime check during code generation.
760+
761+
// TODO(vsm): Detect statically whether this can fail and
762+
// report a static error (must fail) or warning (can fail).
763+
} else {
764+
var staticInfo =
765+
new InvalidVariableDeclaration(_rules, variable, dartType);
766+
_recordMessage(staticInfo);
743767
}
744768
}
745769
}
746-
node.visitChildren(this);
747-
});
748-
}
749-
750-
@override
751-
void visitVariableDeclaration(VariableDeclaration node) {
752-
_visitMaybeConst(node, super.visitVariableDeclaration);
770+
}
771+
node.visitChildren(this);
753772
}
754773

755774
void _checkRuntimeTypeCheck(AstNode node, TypeName typeName) {
@@ -885,7 +904,7 @@ class CodeChecker extends RecursiveAstVisitor {
885904
if (expr is ParenthesizedExpression) {
886905
checkAssignment(expr.expression, type);
887906
} else {
888-
_recordMessage(_rules.checkAssignment(expr, type, _constantContext));
907+
_recordMessage(_rules.checkAssignment(expr, type));
889908
}
890909
}
891910

@@ -956,8 +975,7 @@ class CodeChecker extends RecursiveAstVisitor {
956975
// Check the rhs type
957976
if (staticInfo is! CoercionInfo) {
958977
var paramType = paramTypes.first;
959-
staticInfo = _rules.checkAssignment(
960-
expr.rightHandSide, paramType, _constantContext);
978+
staticInfo = _rules.checkAssignment(expr.rightHandSide, paramType);
961979
_recordMessage(staticInfo);
962980
}
963981
}

‎pkg/dev_compiler/lib/src/checker/rules.dart

+3-4
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ abstract class TypeRules {
4343
bool isNonNullableType(DartType t) => false;
4444
bool maybeNonNullableType(DartType t) => false;
4545

46-
StaticInfo checkAssignment(Expression expr, DartType t, bool constContext);
46+
StaticInfo checkAssignment(Expression expr, DartType t);
4747

4848
DartType getStaticType(Expression expr) => expr.staticType;
4949

@@ -93,8 +93,7 @@ class DartRules extends TypeRules {
9393
return t1.isAssignableTo(t2);
9494
}
9595

96-
StaticInfo checkAssignment(
97-
Expression expr, DartType toType, bool constContext) {
96+
StaticInfo checkAssignment(Expression expr, DartType toType) {
9897
final fromType = getStaticType(expr);
9998
if (!isAssignable(fromType, toType)) {
10099
return new StaticTypeError(this, expr, toType);
@@ -444,7 +443,7 @@ class RestrictedRules extends TypeRules {
444443
return Coercion.error();
445444
}
446445

447-
StaticInfo checkAssignment(Expression expr, DartType toT, bool constContext) {
446+
StaticInfo checkAssignment(Expression expr, DartType toT) {
448447
final fromT = getStaticType(expr);
449448
final Coercion c = _coerceTo(fromT, toT);
450449
if (c is Identity) return null;

0 commit comments

Comments
 (0)