Reduce strong mode errors and warnings

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

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library fasta.body_builder;
 
 import 'package:front_end/src/fasta/parser/parser.dart' show
     FormalParameterType,
     optional;
 
@@ skipping 218 matching lines @@
   List<Expression> popListForEffect(int n) {
     List<Expression> list =
         new List<Expression>.filled(n, null, growable: true);
     for (int i = n - 1; i >= 0; i--) {
       list[i] = popForEffect();
     }
     return list;
   }
 
   Block popBlock(int count) {
-    List<Statement> statements = popList(count) ?? <Statement>[];
-    List<Statement> copy;
+    List statements = popList(count) ?? <Statement>[];
+    // TODO(sigmund): should be List<Statement>. In strong mode the .addAll
+    // below shows an error (and will fail at runtime). The core issue is that
+    // `statements` is a List< Statement or List<Statement>>, but we expect that
+    // some of the nested lists will have a reified type of List not
+    // List<Statement>.

[ahe, 2017/02/07 11:03:07]

How about:

List<dynamic /* Statement | List<Statement> */> statements = popList(count) ??
<Statement>[];

And remove the todo.

[Siggi Cherem (dart-lang), 2017/02/07 21:59:25]

Done. added the comment on the type, but had to do something else for the TODO
(below)

+    List copy;

[ahe, 2017/02/07 11:03:07]

This should be List<Statement>.

[Siggi Cherem (dart-lang), 2017/02/07 21:59:25]

Done. Sorry for the confusion - the TODO was referring to this `List`. I
originally removed the explicit type arg as a way to workaround
https://github.com/dart-lang/sdk/issues/28651. (same issue as
`variableOrExpression` below and with formals in the outline_builder.)

There are two ways to work around it here, either removing the type-arg here or
adding a silly assignment further below. I just switched to the latter and made
the comment highlight the issue. Hope this is clearer.

     for (int i = 0; i < statements.length; i++) {
       var statement = statements[i];
       if (statement is List) {
         copy ??= new List<Statement>.from(statements.getRange(0, i));
         copy.addAll(statement);
       } else if (copy != null) {
         copy.add(statement);
       }
     }
     return new Block(copy ?? statements);
