Major refactor of the transformer, added an `InitializePlugin` class which allows you to hook direc…

lib/transformer.dart

Index: lib/transformer.dart
diff --git a/lib/transformer.dart b/lib/transformer.dart
index 6374bbbcb16805f3e0e2dad069cdd499f26aefe6..cb4a7bfb56c626acfda2fb2a09573eb5de7e33d1 100644
--- a/lib/transformer.dart
+++ b/lib/transformer.dart
@@ -15,14 +15,34 @@ import 'package:html5lib/dom.dart' as dom;
 import 'package:html5lib/parser.dart' show parse;
 import 'package:path/path.dart' as path;
 
-/// Removes the mirror-based initialization logic and replaces it with static
-/// logic.
+import 'build/initializer_plugin.dart';
+export 'build/initializer_plugin.dart';
+
+/// Create a new [Asset] which inlines your [Initializer] annotations into
+/// a new file that bootstraps your application.
+Asset bootstrapInitializers(Resolver resolver, Transform transform,

[Siggi Cherem (dart-lang), 2015/02/13 16:45:37]

looking at the other CL, I think we should rename this a bit.
"bootstrapInitializers" sounds to me like you are about to run the initializers,
some ideas:
 - generateBootstrapWithInitializers
 - generateInitializationMain
 - generateBootstrapFile

[jakemac, 2015/02/13 20:16:05]

chose generateBootstrapFile

+    AssetId primaryAssetId, AssetId newEntryPointId,
+    {bool errorIfNotFound: true, List<InitializerPlugin> plugins,
+    bool appendDefaultPlugin: true}) {

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

I wonder if we need this, or we should just let them populate it into the
[plugins] list. 

Then we would just add it by default when plugins is null, but that's about it.

[jakemac, 2015/02/13 20:16:05]

I think it would be an extremely rare case where we wouldn't want this to be
added though, and it would be really easy for users to forget to add it.

The fact that it is a plugin is really just an implementation detail, that most
people should never care about imo.

+  if (appendDefaultPlugin) {
+    if (plugins == null) plugins = [];
+    plugins.add(const DefaultInitializerPlugin());
+  }
+  return new _BootstrapFileBuilder(
+      resolver, transform, primaryAssetId, newEntryPointId, errorIfNotFound,
+      plugins: plugins).run();
+}
+
+/// Transformer which removes the mirror-based initialization logic and replaces
+/// it with static logic.
 class InitializeTransformer extends Transformer {
   final Resolvers _resolvers;
   final Iterable<Glob> _entryPointGlobs;
   final bool _errorIfNotFound;
+  final List<InitializerPlugin> plugins;
 
-  InitializeTransformer(List<String> entryPoints, {bool errorIfNotFound: true})
+  InitializeTransformer(List<String> entryPoints,
+      {bool errorIfNotFound: true, this.plugins})
       : _entryPointGlobs = entryPoints.map((e) => new Glob(e)),
         _errorIfNotFound = errorIfNotFound,
         _resolvers = new Resolvers.fromMock({
@@ -106,13 +126,36 @@ class InitializeTransformer extends Transformer {
       }
 
       return _resolvers.get(transform, [primaryId]).then((resolver) {
-        new _BootstrapFileBuilder(resolver, transform, primaryId,
-            newEntryPointId, _errorIfNotFound).run();
+        transform.addOutput(bootstrapInitializers(
+            resolver, transform, primaryId, newEntryPointId,
+            errorIfNotFound: _errorIfNotFound, plugins: plugins));
         resolver.release();
         return newEntryPointId;
       });
     });
   }
+  // Finds the first (and only) dart script on an html page and returns the
+  // [AssetId] that points to it
+  AssetId _findMainScript(
+      dom.Document document, AssetId entryPoint, Transform transform) {
+    var scripts = document.querySelectorAll('script[type="application/dart"]');
+    if (scripts.length != 1) {
+      transform.logger.error('Expected exactly one dart script in $entryPoint '
+          'but found ${scripts.length}.');
+      return null;
+    }
+
+    var src = scripts[0].attributes['src'];
+    // TODO(jakemac): Support inline scripts,
+    // https://github.com/dart-lang/initialize/issues/20
+    if (src == null) {
+      transform.logger.error('Inline scripts are not supported at this time.');

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

we could also point to the bug number here so people can follow it if they care.

[jakemac, 2015/02/13 20:16:06]

Done.

+      return null;
+    }
+
+    return uriToAssetId(
+        entryPoint, src, transform.logger, scripts[0].sourceSpan);
+  }
 
   // Replaces script tags pointing to [originalDartFile] with [newDartFile] in
   // [entryPoint].
