Make backend.setAnnotations (mostly) handle LibraryEntities.

pkg/compiler/lib/src/native/resolver.dart

Index: pkg/compiler/lib/src/native/resolver.dart
diff --git a/pkg/compiler/lib/src/native/resolver.dart b/pkg/compiler/lib/src/native/resolver.dart
index b7163e9a559a7c88cc5b0816bc9d2ef3b9ba2a82..0904bff2a56c744136682f15cb4a764f1e288723 100644
--- a/pkg/compiler/lib/src/native/resolver.dart
+++ b/pkg/compiler/lib/src/native/resolver.dart
@@ -30,6 +30,74 @@ import '../patch_parser.dart';
 import '../tree/tree.dart';
 import 'behavior.dart';
 
+/// Class that performs the mechanics to investigate annotations in the code.
+abstract class AnnotationProcessor {
+  factory AnnotationProcessor(Compiler compiler) =>
+      compiler.options.loadFromDill
+          ? new _KernelAnnotationProcessor()
+          : new _ElementAnnotationProcessor(compiler);

[Johnni Winther, 2017/04/19 08:39:51]

Eventually we should use a strategy pattern for all these:

abstract class FrontendStrategy {
  factory FrontendStrategy(Compiler compiler) =>
     compiler.options.loadFromDill
         ? new _KernelStrategy()
         : new _ResolverStrategy(compiler);

  LibraryLoaderTask createLibraryLoaderTask(...);
  ElementEnvironment createElementEnvironment();
  AnnotationProcessor createAnnotationProcessor();
  ...
}

class ResolverStrategy implements FrontEndStrategy {
  final Compiler _compiler;

  ResolverStrategy(this._compiler);

  LibraryLoader createLibraryLoader(...) => 
      new _LibraryLoaderTask(...);
  ElementEnvironment createElementEnvironment() => 
      new _CompilerElementEnvironment(_compiler);
  AnnotationProcessor createAnnotationProcessor() => 
      new _ElementAnnotationProcessor(_compiler);
  ...
}

class KernelStrategy implements FrontStrategy {
  KernelWorldBuilder _worldBuilder;

  KernelStrategy();

  LibraryLoader createLibraryLoader(...) => 
      new _DillLibraryLoaderTask(_worldBuilder, ...);
  ElementEnvironment createElementEnvironment() => 
      _worldBuilder.elementEnvironment;
  AnnotationProcessor createAnnotationProcessor() => 
       new _KernelAnnotationProcessor(_worldBuilder);
  ...
}

[Siggi Cherem (dart-lang), 2017/04/19 15:36:29]

An alternative naming convention could be to rename Strategy as Module,
 Factory, or Injector (I'm borrowing names that are commonly used to refer to
this in dependency-injection frameworks).

[Emily Fortuna, 2017/04/19 22:35:46]

I started doing this, but ultimately I removed it because it felt like I was
grouping things together that were technically unrelated to each other together
just because they were "the kernel way of doing things" which resulted in
passing a "strategy object" around all the time (to the backend and elsewhere)
instead of just what was needed. It felt a bit like a re-inventing of the
compiler object. WDYT?

+
+  void extractNativeAnnotations(
+      LibraryEntity library, NativeBasicDataBuilder nativeBasicDataBuilder);
+
+  void extractJsInteropAnnotations(
+      LibraryEntity library, NativeBasicDataBuilder nativeBasicDataBuilder);
+}
+
+class _KernelAnnotationProcessor implements AnnotationProcessor {

[Emily Fortuna, 2017/04/19 00:59:55]

this class would use the KernelLookup class, but I didn't implement any further
until I got the go-ahead that some sort of lookup thing is what Johnni was
envisioning.

+  void extractNativeAnnotations(
+      LibraryEntity entity, NativeBasicDataBuilder nativeBasicDataBuilder) {
+    throw new UnimplementedError(
+        '_KernelAnnotationProcessor.extractNativeAnnotations');
+  }
+
+  void extractJsInteropAnnotations(
+      LibraryEntity library, NativeBasicDataBuilder nativeBasicDataBuilder) {
+    throw new UnimplementedError(
+        '_KernelAnnotationProcessor.extractJsInteropAnnotations');
+  }
+}
+
+/// Original logic for annotation processing, which involves in some cases
+/// triggering pre-parsing and validation of the annotations.
+class _ElementAnnotationProcessor implements AnnotationProcessor {
+  Compiler _compiler;
+
+  _ElementAnnotationProcessor(this._compiler);
+
+  /// Check whether [cls] has a `@Native(...)` annotation, and if so, set its
+  /// native name from the annotation.
+  void extractNativeAnnotations(
+      LibraryEntity library, NativeBasicDataBuilder nativeBasicDataBuilder) {

[Johnni Winther, 2017/04/19 08:39:51]

Just use LibraryElement here. This is a model dependent class anyway.

[Emily Fortuna, 2017/04/19 22:35:46]

Done.

+    (library as LibraryElement).forEachLocalMember((Element element) {
+      if (element.isClass) {
+        EagerAnnotationHandler.checkAnnotation(_compiler, element,
+            new NativeAnnotationHandler(nativeBasicDataBuilder));
+      }
+    });
+  }
+
+  void extractJsInteropAnnotations(
+      LibraryEntity library, NativeBasicDataBuilder nativeBasicDataBuilder) {

[Johnni Winther, 2017/04/19 08:39:51]

Ditto

[Emily Fortuna, 2017/04/19 22:35:46]

Done.

+    bool checkJsInteropAnnotation(Element element) {
+      return EagerAnnotationHandler.checkAnnotation(
+          _compiler, element, const JsInteropAnnotationHandler());
+    }
+
+    if (checkJsInteropAnnotation(library as LibraryElement)) {
+      nativeBasicDataBuilder.markAsJsInteropLibrary(library);
+    }
+    (library as LibraryElement).forEachLocalMember((Element element) {
+      if (element.isClass) {
+        ClassElement cls = element;
+        if (checkJsInteropAnnotation(element)) {
+          nativeBasicDataBuilder.markAsJsInteropClass(cls);
+        }
+      }
+    });
+  }
+}
+
 /// Interface for computing native members and [NativeBehavior]s in member code
 /// based on the AST.
 abstract class NativeDataResolver {
@@ -327,26 +395,6 @@ class NativeAnnotationHandler extends EagerAnnotationHandler<String> {
   }
 }
 
-void checkJsInteropClassAnnotations(Compiler compiler, LibraryElement library,
-    NativeBasicDataBuilder nativeBasicDataBuilder) {
-  bool checkJsInteropAnnotation(Element element) {
-    return EagerAnnotationHandler.checkAnnotation(
-        compiler, element, const JsInteropAnnotationHandler());
-  }
-
-  if (checkJsInteropAnnotation(library)) {
-    nativeBasicDataBuilder.markAsJsInteropLibrary(library);
-  }
-  library.forEachLocalMember((Element element) {
-    if (element.isClass) {
-      ClassElement cls = element;
-      if (checkJsInteropAnnotation(element)) {
-        nativeBasicDataBuilder.markAsJsInteropClass(cls);
-      }
-    }
-  });
-}
-
 bool checkJsInteropMemberAnnotations(Compiler compiler, MemberElement element,
     NativeDataBuilder nativeDataBuilder) {
   bool isJsInterop = EagerAnnotationHandler.checkAnnotation(