Use static DartType for js_util optimizations.

Change-Id: I294af3925f15a438f8ad76b538de4ca3a227749e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202440
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Riley Porter <rileyporter@google.com>

pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart

@@ -3,7 +3,9 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:kernel/ast.dart';
+import 'package:kernel/class_hierarchy.dart';
 import 'package:kernel/core_types.dart';
+import 'package:kernel/type_environment.dart';
 import 'package:kernel/kernel.dart';
 
 /// Replaces js_util methods with inline calls to foreign_helper JS which
@@ -26,20 +28,41 @@ class JsUtilOptimizer extends Transformer {
   final Iterable<Procedure> _allowedInteropJsUtilTargets;
   final Procedure _allowInteropTarget;
 
-  JsUtilOptimizer(CoreTypes coreTypes)
+  final CoreTypes _coreTypes;
+  final StatefulStaticTypeContext _staticTypeContext;
+
+  JsUtilOptimizer(this._coreTypes, ClassHierarchy hierarchy)
       : _jsTarget =
-            coreTypes.index.getTopLevelMember('dart:_foreign_helper', 'JS'),
+            _coreTypes.index.getTopLevelMember('dart:_foreign_helper', 'JS'),
         _getPropertyTarget =
-            coreTypes.index.getTopLevelMember('dart:js_util', 'getProperty'),
+            _coreTypes.index.getTopLevelMember('dart:js_util', 'getProperty'),
         _setPropertyTarget =
-            coreTypes.index.getTopLevelMember('dart:js_util', 'setProperty'),
-        _setPropertyUncheckedTarget = coreTypes.index
+            _coreTypes.index.getTopLevelMember('dart:js_util', 'setProperty'),
+        _setPropertyUncheckedTarget = _coreTypes.index
             .getTopLevelMember('dart:js_util', '_setPropertyUnchecked'),
         _allowInteropTarget =
-            coreTypes.index.getTopLevelMember('dart:js', 'allowInterop'),
+            _coreTypes.index.getTopLevelMember('dart:js', 'allowInterop'),
         _allowedInteropJsUtilTargets = _allowedInteropJsUtilMembers.map(
             (member) =>
-                coreTypes.index.getTopLevelMember('dart:js_util', member)) {}
+                _coreTypes.index.getTopLevelMember('dart:js_util', member)),
+        _staticTypeContext = StatefulStaticTypeContext.stacked(
+            TypeEnvironment(_coreTypes, hierarchy)) {}
+
+  @override
+  visitLibrary(Library lib) {
+    _staticTypeContext.enterLibrary(lib);
+    lib.transformChildren(this);
+    _staticTypeContext.leaveLibrary(lib);
+    return lib;
+  }
+
+  @override
+  defaultMember(Member node) {
+    _staticTypeContext.enterMember(node);
+    node.transformChildren(this);
+    _staticTypeContext.leaveMember(node);
+    return node;
+  }
 
   /// Replaces js_util method calls with optimization when possible.
   ///
@@ -96,39 +119,32 @@ class JsUtilOptimizer extends Transformer {
       ..fileOffset = node.fileOffset;
   }
 
-  /// Returns whether the given TreeNode is guaranteed to be allowed to interop
-  /// with JS.
+  /// Returns whether the given Expression is guaranteed to be allowed to
+  /// interop with JS.
   ///
   /// Returns true when the node is guaranteed to be not a function:
-  ///    - has a DartType that is NullType or an InterfaceType that is not
-  ///      Function or Object
+  ///    - has a static DartType that is NullType or an InterfaceType that is
+  ///      not Function or Object
   /// Also returns true for allowed method calls within the JavaScript domain:
   ///        - dart:_foreign_helper JS
   ///        - dart:js `allowInterop`
   ///        - dart:js_util and any of the `_allowedInteropJsUtilMembers`