@@ -122,7 +165,8 @@ class InitializeTransformer extends Transformer {
     var scripts = document
         .querySelectorAll('script[type="application/dart"]')
         .where((script) => uriToAssetId(entryPoint, script.attributes['src'],
-            transform.logger, script.sourceSpan) == originalDartFile)
+                transform.logger, script.sourceSpan) ==

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

formatter bug? yikes!

[jakemac, 2015/02/13 20:16:06]

Ya this is pretty bad :(. I broke down and changed this into the {} syntax with
a local variable so its not so horrendous, although it does hit a different bug.
I also filed https://github.com/dart-lang/dart_style/issues/169

+            originalDartFile)
         .toList();
 
     if (scripts.length != 1) {
@@ -135,29 +179,9 @@ class InitializeTransformer extends Transformer {
         from: path.dirname(entryPoint.path));
     transform.addOutput(new Asset.fromString(entryPoint, document.outerHtml));
   }
-
-  AssetId _findMainScript(
-      dom.Document document, AssetId entryPoint, Transform transform) {
-    var scripts = document.querySelectorAll('script[type="application/dart"]');
-    if (scripts.length != 1) {
-      transform.logger.error('Expected exactly one dart script in $entryPoint '
-          'but found ${scripts.length}.');
-      return null;
-    }
-
-    var src = scripts[0].attributes['src'];
-    // TODO(jakemac): Support inline scripts,
-    // https://github.com/dart-lang/initialize/issues/20
-    if (src == null) {
-      transform.logger.error('Inline scripts are not supported at this time.');
-      return null;
-    }
-
-    return uriToAssetId(
-        entryPoint, src, transform.logger, scripts[0].sourceSpan);
-  }
 }
 
+// Class which builds a bootstrap file.
 class _BootstrapFileBuilder {
   final Resolver _resolver;
   final Transform _transform;
@@ -171,14 +195,19 @@ class _BootstrapFileBuilder {
   ClassElement _initializer;
 
   /// Queue for intialization annotations.
-  final _initQueue = new Queue<_InitializerData>();
+  final _initQueue = new Queue<InitializerData>();
   /// All the annotations we have seen for each element
   final _seenAnnotations = new Map<Element, Set<ElementAnnotation>>();
 
+  /// The list of [InitializerPlugin]s to apply. The first plugin which asks to
+  /// be applied to a given initializer is the only one that will apply.
+  List<InitializerPlugin> _plugins;
+
   TransformLogger _logger;
 
   _BootstrapFileBuilder(this._resolver, this._transform, this._entryPoint,
-      this._newEntryPoint, this._errorIfNotFound) {
+      this._newEntryPoint, this._errorIfNotFound,
+      {List<InitializerPlugin> plugins}) {
     _logger = _transform.logger;
     _initializeLibrary =
         _resolver.getLibrary(new AssetId('initialize', 'lib/initialize.dart'));
@@ -189,15 +218,16 @@ class _BootstrapFileBuilder {
           'This file must be imported via $_entryPoint or a transitive '
           'dependency.');
     }
+    _plugins =
+        plugins != null ? plugins : const [const DefaultInitializerPlugin()];

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

not sure if the const list here will be much cheaper, I'd remove it for the sake
of fitting this in a single line :)

Same thing with const vs new on the DefaultInitia...

[jakemac, 2015/02/13 20:16:05]

left the const on the DefaultInitializerPlugin since that doesn't cause line
wrap, removed it from the list though.

   }
 
-  /// Adds the new entry point file to the transform. Should only be ran once.
-  void run() {
+  /// Creates and returns the new bootstrap file.
+  Asset run() {
     var entryLib = _resolver.getLibrary(_entryPoint);
     _readLibraries(entryLib);
 
-    _transform.addOutput(
-        new Asset.fromString(_newEntryPoint, _buildNewEntryPoint(entryLib)));
+    return new Asset.fromString(_newEntryPoint, _buildNewEntryPoint(entryLib));
   }
 
   /// Reads Initializer annotations on this library and all its dependencies in
@@ -252,7 +282,7 @@ class _BootstrapFileBuilder {
       return !_seenAnnotations[element].contains(meta);
     }).forEach((ElementAnnotation meta) {
       _seenAnnotations[element].add(meta);
-      _initQueue.addLast(new _InitializerData(element, meta));
+      _initQueue.addLast(new InitializerData(element, meta));
       found = true;
     });
     return found;
