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

lib/transformer.dart

 // Copyright (c) 2015, 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 initialize.transformer;
 
 import 'dart:async';
 import 'dart:collection' show Queue;
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
 import 'package:barback/barback.dart';
 import 'package:code_transformers/assets.dart';
 import 'package:code_transformers/resolver.dart';
 import 'package:glob/glob.dart';
 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({
           // The list of types below is derived from:
           //   * types that are used internally by the resolver (see
           //   _initializeFrom in resolver.dart).
           // TODO(jakemac): Move this into code_transformers so it can be shared.
           'dart:core': '''
             library dart.core;
             class Object {}
@@ skipping 63 matching lines @@
     var newEntryPointId = new AssetId(primaryId.package,
         '${path.url.withoutExtension(primaryId.path)}.initialize.dart');
     return transform.hasInput(newEntryPointId).then((exists) {
       if (exists) {
         transform.logger
             .error('New entry point file $newEntryPointId already exists.');
         return null;
       }
 
       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].
   void _replaceEntryWithBootstrap(Transform transform, dom.Document document,
       AssetId entryPoint, AssetId originalDartFile, AssetId newDartFile) {
     var found = false;
     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) {
       transform.logger.error(
           'Expected exactly one script pointing to $originalDartFile in '
           '$entryPoint, but found ${scripts.length}.');
       return;
     }
     scripts[0].attributes['src'] = path.url.relative(newDartFile.path,
         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;
   final bool _errorIfNotFound;
   AssetId _entryPoint;
   AssetId _newEntryPoint;
 
   /// The resolved initialize library.
   LibraryElement _initializeLibrary;
   /// The resolved Initializer class from the initialize library.
   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'));
     if (_initializeLibrary != null) {
       _initializer = _initializeLibrary.getType('Initializer');
     } else if (_errorIfNotFound) {
       _logger.warning('Unable to read "package:initialize/initialize.dart". '
           '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
   /// post-order.
   void _readLibraries(LibraryElement library, [Set<LibraryElement> seen]) {
     if (seen == null) seen = new Set<LibraryElement>();
     seen.add(library);
 
     // Visit all our dependencies.
     for (var importedLibrary in _sortedLibraryImports(library)) {
@@ skipping 34 matching lines @@
         return _isInitializer(e.variable.evaluationResult.value.type);
       } else if (e is ConstructorElement) {
         return _isInitializer(e.returnType);
       }
       return false;
     }).where((ElementAnnotation meta) {
       _seenAnnotations.putIfAbsent(element, () => new Set<ElementAnnotation>());
       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;
   }
 
   String _buildNewEntryPoint(LibraryElement entryLib) {
     var importsBuffer = new StringBuffer();
     var initializersBuffer = new StringBuffer();
     var libraryPrefixes = new Map<LibraryElement, String>();
 
     // Import the static_loader, initializer, and original entry point.
     importsBuffer
         .writeln("import 'package:initialize/src/static_loader.dart';");
     importsBuffer.writeln("import 'package:initialize/initialize.dart';");
     libraryPrefixes[entryLib] = 'i0';
 
     initializersBuffer.writeln('  initializers.addAll([');
     while (_initQueue.isNotEmpty) {
       var next = _initQueue.removeFirst();
 
       libraryPrefixes.putIfAbsent(
           next.element.library, () => 'i${libraryPrefixes.length}');
-      libraryPrefixes.putIfAbsent(
-          next.annotation.element.library, () => 'i${libraryPrefixes.length}');
+      libraryPrefixes.putIfAbsent(next.elementAnnotation.element.library,
+          () => 'i${libraryPrefixes.length}');
 
-      _writeInitializer(next, libraryPrefixes, initializersBuffer);
+      // 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('  ]);');
 
     libraryPrefixes
         .forEach((lib, prefix) => _writeImport(lib, prefix, importsBuffer));
 
     // TODO(jakemac): copyright and library declaration
     return '''
 $importsBuffer
 main() {
@@ skipping 15 matching lines @@
         path.url.split(_newEntryPoint.path)[0]) {
       var relativePath =
           path.relative(id.path, from: path.dirname(_newEntryPoint.path));
       buffer.write("import '${relativePath}'");
     } else {
       _logger.error("Can't import `${id}` from `${_newEntryPoint}`");
     }
     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.
     if (_initializer == null) return false;
     if (type == null) return false;
     if (type.element.type == _initializer.type) return true;
     if (_isInitializer(type.superclass)) return true;
     for (var interface in type.interfaces) {
       if (_isInitializer(interface)) return true;
     }
@@ skipping 69 matching lines @@
 
       // And finally compare based on the relative uri if both are file paths.
       var aUri = path.relative(a.source.uri.path,
           from: path.dirname(library.source.uri.path));
       var bUri = path.relative(b.source.uri.path,
           from: path.dirname(library.source.uri.path));
       return aUri.compareTo(bUri);
     })).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;
 
-  _InitializerData(this.element, this.annotation);
+  // 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.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.
 _readFileList(BarbackSettings settings, String field) {
   var value = settings.configuration[field];
   if (value == null) return null;
   var files = [];
   bool error;
   if (value is List) {
     files = value;
     error = value.any((e) => e is! String);
   } else if (value is String) {
     files = [value];
     error = false;
   } else {
     error = true;
   }
   if (error) {
     print('Bad value for "$field" in the initialize transformer. '
         'Expected either one String or a list of Strings.');
   }
   return files;
 }