Check void usage and invalid returns

Second take on 1100179,
but we treat all the problems as warnings.

This reverts commit 5c9c90f.

Change-Id: Id9fdeabf5d49fefb48cfe4a0ca1b6da2be7dff59
Reviewed-on: https://dart-review.googlesource.com/66160
Reviewed-by: Dmitry Stefantsov <[email protected]>
Commit-Queue: Peter von der Ahé <[email protected]>

Author: peter-ahe-google

pkg/front_end/lib/src/fasta/fasta_codes_generated.dart

@@ -5967,6 +5967,17 @@ const MessageCode messageRequiredParameterWithDefault = const MessageCode(
     tip:
         r"""Try removing the default value or making the parameter optional.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeReturnFromVoidFunction = messageReturnFromVoidFunction;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageReturnFromVoidFunction = const MessageCode(
+    "ReturnFromVoidFunction",
+    analyzerCode: "RETURN_OF_INVALID_TYPE",
+    dart2jsCode: "*fatal*",
+    severity: Severity.warning,
+    message: r"""Can't return a value from a void function.""");
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeReturnTypeFunctionExpression =
     messageReturnTypeFunctionExpression;
@@ -7068,6 +7079,16 @@ const MessageCode messageVarReturnType = const MessageCode("VarReturnType",
     tip:
         r"""Try removing the keyword 'var', or replacing it with the name of the return type.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeVoidExpression = messageVoidExpression;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageVoidExpression = const MessageCode("VoidExpression",
+    analyzerCode: "USE_OF_VOID_RESULT",
+    dart2jsCode: "*fatal*",
+    severity: Severity.warning,
+    message: r"""This expression has type 'void' and can't be used.""");
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<
     Message Function(

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

@@ -56,6 +56,8 @@ import '../scanner/token.dart' show isBinaryOperator, isMinusOperator;
 
 import '../scope.dart' show ProblemBuilder;
 
+import '../severity.dart' show Severity;
+
 import '../source/outline_builder.dart' show OutlineBuilder;
 
 import '../source/scope_listener.dart'
@@ -3968,6 +3970,28 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
         context: context);
   }
 
+  @override
+  Expression wrapInProblem(Expression expression, Message message, int length,
+      {List<LocatedMessage> context}) {
+    int charOffset = expression.fileOffset;
+    Severity severity = message.code.severity;
+    if (severity == null) {
+      addCompileTimeError(message, charOffset, length, context: context);
+      internalProblem(
+          fasta.templateInternalProblemMissingSeverity
+              .withArguments(message.code.name),
+          charOffset,
+          uri);
+    } else if (severity == Severity.error ||
+        severity == Severity.errorLegacyWarning &&
+            library.loader.target.strongMode) {
+      return wrapInCompileTimeError(expression, message, context: context);
+    } else {
+      addProblem(message, charOffset, length, context: context);
+    }
+    return expression;
+  }
+
   @override
   Expression wrapInLocatedCompileTimeError(
       Expression expression, LocatedMessage message,

pkg/front_end/lib/src/fasta/kernel/expression_generator_helper.dart

@@ -77,6 +77,9 @@ abstract class ExpressionGeneratorHelper implements InferenceHelper {
 
   Expression wrapInCompileTimeError(Expression expression, Message message);
 
+  Expression wrapInProblem(Expression expression, Message message, int length,
+      {List<LocatedMessage> context});
+
   Expression deprecated_buildCompileTimeError(String error,
       [int offset, Message message]);
 

pkg/front_end/lib/src/fasta/kernel/kernel_expression_generator.dart

@@ -794,8 +794,11 @@ class KernelIndexedAccessGenerator extends KernelGenerator
   @override
   Expression _finish(
       Expression body, ComplexAssignmentJudgment complexAssignment) {
+    int offset = offsetForToken(token);
     return super._finish(
-        makeLet(receiverVariable, makeLet(indexVariable, body)),
+        makeLet(
+            receiverVariable, makeLet(indexVariable, body)..fileOffset = offset)
+          ..fileOffset = offset,
         complexAssignment);
   }
 
@@ -1055,7 +1058,9 @@ class KernelSuperIndexedAccessGenerator extends KernelGenerator
   @override
   Expression _finish(
       Expression body, ComplexAssignmentJudgment complexAssignment) {
-    return super._finish(makeLet(indexVariable, body), complexAssignment);
+    return super._finish(
+        makeLet(indexVariable, body)..fileOffset = offsetForToken(token),
+        complexAssignment);
   }
 
   @override

pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart

@@ -39,6 +39,7 @@ import '../../base/instrumentation.dart'
 
 import '../fasta_codes.dart'
     show
+        messageVoidExpression,
         noLength,
         templateCantInferTypeDueToCircularity,
         templateCantUseSuperBoundedTypeForInstanceCreation,
@@ -439,7 +440,8 @@ class CascadeJudgment extends Let implements ExpressionJudgment {
       variable.type = inferredType;
     }
     for (var judgment in cascadeJudgments) {
-      inferrer.inferExpression(factory, judgment, const UnknownType(), false);
+      inferrer.inferExpression(factory, judgment, const UnknownType(), false,
+          isVoidAllowed: true);
     }
     inferrer.listener.cascadeExpression(this, fileOffset, inferredType);
     return null;
@@ -1176,7 +1178,8 @@ class ForInJudgment extends ForInStatement implements StatementJudgment {
             this.variable.type,
             syntheticAssignment.rhs,
             syntheticAssignment.rhs.fileOffset,
-            template: templateForInLoopElementTypeNotAssignable);
+            template: templateForInLoopElementTypeNotAssignable,
+            isVoidAllowed: true);
         if (syntheticAssignment is PropertyAssignmentJudgment) {
           syntheticAssignment._handleWriteContravariance(
               inferrer, inferrer.thisType);
@@ -1510,6 +1513,10 @@ class IfNullJudgment extends Let implements ExpressionJudgment {
       inferrer.inferExpression(factory, rightJudgment, typeContext, _forceLub);
     }
     var rhsType = rightJudgment.inferredType;
+    if (rhsType is VoidType) {
+      inferrer.helper?.addProblem(
+          messageVoidExpression, rightJudgment.fileOffset, noLength);
+    }
     // - Let T = greatest closure of K with respect to `?` if K is not `_`, else
     //   UP(t0, t1)
     // - Then the inferred type is T.
@@ -2514,7 +2521,7 @@ class ReturnJudgment extends ReturnStatement implements StatementJudgment {
       Factory<Expression, Statement, Initializer, Type> factory) {
     var judgment = this.judgment;
     var closureContext = inferrer.closureContext;
-    var typeContext = !closureContext.isGenerator
+    DartType typeContext = !closureContext.isGenerator
         ? closureContext.returnOrYieldContext
         : const UnknownType();
     DartType inferredType;
@@ -2528,8 +2535,8 @@ class ReturnJudgment extends ReturnStatement implements StatementJudgment {
     // inferred type of the closure.  TODO(paulberry): is this what we want
     // for Fasta?
     if (judgment != null) {
-      closureContext.handleReturn(
-          inferrer, inferredType, expression, fileOffset);
+      closureContext.handleReturn(inferrer, inferredType, expression,
+          fileOffset, !identical(returnKeyword?.lexeme, "return"));
     }
     inferrer.listener
         .returnStatement(this, fileOffset, returnKeyword, null, semicolon);
@@ -3313,7 +3320,8 @@ class ShadowTypeInferrer extends TypeInferrerImpl {
       Factory<Expression, Statement, Initializer, Type> factory,
       kernel.Expression expression,
       DartType typeContext,
-      bool typeNeeded) {
+      bool typeNeeded,
+      {bool isVoidAllowed: false}) {
     // `null` should never be used as the type context.  An instance of
     // `UnknownType` should be used instead.
     assert(typeContext != null);
@@ -3339,7 +3347,34 @@ class ShadowTypeInferrer extends TypeInferrerImpl {
       // so that the type hierarchy will be simpler (which may speed up "is"
       // checks).
       expression.infer(this, factory, typeContext);
-      return expression.inferredType;
+      DartType inferredType = expression.inferredType;
+      if (inferredType is VoidType && !isVoidAllowed) {
+        TreeNode parent = expression.parent;
+        if (parent is ReturnStatement ||
+            parent is ExpressionStatement ||
+            parent is AsExpression) {
+          return inferredType;
+        } else if (parent is ForStatement &&
+            parent.updates.contains(expression)) {
+          return inferredType;
+        } else if (parent is VariableDeclaration) {
+          TreeNode grandParent = parent.parent;
+          if (grandParent is ForStatement &&
+              parent.name == null &&
+              grandParent.variables.contains(parent)) {
+            return inferredType;
+          }
+        } else if (parent is ConditionalExpression) {
+          if (parent.then == expression || parent.otherwise == expression) {
+            return inferredType;
+          }
+        } else if (parent is DeferredCheckJudgment) {
+          return inferredType;
+        }
+        helper?.addProblem(
+            messageVoidExpression, expression.fileOffset, noLength);
+      }
+      return inferredType;
     } else {
       // Encountered an expression type for which type inference is not yet
       // implemented, so just infer dynamic for now.

pkg/front_end/lib/src/fasta/type_inference/inference_helper.dart

@@ -16,4 +16,7 @@ abstract class InferenceHelper {
       FunctionType function, Arguments arguments, int offset);
 
   void addProblem(Message message, int charOffset, int length);
+
+  Expression wrapInProblem(Expression expression, Message message, int length,
+      {List<LocatedMessage> context});
 }