@@ -275,10 +305,25 @@ class _BootstrapFileBuilder {
 
       libraryPrefixes.putIfAbsent(
           next.element.library, () => 'i${libraryPrefixes.length}');
-      libraryPrefixes.putIfAbsent(
-          next.annotation.element.library, () => 'i${libraryPrefixes.length}');
-
-      _writeInitializer(next, libraryPrefixes, initializersBuffer);
+      libraryPrefixes.putIfAbsent(next.elementAnnotation.element.library,
+          () => 'i${libraryPrefixes.length}');
+
+      // Run the first plugin which asks to be ran and then stop.
+      var i = 0;
+      var pluginRan = false;
+      var pluginText;

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

delete this one? (it's declared again inside the loop)

[jakemac, 2015/02/13 20:16:06]

Done.

+      while (!pluginRan && i < _plugins.length) {

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

maybe we could simplify it like this:

  var data = ...;
  var plugin = plugins.firstWhere((p) => p.shouldApply(data));
  var text = plugin.apply(data);
  if (text != null) initializersBuffer.writeln('$text,');

[jakemac, 2015/02/13 20:16:05]

Done, this is much better for sure. I also added a try/catch with a logger error
if firstWhere throws.

+        var plugin = _plugins[i];
+        var pluginData = new InitializerPluginData(
+            next, _newEntryPoint, libraryPrefixes, _resolver, _logger);
+        i++;
+        if (!plugin.shouldApply(pluginData)) continue;
+        var pluginText = plugin.apply(pluginData);
+        if (pluginText != null) {
+          initializersBuffer.writeln('$pluginText,');
+        }
+        pluginRan = true;
+      }
     }
     initializersBuffer.writeln('  ]);');
 
@@ -314,164 +359,6 @@ $initializersBuffer
     buffer.writeln(' as $prefix;');
   }
 
-  _writeInitializer(_InitializerData data,
-      Map<LibraryElement, String> libraryPrefixes, StringBuffer buffer) {
-    final annotationElement = data.annotation.element;
-    final element = data.element;
-
-    final metaPrefix = libraryPrefixes[annotationElement.library];
-    var elementString;
-    if (element is LibraryElement) {
-      var segments = element.source.uri.pathSegments;
-      var package = segments[0];
-      var libraryPath;
-      var packageString;
-      if (_newEntryPoint.package == package &&
-          _newEntryPoint.path.startsWith('${segments[1]}/')) {
-        // reset `package` to null, we will do a relative path in this case.
-        packageString = 'null';
-        libraryPath = path.url.relative(
-            path.url.joinAll(segments.getRange(1, segments.length)),
-            from: path.url.dirname(path.url.join(_newEntryPoint.path)));
-      } else if (segments[1] == 'lib') {
-        packageString = "'$package'";
-        libraryPath = path.url.joinAll(segments.getRange(2, segments.length));
-      } else {
-        _logger.error('Unable to import `${element.source.uri.path}` from '
-            '${_newEntryPoint.path}.');
-      }
-
-      elementString = "const LibraryIdentifier("
-          "#${element.name}, $packageString, '$libraryPath')";
-    } else if (element is ClassElement || element is FunctionElement) {
-      elementString =
-          '${libraryPrefixes[data.element.library]}.${element.name}';
-    } else {
-      _logger.error('Initializers can only be applied to top level functins, '
-          'libraries, and classes.');
-    }
-
-    if (annotationElement is ConstructorElement) {
-      var node = data.element.node;
-      List<Annotation> astMeta;
-      if (node is SimpleIdentifier) {
-        astMeta = node.parent.parent.metadata;
-      } else if (node is ClassDeclaration || node is FunctionDeclaration) {
-        astMeta = node.metadata;
-      } else {
-        _logger.error(
-            'Initializer annotations are only supported on libraries, classes, '
-            'and top level methods. Found $node.');
-      }
-      final annotation =
-          astMeta.firstWhere((m) => m.elementAnnotation == data.annotation);
-      final clazz = annotation.name;
-      final constructor = annotation.constructorName == null
-          ? ''
-          : '.${annotation.constructorName}';
-      // TODO(jakemac): Support more than raw values here
-      // https://github.com/dart-lang/static_init/issues/5
-      final args = _buildArgsString(annotation.arguments, libraryPrefixes);
-      buffer.write('''
-    new InitEntry(const $metaPrefix.${clazz}$constructor$args, $elementString),
-''');
-    } else if (annotationElement is PropertyAccessorElement) {
-      buffer.write('''
-    new InitEntry($metaPrefix.${annotationElement.name}, $elementString),
-''');
-    } else {
-      _logger.error('Unsupported annotation type. Only constructors and '
-          'properties are supported as initializers.');
-    }
-  }
-
-  String _buildArgsString(
-      ArgumentList args, Map<LibraryElement, String> libraryPrefixes) {
-    var buffer = new StringBuffer();
-    buffer.write('(');
-    var first = true;
-    for (var arg in args.arguments) {
-      if (!first) buffer.write(', ');
-      first = false;
-
-      Expression expression;
-      if (arg is NamedExpression) {
-        buffer.write('${arg.name.label.name}: ');
-        expression = arg.expression;
-      } else {
-        expression = arg;
-      }
-
-      buffer.write(_expressionString(expression, libraryPrefixes));
-    }
-    buffer.write(')');
-    return buffer.toString();
-  }
-
-  String _expressionString(
-      Expression expression, Map<LibraryElement, String> libraryPrefixes) {
-    var buffer = new StringBuffer();
-    if (expression is StringLiteral) {
-      var value = expression.stringValue;
-      if (value == null) {
-        _logger.error('Only const strings are allowed in initializer '
-            'expressions, found $expression');
-      }
-      value = value.replaceAll(r'\', r'\\').replaceAll(r"'", r"\'");
-      buffer.write("'$value'");
-    } else if (expression is BooleanLiteral ||
-        expression is DoubleLiteral ||
-        expression is IntegerLiteral ||
-        expression is NullLiteral) {
-      buffer.write('${expression}');
-    } else if (expression is ListLiteral) {
-      buffer.write('const [');
-      var first = true;
-      for (Expression listExpression in expression.elements) {
-        if (!first) buffer.write(', ');
-        first = false;
-        buffer.write(_expressionString(listExpression, libraryPrefixes));
-      }
-      buffer.write(']');
-    } else if (expression is MapLiteral) {
-      buffer.write('const {');
-      var first = true;
-      for (MapLiteralEntry entry in expression.entries) {
-        if (!first) buffer.write(', ');
-        first = false;
-        buffer.write(_expressionString(entry.key, libraryPrefixes));
-        buffer.write(': ');
-        buffer.write(_expressionString(entry.value, libraryPrefixes));
-      }
-      buffer.write('}');
-    } else if (expression is Identifier) {
-      var element = expression.bestElement;
-      if (element == null || !element.isPublic) {
-        _logger.error('Private constants are not supported in intializer '
-            'constructors, found $element.');
-      }
-      libraryPrefixes.putIfAbsent(
-          element.library, () => 'i${libraryPrefixes.length}');
-
-      buffer.write('${libraryPrefixes[element.library]}.');
-      if (element is ClassElement) {
-        buffer.write(element.name);
-      } else if (element is PropertyAccessorElement) {
-        var variable = element.variable;
-        if (variable is FieldElement) {
-          buffer.write('${variable.enclosingElement.name}.');
-        }
-        buffer.write('${variable.name}');
-      } else {
-        _logger.error('Unsupported argument to initializer constructor.');
-      }
-    } else {
-      _logger.error('Only literals and identifiers are allowed for initializer '
-          'expressions, found $expression.');
-    }
-    return buffer.toString();
-  }
-
   bool _isInitializer(InterfaceType type) {
     // If `_initializer` wasn't found then it was never loaded (even
     // transitively), and so no annotations can be initializers.
@@ -561,12 +448,36 @@ $initializersBuffer
     })).map((import) => import.importedLibrary);
 }
 