-  bool _allowedInterop(TreeNode node) {
+  bool _allowedInterop(Expression node) {
     // TODO(rileyporter): Detect functions that have been wrapped at some point
     // with `allowInterop`
-    // TODO(rileyporter): Use staticTypeContext to generalize type checking and
-    // allow more non-function types. Currently, we skip all literal types.
-    var checkType;
-    if (node is VariableGet) {
-      checkType = node.variable.type;
-    }
-
     if (node is StaticInvocation) {
       if (node.target == _allowInteropTarget) return true;
       if (node.target == _jsTarget) return true;
       if (_allowedInteropJsUtilTargets.contains(node.target)) return true;
-      checkType = node.target.function.returnType;
     }
 
-    if (checkType is InterfaceType) {
-      return checkType.classNode.name != 'Function' &&
-          checkType.classNode.name != 'Object';
+    var type = node.getStaticType(_staticTypeContext);
+    if (type is InterfaceType) {
+      return type.classNode != _coreTypes.functionClass &&
+          type.classNode != _coreTypes.objectClass;
     } else {
       // Only other DartType guaranteed to not be a function.
-      return checkType is NullType;
+      return type is NullType;
     }
   }
 }

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

@@ -154,7 +154,7 @@ class Dart2jsTarget extends Target {
       {void logger(String msg),
       ChangedStructureNotifier changedStructureNotifier}) {
     _nativeClasses ??= JsInteropChecks.getNativeClasses(component);
-    var jsUtilOptimizer = JsUtilOptimizer(coreTypes);
+    var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy);
     for (var library in libraries) {
       // TODO (rileyporter): Merge js_util optimizations with other lowerings
       // in the single pass in `transformations/lowering.dart`.

pkg/dev_compiler/lib/src/kernel/target.dart

@@ -159,7 +159,7 @@ class DevCompilerTarget extends Target {
       {void Function(String msg) logger,
       ChangedStructureNotifier changedStructureNotifier}) {
     _nativeClasses ??= JsInteropChecks.getNativeClasses(component);
-    var jsUtilOptimizer = JsUtilOptimizer(coreTypes);
+    var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy);
     for (var library in libraries) {
       _CovarianceTransformer(library).transform();
       jsUtilOptimizer.visitLibrary(library);

tests/lib/js/js_util/properties_test.dart

@@ -50,11 +50,20 @@ class ExampleTypedLiteral {
   external get b;
 }
 
-class ExampleTearoff {
+class DartClass {
   int x = 3;
-  foo() => x;
+  int getX() => x;
 }
 
+class GenericDartClass<T> {
+  final T myT;
+  GenericDartClass(this.myT);
+
+  T getT() => myT;
+}
+
+T getTopLevelGenerics<T>(T t) => t;
+
 String _getBarWithSideEffect() {
   var x = 5;
   expect(x, equals(5));
@@ -346,9 +355,19 @@ main() {
       String expected = dartFunction();
       expect(f.a, equals(expected));
 
-      // Using a tearoff as the property value
-      js_util.setProperty(f, 'tearoff', allowInterop(ExampleTearoff().foo));
+      // Using a tearoff as the property value.
+      js_util.setProperty(f, 'tearoff', allowInterop(DartClass().getX));
       expect(js_util.callMethod(f, 'tearoff', []), equals(3));
+
+      // Set property to instance method calls.
+      js_util.setProperty(f, 'a', DartClass().getX());
+      expect(f.a, equals(3));
+      js_util.setProperty(f, 'a', GenericDartClass<int>(5).getT());
+      expect(f.a, equals(5));
+
+      // Set property using a generics wrapper on value.
+      js_util.setProperty(f, 'a', getTopLevelGenerics<int>(10));
+      expect(f.a, equals(10));
     });
   });
 

tests/lib_2/js/js_util/properties_test.dart

@@ -52,11 +52,20 @@ class ExampleTypedLiteral {
   external get b;
 }
 
-class ExampleTearoff {
+class DartClass {
   int x = 3;
-  foo() => x;
+  int getX() => x;
 }
 
+class GenericDartClass<T> {
+  final T myT;
+  GenericDartClass(this.myT);
+
+  T getT() => myT;
+}
+
+T getTopLevelGenerics<T>(T t) => t;
+
 String _getBarWithSideEffect() {
   var x = 5;
   expect(x, equals(5));
@@ -348,9 +357,19 @@ main() {
       String expected = dartFunction();
       expect(f.a, equals(expected));
 
-      // Using a tearoff as the property value
-      js_util.setProperty(f, 'tearoff', allowInterop(ExampleTearoff().foo));
+      // Using a tearoff as the property value.
+      js_util.setProperty(f, 'tearoff', allowInterop(DartClass().getX));
       expect(js_util.callMethod(f, 'tearoff', []), equals(3));
+
+      // Set property to instance method calls.
+      js_util.setProperty(f, 'a', DartClass().getX());
+      expect(f.a, equals(3));
+      js_util.setProperty(f, 'a', GenericDartClass<int>(5).getT());
+      expect(f.a, equals(5));
+
+      // Set property using a generics wrapper on value.
+      js_util.setProperty(f, 'a', getTopLevelGenerics<int>(10));
+      expect(f.a, equals(10));
     });
   });