@@ skipping 247 matching lines @@
   @override
   void handleParenthesizedExpression(BeginGroupToken token) {
     debugEvent("ParenthesizedExpression");
     push(popForValue());
   }
 
   @override
   void endSend(Token token) {
     debugEvent("Send");
     Arguments arguments = pop();
-    List typeArguments = pop();
+    List<DartType> typeArguments = pop();
     Object receiver = pop();
     if (arguments != null && typeArguments != null) {
       arguments.types.addAll(typeArguments);
     } else {
       assert(typeArguments == null);
     }
     if (receiver is Identifier) {
       Name name = new Name(receiver.name, library.library);
       if (arguments == null) {
         push(new IncompletePropertyAccessor(this, token.charOffset, name));
@@ skipping 467 matching lines @@
     Statement body = popStatement();
     List<Expression> updates = popListForEffect(updateExpressionCount);
     Statement conditionStatement = popStatement();
     Expression condition = null;
     if (conditionStatement is ExpressionStatement) {
       condition = conditionStatement.expression;
     } else {
       assert(conditionStatement is EmptyStatement);
     }
     List<VariableDeclaration> variables = <VariableDeclaration>[];
-    var variableOrExpression = pop();
+    dynamic variableOrExpression = pop();
     Statement begin;
     if (variableOrExpression is BuilderAccessor) {
       variableOrExpression = variableOrExpression.buildForEffect();
     }
     if (variableOrExpression is VariableDeclaration) {
       variables.add(variableOrExpression);
     } else if (variableOrExpression is List) {
-      variables.addAll(variableOrExpression);
+      // TODO(sigmund): remove this assignment (see issue #28651)
+      Iterable vars = variableOrExpression;
+      variables.addAll(vars);
     } else if (variableOrExpression == null) {
       // Do nothing.
     } else if (variableOrExpression is Expression) {
       begin = new ExpressionStatement(variableOrExpression);
     } else {
       return internalError("Unhandled: ${variableOrExpression.runtimeType}");
     }
     exitLocalScope();
     JumpTarget continueTarget = exitContinueTarget();
     JumpTarget breakTarget = exitBreakTarget();
@@ skipping 139 matching lines @@
     }
     // TODO(ahe): Create an error somehow.
     return const DynamicType();
   }
 
   @override
   void endType(Token beginToken, Token endToken) {
     // TODO(ahe): The scope is wrong for return types of generic functions.
     debugEvent("Type");
     List<DartType> arguments = pop();
-    var name = pop();
+    dynamic name = pop();
     if (name is List) {
       if (name.length != 2) {
         return internalError("Unexpected: $name.length");
       }
       var prefix = name[0];
       if (prefix is Identifier) {
         prefix = prefix.name;
       }
       var suffix = name[1];
       if (suffix is Identifier) {
@@ skipping 93 matching lines @@
         return inputError("'this' parameters can only be used on constructors.",
             thisKeyword.charOffset);
       }
     }
     Identifier name = pop();
     DartType type = pop();
     pop(); // Modifiers.
     ignore(Unhandled.Metadata);
     VariableDeclaration variable;
     if (!inCatchClause && functionNestingLevel == 0) {
       var builder = formalParameterScope.lookup(name.name);

[ahe, 2017/02/07 11:03:08]

dynamic.

[Siggi Cherem (dart-lang), 2017/02/07 21:59:25]

Done.

       if (builder == null) {
         return inputError("'${name.name}' isn't a field in this class.",
             name.fileOffset);
       }
       if (thisKeyword == null) {
-        variable = builder.build();
+        KernelFormalParameterBuilder formalBuilder = builder;

[ahe, 2017/02/07 11:03:08]

Remove this.

[Siggi Cherem (dart-lang), 2017/02/07 21:59:25]

Done.

+        variable = formalBuilder.build();
         variable.initializer = name.initializer;
       } else if (builder.isField && builder.parent == classBuilder) {
         FieldBuilder field = builder;
         if (type != null) {
           nit("Ignoring type on 'this' parameter '${name.name}'.",
               name.fileOffset);
         }
         type = field.target.type ?? const DynamicType();
         variable = new VariableDeclaration(name.name, type: type,
             initializer: name.initializer);
@@ skipping 202 matching lines @@
     //
     // This method pops 2 (or 3 if periodBeforeName != null) values from the
     // stack and pushes 3 values: a type, a list of type arguments, and a name.
     //
     // If the constructor reference can be resolved, type is either a
     // ClassBuilder, or a ThisPropertyAccessor. Otherwise, it's an error that
     // should be handled later.
     Identifier suffix = popIfNotNull(periodBeforeName);
     Identifier identifier;
     List<DartType> typeArguments = pop();
-    var type = pop();
+    dynamic type = pop();
     if (type is List) {
       var prefix = type[0];
       identifier = type[1];
       if (prefix is PrefixBuilder) {
         // TODO(ahe): Handle privacy in prefix.exports.
         type = builderToFirstExpression(
             prefix.exports[identifier.name], identifier.name, start.charOffset);
         identifier = null;
       } else if (prefix is ClassBuilder) {
         type = prefix;
@@ skipping 96 matching lines @@
       library.addArgumentsWithMissingDefaultValues(arguments, function);
     }
     return true;
   }
 
   @override
   void handleNewExpression(Token token) {
     debugEvent("NewExpression");
     Arguments arguments = pop();
     String name = pop();
-    List typeArguments = pop();
+    List<DartType> typeArguments = pop();
     var type = pop();
 
     if (typeArguments != null) {
       assert(arguments.types.isEmpty);
       arguments.types.addAll(typeArguments);
     }
 
     String errorName;
-    if (type is ClassBuilder) {
+    if (type is KernelClassBuilder) {

[ahe, 2017/02/07 11:03:07]

Change this back.

[Siggi Cherem (dart-lang), 2017/02/07 21:59:25]

Sorry, I totally meant to ask you a follow up question about this: below there
are two accesses to `type.cls` that are producing an error because `cls` is only
defined in KernelClassBuilder.

On what cases can this be a non-kernel class builder?

[ahe, 2017/02/08 19:59:23]

I don't think it can happen currently, but I'd like to keep the type test and
fix the problems below. I've added some suggestions.

[Siggi Cherem (dart-lang), 2017/02/09 04:44:02]

Done.

       Builder b = type.findConstructorOrFactory(name);
       Member target;
       if (b == null) {
         // Not found. Reported below.
       } else if (b.isConstructor) {
         if (type.cls.isAbstract) {

[ahe, 2017/02/08 19:59:23]

I don't think we have isAbstract on ClassBuilder, could you add that?

In ClassBuilder it would just be abstract and overridden by KernelClass builder.

[Siggi Cherem (dart-lang), 2017/02/09 04:44:02]

Done.

           // TODO(ahe): Generate abstract instantiation error.
         } else {
           target = b.target;
         }
       } else if (b.isFactory) {
         target = getRedirectionTarget(b.target);
         if (target == null) {
           push(buildCompileTimeError(
                   "Cyclic definition of factory '${name}'.",
                   token.charOffset));
           return;
         }
       }
       if (target is Constructor ||
           (target is Procedure && target.kind == ProcedureKind.Factory)) {
         push(buildStaticInvocation(
                 target, arguments, isConst: optional("const", token)));
         return;
       } else {
         errorName = debugName(type.cls.name, name);

[ahe, 2017/02/08 19:59:23]

I think debugName(type.name, name) should work instead.

[Siggi Cherem (dart-lang), 2017/02/09 04:44:02]

Done.

       }
     } else {
       errorName = debugName(getNodeName(type), name);
     }
     errorName ??= name;
     push(throwNoSuchMethodError(errorName, arguments, token.charOffset));
   }
 
   @override
   void handleConstExpression(Token token) {
@@ skipping 66 matching lines @@
 
   @override
   void endFunction(Token getOrSet, Token endToken) {
     debugEvent("Function");
     Statement body = popStatement();
     AsyncMarker asyncModifier = pop();
     if (functionNestingLevel != 0) {
       exitLocalScope();
     }
     FormalParameters formals = pop();
-    List typeParameters = pop();
+    List<TypeParameter> typeParameters = pop();
     push(formals.addToFunction(new FunctionNode(body,
                 typeParameters: typeParameters, asyncMarker: asyncModifier)));
     functionNestingLevel--;
   }
 
   @override
   void endFunctionDeclaration(Token token) {
     debugEvent("FunctionDeclaration");
     FunctionNode function = pop();
     exitLocalScope();
     FunctionDeclaration declaration = pop();
     function.returnType = pop() ?? const DynamicType();
     pop(); // Modifiers.
     declaration.function = function;
     function.parent = declaration;
     push(declaration);
   }
 
   @override
   void endUnnamedFunction(Token token) {
     debugEvent("UnnamedFunction");
     Statement body = popStatement();
     AsyncMarker asyncModifier = pop();
     exitLocalScope();
     FormalParameters formals = pop();
-    List typeParameters = pop();
+    List<TypeParameter> typeParameters = pop();
     FunctionNode function = formals.addToFunction(new FunctionNode(body,
             typeParameters: typeParameters, asyncMarker: asyncModifier));
     push(new FunctionExpression(function));
     functionNestingLevel--;
   }
 
   @override
   void endDoWhileStatement(
       Token doKeyword, Token whileKeyword, Token endToken) {
     debugEvent("DoWhileStatement");
@@ skipping 891 matching lines @@
   } else if (node is TypeDeclarationBuilder) {
     return node.name;
   } else if (node is PrefixBuilder) {
     return node.name;
   } else if (node is ThisPropertyAccessor) {
     return node.name.name;
   } else {
     return internalError("Unhandled: ${node.runtimeType}");
   }
 }