Implement newInstance in the transformer.

reflectable/lib/src/transformer_implementation.dart

Index: reflectable/lib/src/transformer_implementation.dart
diff --git a/reflectable/lib/src/transformer_implementation.dart b/reflectable/lib/src/transformer_implementation.dart
index d7709061f90ca8f8b9a6f98ec3f5362c2ccea25b..7d64e2427fb004be10f11eb18f0febe44d3a208c 100644
--- a/reflectable/lib/src/transformer_implementation.dart
+++ b/reflectable/lib/src/transformer_implementation.dart
@@ -82,14 +82,14 @@ class ClassDomain {
   ReflectorDomain reflectorDomain;
 
   Iterable<MethodElement> get invokableMethods => instanceMembers
-    .where((ExecutableElement element) => element is MethodElement);
+      .where((ExecutableElement element) => element is MethodElement);
 
   String staticClassMirrorName;
   String staticInstanceMirrorName;
   String get baseName => classElement.name;
 
-  ClassDomain(this.classElement, this.declaredMethods,
-      this.declaredAccessors, this.constructors, this.reflectorDomain);
+  ClassDomain(this.classElement, this.declaredMethods, this.declaredAccessors,
+      this.constructors, this.reflectorDomain);
 
   Iterable<ExecutableElement> get declarations {
     // TODO(sigurdm): Include fields.
@@ -99,7 +99,6 @@ class ClassDomain {
 
   /// Finds all instance members by going through the class hierarchy.
   Iterable<ExecutableElement> get instanceMembers {
-
     Map<String, ExecutableElement> helper(ClassElement classElement) {
       if (reflectorDomain._instanceMemberCache[classElement] != null) {
         return reflectorDomain._instanceMemberCache[classElement];
@@ -203,14 +202,86 @@ class ClassDomain {
   String get instanceMembersString {
     // TODO(sigurdm): Find out how to set the right owner.
     Iterable<String> instanceMemberParts = instanceMembers.map(
-            (ExecutableElement declaration) {
-          return '"${nameOfDeclaration(declaration)}": '
+        (ExecutableElement declaration) {
+      return '"${nameOfDeclaration(declaration)}": '
           'new MethodMirrorImpl("${declaration.name}", '
           '${_declarationDescriptor(declaration)}, null)';
-        });
+    });
     return "{${instanceMemberParts.join(", ")}}";
   }
 
+  String get newInstanceString {

[eernst, 2015/06/18 13:35:41]

I've been looking for a general hint (some DartDoc comment, presumably) about
the general idea behind xString for various x. Is it a factorization that
includes both (1) obtaining various data from the Element model about the shape
of the target program, and also (2) producing a string that represents the same
semantics in generated code? I guess these two elements were often inlined in
code for larger tasks, so the factorization looks fine in itself.

On the other hand, it does look somewhat "impure" that the data extraction and
the string construction are packaged together. Would it be a meaningless further
factorization to express them separately? (I realize that it may well be a bit
inconvenient, because the two are quite intertwined.)

Also, it is tempting to think that some of the string construction tasks can be
obtained by using some 'prettyprint' feature from the Element world, if there is
such a thing. (Copying an excerpt from the original source would surely not be a
safe bet, because that might be formatted in ways that make no sense when moved
into an entirely different context).

Is there a natural place where we can add a few lines of text explaining the
situation?

[sigurdm, 2015/06/19 07:50:54]

Your intuition is right. There is no exact defined semantics of the -String
methods, but something along those lines.
Because I am going to refactor how we represent the data - I will leave this
uncommented for now, and try to document better how the new approach works.

+    // TODO(eernst, sigurdm, #8): Recreate the default values faithfully.
+    List<String> constructorLines = new List<String>();
+
+    String name(int i) {
+      String alphabet = "abcdefghijklmnopqrstuvwxyz";
+      int alphabetLength = alphabet.length;
+      if (i < alphabetLength) {
+        return "_${alphabet[i]}";
+      }
+      return "_${alphabet[i % alphabetLength]}${i ~/ alphabetLength}";
+    }
+    for (ConstructorElement constructor in constructors) {
+      FunctionType type = constructor.type;
+      int positionalCount = type.normalParameterTypes.length;
+      int optionalCount = type.optionalParameterTypes.length;
+      List<String> argumentNames = type.namedParameterTypes.keys.toList();
+      String positionals = new Iterable.generate(
+          positionalCount, (int i) => name(i)).join(", ");
+      String optionalsWithDefaults = new Iterable.generate(optionalCount,
+          (int i) {
+        String defaultValueCode =
+            constructor.parameters[positionalCount + i].defaultValueCode;
+        String defaultValueString =
+            defaultValueCode == null ? "" : " = $defaultValueCode";
+        return "${name(i + positionalCount)}$defaultValueString";
+      }).join(", ");

[eernst, 2015/06/18 13:35:41]

    Maybe
  we can sell this style of
  indentation to the rest of the
world

rather
  than this boring
one?

[sigurdm, 2015/06/19 07:50:54]

?

+      String namedWithDefaults = new Iterable.generate(argumentNames.length,
+          (int i) {
+        String defaultValueCode =
+            constructor.parameters[positionalCount + i].defaultValueCode;
+        String defaultValueString =
+            defaultValueCode == null ? "" : ": $defaultValueCode";
+        return "${argumentNames[i]}$defaultValueString";
+      }).join(", ");
+
+      String optionalArguments = new Iterable.generate(
+          optionalCount, (int i) => name(i + positionalCount)).join(", ");
+      String namedArguments =
+          argumentNames.map((String name) => "$name: $name").join(", ");
+      List<String> parameterParts = new List<String>();
+      List<String> argumentParts = new List<String>();
+      if (positionalCount != 0) {
+        parameterParts.add(positionals);
+        argumentParts.add(positionals);
+      }
+      if (optionalCount != 0) {
+        parameterParts.add("[$optionalsWithDefaults]");
+        argumentParts.add(optionalArguments);
+      }
+      if (argumentNames.isNotEmpty) {
+        parameterParts.add("{${namedWithDefaults}}");
+        argumentParts.add(namedArguments);
+      }
+      constructorLines.add('case "${constructor.name}": '
+          'c = (${parameterParts.join(', ')}) => '
+          'new ${nameOfDeclaration(constructor)}'
+          '(${argumentParts.join(", ")});');
+      constructorLines.add('  break;');
+    }
+    return """newInstance(String constructorName, List positionalArguments,
+      [Map<Symbol, dynamic> namedArguments]) {
+    Function c;
+    switch (constructorName) {
+      ${constructorLines.join("\n      ")}
+      default: throw new NoSuchInvokeCapabilityError(${classElement.name},
+          constructorName, positionalArguments, namedArguments);
+    }
+    return Function.apply(c, positionalArguments, namedArguments);
+  }""";
+  }
+
   void computeNames(Namer namer) {
     staticClassMirrorName = namer.freshName("Static_${baseName}_ClassMirror");
     staticInstanceMirrorName =
@@ -855,8 +926,13 @@ class TransformerImplementation {
   /// [ClassMirror] which is specialized for a `reflectedType` which
   /// is the class modeled by [classElement].
   String _staticClassMirrorCode(ClassDomain classDomain) {
+
     String declarationsString = classDomain.declarationsString;
     String instanceMembersString = classDomain.instanceMembersString;
+    List<String> memberStrings = new List<String>();
+    if (!classDomain.classElement.isAbstract) {
+      memberStrings.add(classDomain.newInstanceString);
+    }

[eernst, 2015/06/18 13:35:41]

This one would be new, but still related to the treatment of `abstract`
features.

But it's not obvious to me that `isAbstract` justifies omission.


An element.dart `ClassElement` represents a class C, so `newInstanceString`
would describe constructors producing instances of C, and this would make them
part of the "members strings" of the static mirror class under construction.

We should be able to obtain a class mirror for an abstract class, because users
can use reflectType to directly request such a mirror.

It is possible to invoke a constructor for an abstract class, but it's a runtime
error, Spec. 1.9, Sect. 16.12.1, so presumably it's OK to leave it as
'NoSuchCapability' rather than reproducing the original runtime error (didn't
check which one that is).

On the other hand, it would make sense to include constructors in abstract
classes for informational purposes (when using reflection to "browse the code"),
and such constructors are of course useful in general because they can factor
out common pieces of code which would otherwise occur in many duplicates in the
constructors of concrete subclasses.

Do we have a place where the constructors in abstract classes are available for
"browsing"? Would be ClassMirror.declarations (checking: yes, that's
_everything_, all regular (instance or static) methods, getters, setters,
fields, constructors, and even type variables).

OK: So we should actually drop constructors of abstract classes here.

[sigurdm, 2015/06/19 07:50:54]

You cannot invoke abstract cosntructors directly, therefore we do not generate a
custom newInstance method.

     return """
 class ${classDomain.staticClassMirrorName} extends ClassMirrorUnimpl {
   final String simpleName = "${classDomain.classElement.name}";
@@ -879,6 +955,7 @@ class ${classDomain.staticClassMirrorName} extends ClassMirrorUnimpl {
     return _instanceMembersCache;
   }
 
+  ${memberStrings.join("\n\n  ")}
 }
 """;
   }
@@ -976,6 +1053,8 @@ class ${classDomain.staticClassMirrorName} extends ClassMirrorUnimpl {
         throw new UnimplementedError("$classElement not yet supported");
       case "InvokeStaticMemberCapability":
         throw new UnimplementedError("$classElement not yet supported");
+      case "_InvokeConstructorsCapability":
+        return invokeConstructorsCapability;

[eernst, 2015/06/18 13:35:41]

I believe this is the same thing as the capability
NewInstanceCapability([RegExp]) and the instance newInstanceCapability. So we
need to eliminate the one that we end up not using.

(PS: `newInstanceCapability` was named this way because it controls the
availability of `newInstance` on mirrors, just like `metadataCapability`
controls the avail. of `metadata`, etc., so `invokeConstructor` may be more
intuitive, but `newInstance` is probably more consistent.)

[sigurdm, 2015/06/19 07:50:54]

This was based on the previous incantation of capabilities. Should be updated in
the rebase.

       default:
         throw new UnimplementedError("Unexpected capability $classElement");
     }
@@ -1041,7 +1120,6 @@ class ${classDomain.staticClassMirrorName} extends ClassMirrorUnimpl {
   /// the given [classElement], bounded by the permissions given
   /// in [capabilities].
   String _staticInstanceMirrorInvokeCode(ClassDomain classDomain) {
-
     String tearOff(MethodElement methodElement) {
       if (!methodElement.isOperator) return "reflectee.${methodElement.name}";
       if (methodElement.name == "[]=") return "(x, v) => reflectee[x] = v";
@@ -1049,7 +1127,6 @@ class ${classDomain.staticClassMirrorName} extends ClassMirrorUnimpl {
       return "(x) => reflectee ${methodElement.name} x";
     }
 
-
     List<String> methodCases = new List<String>();
     for (MethodElement methodElement in classDomain.invokableMethods) {
       methodCases.add("if (memberName == '${methodElement.name}') {"