-// Element/ElementAnnotation pair.
-class _InitializerData {
+/// Element/ElementAnnotation pair.

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

reword - maybe just focus on the fact that this is a target and an annotation?

[jakemac, 2015/02/13 20:16:06]

Done.

+class InitializerData {
   final Element element;

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

Since this class is now part of the plugin API, we should add dartdocs for each
member. 

Maybe rename:
 element -> targetElement
 node -> targetNode

[jakemac, 2015/02/13 20:16:05]

done, also renamed elementAnnotation -> annotationElement and annotation ->
annotationNode

-  final ElementAnnotation annotation;
+  final ElementAnnotation elementAnnotation;
+  AstNode _node;
+
+  // At least cache this for now, but provides a path to doing something better.

[Siggi Cherem (dart-lang), 2015/02/13 00:50:36]

nit: add a "TODO(jackemac): " prefix here

[jakemac, 2015/02/13 20:16:05]

Done.

+  // Ideally `element` would actually be the getter, and `node` would be the
+  // property.
+  AstNode get node {
+    if (_node == null) _node = element.node;
+    return _node;
+  }
+
+  Annotation get annotation {
+    var annotatedNode;
+    if (node is SimpleIdentifier && node.parent is LibraryIdentifier) {
+      annotatedNode = node.parent.parent;
+    } else if (node is ClassDeclaration || node is FunctionDeclaration) {
+      annotatedNode = node;
+    } else {
+      return null;
+    }
+    if (annotatedNode is! AnnotatedNode) return null;
+    var astMeta = annotatedNode.metadata;
+
+    return astMeta.firstWhere((m) => m.elementAnnotation == elementAnnotation);
+  }
 
-  _InitializerData(this.element, this.annotation);
+  InitializerData(this.element, this.elementAnnotation);

[jakemac, 2015/02/13 20:16:05]

I also renamed this to a private constructor, so we can change it to take a node
later without issue.

 }
 
 // Reads a file list from a barback settings configuration field.