[ddc] Refactor visitFunctionTearOff

Stop passing `null` as the member parameter of `_emitPropertyGet`.

Avoids the use of one method with multiple code paths and returns
to handle any situation because it becomes very hard to reason
about what original source code leads to each path.

Issue: #54463
Change-Id: I1255817c7cf76578b256650789f6530b456b2f03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357207
Reviewed-by: Srujan Gaddam <[email protected]>
Reviewed-by: Sigmund Cherem <[email protected]>
Reviewed-by: Bob Nystrom <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>

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

@@ -4896,6 +4896,13 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         node.receiver, node.interfaceTarget, node.name.text);
   }
 
+  /// Returns `true` when [member] is a `.call` member (field, getter or method)
+  /// of a non-static JavaScript Interop class.
+  bool _isNonStaticJsInteropCallMember(Member? member) =>
+      member != null &&
+      member.name.text == 'call' &&
+      isNonStaticJsInterop(member);
+
   @override
   js_ast.Expression visitDynamicSet(DynamicSet node) {
     return _emitPropertySet(node.receiver, null, node.value, node.name.text);
@@ -4958,16 +4965,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   // TODO(54463): Refactor and specialize this code for each type of 'get'.
   js_ast.Expression _emitPropertyGet(
       Expression receiver, Member? member, String memberName) {
-    // TODO(jmesserly): should tearoff of `.call` on a function type be
-    // encoded as a different node, or possibly eliminated?
-    // (Regardless, we'll still need to handle the callable JS interop classes.)
-    if (memberName == 'call' &&
-        _isDirectCallable(receiver.getStaticType(_staticTypeContext))) {
-      // Tearoff of `call` on a function type is a no-op;
-      return _visitExpression(receiver);
+    var jsReceiver = _visitExpression(receiver);
+    if (_isNonStaticJsInteropCallMember(member)) {
+      // Historically DDC has treated this as a "callable class" and the
+      // tearoff of `.call` as a no-op. This is still needed here to preserve
+      // the existing behavior for the non-static JavaScript interop (including
+      // potentially failing cases).
+      return jsReceiver;
     }
     var jsName = _emitMemberName(memberName, member: member);
-    var jsReceiver = _visitExpression(receiver);
 
     // TODO(jmesserly): we need to mark an end span for property accessors so
     // they can be hovered. Unfortunately this is not possible as Kernel does
@@ -7391,7 +7397,22 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
   @override
   js_ast.Expression visitFunctionTearOff(FunctionTearOff node) {
-    return _emitPropertyGet(node.receiver, null, 'call');
+    var receiver = node.receiver;
+    var receiverType = receiver.getStaticType(_staticTypeContext);
+    var jsReceiver = _visitExpression(receiver);
+    if (receiverType is InterfaceType &&
+        receiverType.classNode == _coreTypes.functionClass) {
+      // Historically DDC has treated this case as a dynamic get and allowed it
+      // to evaluate at runtime.
+      //
+      // This is here to preserve the existing behavior for the non-static
+      // JavaScript interop (including some failing cases) but could potentially
+      // be cleaned up as a breaking change.
+      return runtimeCall(
+          'dload$_replSuffix(#, #)', [jsReceiver, js.string('call')]);
+    }
+    // Otherwise, tearoff of `call` on a function type is a no-op.
+    return jsReceiver;
   }
 
   /// Creates header comments with helpful compilation information.

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

@@ -2,6 +2,7 @@
 // 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.
 
+import 'package:_js_interop_checks/src/js_interop.dart';
 import 'package:kernel/kernel.dart';
 
 import 'kernel_helpers.dart';
@@ -158,3 +159,33 @@ bool usesJSInterop(NamedNode n) {
   }
   return false;
 }
+
+/// Returns `true` when [member] is a JavaScript interop member using the
+/// package:js style of interop.
+///
+/// Will return `false` when the newer static interop style is used or when
+/// [member] is not a JavaScript interop member at all.
+bool isNonStaticJsInterop(Member member) {
+  return member.isExternal &&
+      !member.isExtensionMember &&
+      !member.isExtensionTypeMember &&
+      _usesNonStaticInteropAnnotation(member);
+}
+
+/// Returns `true` when [member] is a JavaScript interop member using the
+/// package:js style of interop annotation.
+///
+/// This does not mean [member] has the annotation directly because it can
+/// be applied on the class or the library level.
+///
+/// Will return `false` when the newer static interop style annotation is
+/// found or when [member] is not a JavaScript interop member at all.
+bool _usesNonStaticInteropAnnotation(Member member) {
+  var enclosingClass = member.enclosingClass;
+  if (enclosingClass != null) {
+    return hasPackageJSAnnotation(enclosingClass) &&
+        !hasStaticInteropAnnotation(enclosingClass);
+  }
+  return hasPackageJSAnnotation(member) ||
+      hasPackageJSAnnotation(member.enclosingLibrary);
+}