Handle js interop members in impact computation.

pkg/compiler/lib/src/kernel/native_basic_data.dart

Index: pkg/compiler/lib/src/kernel/native_basic_data.dart
diff --git a/pkg/compiler/lib/src/kernel/native_basic_data.dart b/pkg/compiler/lib/src/kernel/native_basic_data.dart
index b9420fa270ba2cddc7251310039689f6a4c7caae..7ae6d5dddfa0f04266c1853465b0610ce9101136 100644
--- a/pkg/compiler/lib/src/kernel/native_basic_data.dart
+++ b/pkg/compiler/lib/src/kernel/native_basic_data.dart
@@ -17,9 +17,7 @@ class KernelAnnotationProcessor implements AnnotationProcessor {
 
     elementEnvironment.forEachClass(library, (ClassEntity cls) {
       String annotationName;
-      // TODO(johnniwinther): Make [_getClassMetadata] public and at test to
-      // guard against misuse.
-      for (ConstantValue value in elementMap._getClassMetadata(cls)) {
+      for (ConstantValue value in elementEnvironment.getClassMetadata(cls)) {
         String name = readAnnotationName(
             cls, value, commonElements.nativeAnnotationClass);
         if (annotationName == null) {
@@ -34,13 +32,129 @@ class KernelAnnotationProcessor implements AnnotationProcessor {
     });
   }
 
+  String getJsInteropName(
+      Spannable spannable, Iterable<ConstantValue> metadata) {
+    CommonElements commonElements = elementMap.commonElements;
+    String annotationName;
+    for (ConstantValue value in metadata) {
+      String name = readAnnotationName(
+          spannable, value, commonElements.jsAnnotationClass,
+          defaultValue: '');
+      if (annotationName == null) {
+        annotationName = name;
+      } else if (name != null) {
+        failedAt(spannable, 'Too many name annotations.');

[Siggi Cherem (dart-lang), 2017/08/08 00:06:46]

should this be a warning/error to report to the user (rather than a crash?)

[Johnni Winther, 2017/08/09 08:34:36]

It should. Adding a TODO.

+      }
+    }
+    return annotationName;
+  }
+
+  void checkFunctionParameters(FunctionEntity function) {
+    if (function.parameterStructure.namedParameters.isNotEmpty) {
+      elementMap.reporter.reportErrorMessage(
+          function,
+          MessageKind.JS_INTEROP_METHOD_WITH_NAMED_ARGUMENTS,
+          {'method': function.name});
+    }
+  }
+
   void extractJsInteropAnnotations(LibraryEntity library) {
-    // TODO(redemption): Implement this.
+    DiagnosticReporter reporter = elementMap.reporter;
+    ElementEnvironment elementEnvironment = elementMap.elementEnvironment;
+    CommonElements commonElements = elementMap.commonElements;
+
+    // TODO(johnniwinther): Mark library directly when .dill supports library

[Siggi Cherem (dart-lang), 2017/08/08 00:06:46]

I wasn't sure I followed this withotu looking at the entire code. Do you mean
that libraryName is null today because we are missing this in fasta, and
therefore you added the `isJsLibrary=true` further below?

If so, here is a suggestion:
- change the code here to initialize isJsLibrary one line further down:

  bool isJsLibrary = (libraryName != null);

- move the TODO to the `isJsLibrary=true`. Then it should say:
   * delete when the FE supports library metadata
   * report an error if it is not `true` already.

[Johnni Winther, 2017/08/09 08:34:36]

Done.

+    // metadata.
+    bool isJsLibrary = false;
+    String libraryName = getJsInteropName(
+        library, elementEnvironment.getLibraryMetadata(library));
+    if (libraryName != null) {
+      isJsLibrary = true;
+    }
+    elementEnvironment.forEachLibraryMember(library, (MemberEntity member) {
+      if (member.isField) return;
+      String memberName = getJsInteropName(
+          library, elementEnvironment.getMemberMetadata(member));
+      if (memberName != null) {
+        _nativeBasicDataBuilder.markAsJsInteropMember(member, memberName);
+        checkFunctionParameters(member);
+      }
+    });
+
+    elementEnvironment.forEachClass(library, (ClassEntity cls) {
+      Iterable<ConstantValue> metadata =
+          elementEnvironment.getClassMetadata(cls);
+      String className = getJsInteropName(cls, metadata);
+      if (className != null) {
+        bool isAnonymous = false;
+        for (ConstantValue value in metadata) {
+          if (isAnnotation(cls, value, commonElements.jsAnonymousClass)) {
+            isAnonymous = true;
+            break;
+          }
+        }
+        _nativeBasicDataBuilder.markAsJsInteropClass(cls,
+            name: className, isAnonymous: isAnonymous);
+        // For now, assume the library is a js-interop library.
+        isJsLibrary = true;
+
+        ClassEntity superclass = elementEnvironment.getSuperClass(cls);
+        if (superclass != commonElements.jsJavaScriptObjectClass) {
+          reporter.reportErrorMessage(
+              cls,
+              MessageKind.JS_INTEROP_CLASS_CANNOT_EXTEND_DART_CLASS,
+              {'cls': cls.name, 'superclass': superclass.name});
+        }
+
+        elementEnvironment.forEachClassMember(cls,
+            (ClassEntity declarer, MemberEntity member) {
+          if (declarer != cls) return;
+          if (member.isField) return;
+          FunctionEntity function = member;
+
+          String memberName = getJsInteropName(
+              library, elementEnvironment.getMemberMetadata(function));
+          if (memberName != null) {
+            _nativeBasicDataBuilder.markAsJsInteropMember(function, memberName);
+          }
+
+          if (!function.isExternal &&
+              !function.isAbstract &&
+              !function.isConstructor &&
+              !function.isStatic) {
+            reporter.reportErrorMessage(
+                function,
+                MessageKind.JS_INTEROP_CLASS_NON_EXTERNAL_MEMBER,
+                {'cls': cls.name, 'member': member.name});
+          }
+
+          if (function is ConstructorEntity &&
+              function.isFactoryConstructor &&
+              isAnonymous) {
+            if (function.parameterStructure.requiredParameters > 0) {
+              reporter.reportErrorMessage(
+                  function,
+                  MessageKind
+                      .JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS,
+                  {'cls': cls.name});
+            }
+          } else {
+            checkFunctionParameters(function);
+          }
+        });
+      }
+    });
+    if (isJsLibrary) {
+      // Assume the empty name.
+      libraryName ??= '';

[Siggi Cherem (dart-lang), 2017/08/08 00:06:46]

I guess this line will also have a TODO delete?

[Johnni Winther, 2017/08/09 08:34:36]

Done.

+      _nativeBasicDataBuilder.markAsJsInteropLibrary(library,
+          name: libraryName);
+    }
   }
 
   @override
   void processJsInteropAnnotations(
-      NativeBasicData nativeData, NativeDataBuilder nativeDataBuilder) {
-    // TODO(redemption): Implement this.
+      NativeBasicData nativeBasicData, NativeDataBuilder nativeDataBuilder) {
+    // Nothing to do; all is computed in [extractJsInteropAnnotations].
   }
 }