Arglist `children:` default value gen fixes.

pkg/analysis_server/lib/src/services/completion/dart/utilities.dart

 // Copyright (c) 2017, 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.
 
 /**
  * A collection of utility methods used by completion contributors.
  */
 import 'package:analysis_server/plugin/protocol/protocol.dart' as protocol
     show Element, ElementKind;
 import 'package:analysis_server/src/ide_options.dart';
@@ skipping 53 matching lines @@
       }
       String name = param.name;
       sb.write('$name: ');
       offset = sb.length;
       String defaultValue = _getDefaultValue(param);
       sb.write(defaultValue);
       ranges.addAll([offset, defaultValue.length]);
     }
   }
 
-  //TODO(pq): generalize and unify with _getDefaultValue
   if (options?.generateFlutterWidgetChildrenBoilerPlate == true) {
     if (element is ConstructorElement) {
-      ConstructorElement constructorElement = element;
-      ClassElement classElement = constructorElement.enclosingElement;
+      ClassElement classElement = element.enclosingElement;

[danrubel, 2017/03/24 13:16:19]

It appears that "element" is not modified in this method, thus you can use
"element" as a ClassElement within this "if" statement and remove
"classElement".

[pquitslund, 2017/03/24 15:56:23]

Done.

       if (isFlutterWidget(classElement)) {
-        for (ParameterElement param in constructorElement.parameters) {
+        for (ParameterElement param in element.parameters) {
           if (param.name == 'children') {
-            DartType type = param.type;
-            if (type is InterfaceType && isDartList(type)) {
-              InterfaceType interfaceType = type;
-              List<DartType> typeArguments = interfaceType.typeArguments;
-              if (typeArguments.length == 1) {
-                if (sb.isNotEmpty) {
-                  sb.write(', ');
-                }
-                offset = sb.length;
-                sb.write('children: <${typeArguments.first.name}>[]');
-              }
+            String defaultValue = getDefaultStringParameterValue(param);
+            if (sb.isNotEmpty) {
+              sb.write(', ');
             }
+            sb.write('children: ');
+            offset = sb.length;
+            sb.write(defaultValue);

[Brian Wilkerson, 2017/03/23 23:34:48]

'defaultValue' can be null here.

[pquitslund, 2017/03/24 15:56:23]

Right.  I was thinking this was OK since the `toString()` of null is "null"
(which is I *think* what we want).

I'll add a test though.

[Brian Wilkerson, 2017/03/24 16:04:32]

Given a widget class with a constructor of the form:

  Widget2({Map<String, Widget> children});

If the user requests code completion here:

  Widget2(^)

the suggestion will be "children: null". If we can't do better than this, do we
really want this, or should we call back to the behavior used everywhere else of
just suggesting "children: "? (I'd vote for the latter, but you might want a
second opinion :-)

+            ranges.addAll([offset, defaultValue.length]);
           }
         }
       }
     }
   }
 
   suggestion.defaultArgumentListString = sb.isNotEmpty ? sb.toString() : null;
   suggestion.defaultArgumentListTextRanges = ranges.isNotEmpty ? ranges : null;
 }
 
@@ skipping 73 matching lines @@
     if (classId != null) {
       String className = classId.name;
       if (className != null && className.length > 0) {
         suggestion.declaringType = className;
       }
     }
   }
   return suggestion;
 }
 
+String getDefaultStringParameterValue(ParameterElement param) {

[Brian Wilkerson, 2017/03/23 23:34:47]

Consider passing in a StringSink here rather than returning a value that has to
be null-checked (and changing the name of the method as appropriate).

[pquitslund, 2017/03/24 15:56:23]

Interesting idea.  As mentioned above, I don't think the null check is necessary
unless I'm confused.  In any event some tests are a good idea and I have a
parallel change that addresses test fixes in general and will fill this out a
bit more.

To be continued... :)

+  DartType type = param.type;
+  if (type is InterfaceType && isDartList(type)) {
+    List<DartType> typeArguments = type.typeArguments;
+    StringBuffer sb = new StringBuffer();
+    if (typeArguments.length == 1) {
+      DartType typeArg = typeArguments.first;
+      if (!typeArg.isDynamic) {
+        sb.write('<${typeArg.name}>');
+      }
+      sb.write('[]');
+      return sb.toString();
+    }
+  }
+  return null;
+}
+
 bool isDartList(DartType type) {
   ClassElement element = type.element;
   if (element != null) {
     return element.name == "List" && element.library.isDartCore;
   }
   return false;
 }
 
 /**
  * Return `true` if the @deprecated annotation is present on the given [node].
@@ skipping 34 matching lines @@
       //TODO (danrubel) include type arguments
     }
     return name;
   } else if (type is GenericFunctionType) {
     // TODO(brianwilkerson) Implement this.
   }
   return DYNAMIC;
 }
 
 String _getDefaultValue(ParameterElement param) => 'null';