[ddc] Unify pkg:js types and allow subtyping between them

Removes the lazy loading of the underlying type in LazyJSTypes.
As such, this removes the need to keep AnonymousJSType and
LazyJSType separate, and is therefore refactored to
PackageJSType. Similarly, subtyping is fixed such that
PackageJSTypes are all subtypes of each other.

Change-Id: If489defdbeb5cb932db802a7d146ad2fc393b12c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207982
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

CHANGELOG.md

@@ -264,6 +264,16 @@ Updated the Linter to `1.8.0`, which includes changes that
 [#46545]: https://github.com/dart-lang/sdk/issues/46545
 [1]: https://dart.dev/faq#q-what-browsers-do-you-support-as-javascript-compilation-targets
 
+#### Dart Dev Compiler (DDC)
+
+- **Breaking Change** [#44154][]: Subtyping relations of `package:js` classes
+  have been changed to be more correct and consistent with Dart2JS.
+  Like `anonymous` classes, non-`anonymous` classes will no longer check the
+  underlying type in DDC. The internal type representation of these objects have
+  changed as well, which will affect the `toString` value of these types.
+
+[#44154]: https://github.com/dart-lang/sdk/issues/44154
+
 ## 2.13.4 - 2021-06-28
 
 This is a patch release that fixes:

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

@@ -1677,7 +1677,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       ctorFields = ctor.initializers
           .map((c) => c is FieldInitializer ? c.field : null)
           .toSet()
-            ..remove(null);
+        ..remove(null);
     }
 
     var body = <js_ast.Statement>[];
@@ -2884,27 +2884,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     js_ast.Expression typeRep;
 
     // Type parameters don't matter as JS interop types cannot be reified.
-    // We have to use lazy JS types because until we have proper module
-    // loading for JS libraries bundled with Dart libraries, we will sometimes
-    // need to load Dart libraries before the corresponding JS libraries are
-    // actually loaded.
-    // Given a JS type such as:
-    //     @JS('google.maps.Location')
-    //     class Location { ... }
-    // We can't emit a reference to MyType because the JS library that defines
-    // it may be loaded after our code. So for now, we use a special lazy type
-    // object to represent MyType.
-    // Anonymous JS types do not have a corresponding concrete JS type so we
-    // have to use a helper to define them.
-    if (isJSAnonymousType(c)) {
-      typeRep = runtimeCall(
-          'anonymousJSType(#)', [js.escapedString(getLocalClassName(c))]);
-    } else {
-      var jsName = _emitJsNameWithoutGlobal(c);
-      if (jsName != null) {
-        typeRep = runtimeCall('lazyJSType(() => #, #)',
-            [_emitJSInteropForGlobal(jsName), js.escapedString(jsName)]);
-      }
+    // package:js types fall under either named or anonymous types. Named types
+    // are used to correspond to JS types that exist, but we do not use the
+    // underlying type for type checks, so they operate virtually the same as
+    // anonymous types. We represent package:js types with a corresponding type
+    // object.
+    var jsName = isJSAnonymousType(c) ?
+        getLocalClassName(c) : _emitJsNameWithoutGlobal(c);
+    if (jsName != null) {
+      typeRep = runtimeCall('packageJSType(#)', [js.escapedString(jsName)]);
     }
 
     if (typeRep != null) {

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart

@@ -181,8 +181,8 @@ final List<Object> _cacheMaps = JS('!', '[]');
 /// A list of functions to reset static fields back to their uninitialized
 /// state.
 ///
-/// This is populated by [defineLazyField] and [LazyJSType] and only contains
-/// fields that have been initialized.
+/// This is populated by [defineLazyField] and only contains fields that have
+/// been initialized.
 @notNull
 final List<void Function()> _resetFields = JS('', '[]');
 

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart

@@ -81,6 +81,15 @@ void nativeNonNullAsserts(bool enable) {
 
 final metadata = JS('', 'Symbol("metadata")');
 
+/// A javascript Symbol used to store a canonical version of T? on T.
+final _cachedNullable = JS('', 'Symbol("cachedNullable")');
+
+/// A javascript Symbol used to store a canonical version of T* on T.
+final _cachedLegacy = JS('', 'Symbol("cachedLegacy")');
+
+/// A javascript Symbol used to store prior subtype checks and their results.
+final _subtypeCache = JS('', 'Symbol("_subtypeCache")');
+
 /// Types in dart are represented internally at runtime as follows.
 ///
 ///   - Normal nominal types, produced from classes, are represented
@@ -197,67 +206,17 @@ F tearoffInterop<F extends Function?>(F f) {
   return JS('', '#', ret);
 }
 
-/// The Dart type that represents a JavaScript class(/constructor) type.
-///
-/// The JavaScript type may not exist, either because it's not loaded yet, or
-/// because it's not available (such as with mocks). To handle this gracefully,
-/// we disable type checks for in these cases, and allow any JS object to work
-/// as if it were an instance of this JS type.
-class LazyJSType extends DartType {
-  Function() _getRawJSTypeFn;
-  @notNull
-  final String _dartName;
-  Object? _rawJSType;
-
-  LazyJSType(this._getRawJSTypeFn, this._dartName);
-
-  toString() {
-    var raw = _getRawJSType();
-    return raw != null ? typeName(raw) : "JSObject<$_dartName>";
-  }
-
-  Object? _getRawJSType() {
-    var raw = _rawJSType;
-    if (raw != null) return raw;
-
-    // Try to evaluate the JS type. If this fails for any reason, we'll try
-    // again next time.
-    // TODO(jmesserly): is it worth trying again? It may create unnecessary
-    // overhead, especially if exceptions are being thrown. Also it means the
-    // behavior of a given type check can change later on.
-    try {
-      raw = _getRawJSTypeFn();
-    } catch (e) {}
-
-    if (raw == null) {
-      _warn('Cannot find native JavaScript type ($_dartName) for type check');
-    } else {
-      _rawJSType = raw;
-      JS('', '#.push(() => # = null)', _resetFields, _rawJSType);
-    }
-    return raw;
-  }
-
-  Object rawJSTypeForCheck() => _getRawJSType() ?? typeRep<JavaScriptObject>();
-
-  @notNull
-  @JSExportName('is')
-  bool is_T(obj) =>
-      obj != null &&
-      (_isJsObject(obj) || isSubtypeOf(getReifiedType(obj), this));
-
-  @JSExportName('as')
-  as_T(obj) => is_T(obj) ? obj : castError(obj, this);
-}
-
-/// An anonymous JS type
+/// Dart type that represents a package:js class type (either anonymous or not).
 ///
 /// For the purposes of subtype checks, these match any JS type.
-class AnonymousJSType extends DartType {
+class PackageJSType extends DartType {
   final String _dartName;
-  AnonymousJSType(this._dartName);
-  toString() => _dartName;
+  PackageJSType(this._dartName);
 
+  @override
+  String toString() => _dartName;
+
+  @notNull
   @JSExportName('is')
   bool is_T(obj) =>
       obj != null &&
@@ -299,35 +258,25 @@ void _nullWarnOnType(type) {
   }
 }
 
-var _lazyJSTypes = JS<Object>('', 'new Map()');
-var _anonymousJSTypes = JS<Object>('', 'new Map()');
+var _packageJSTypes = JS<Object>('', 'new Map()');
 
-lazyJSType(Function() getJSTypeCallback, String name) {
-  var ret = JS('', '#.get(#)', _lazyJSTypes, name);
+packageJSType(String name) {
+  var ret = JS('', '#.get(#)', _packageJSTypes, name);
   if (ret == null) {
-    ret = LazyJSType(getJSTypeCallback, name);
-    JS('', '#.set(#, #)', _lazyJSTypes, name, ret);
+    ret = PackageJSType(name);
+    JS('', '#.set(#, #)', _packageJSTypes, name, ret);
   }
   return ret;
 }
 
-anonymousJSType(String name) {
-  var ret = JS('', '#.get(#)', _anonymousJSTypes, name);
-  if (ret == null) {
-    ret = AnonymousJSType(name);
-    JS('', '#.set(#, #)', _anonymousJSTypes, name, ret);
-  }
-  return ret;
-}
-
-/// A javascript Symbol used to store a canonical version of T? on T.
-final _cachedNullable = JS('', 'Symbol("cachedNullable")');
-
-/// A javascript Symbol used to store a canonical version of T* on T.
-final _cachedLegacy = JS('', 'Symbol("cachedLegacy")');
-
-/// A javascript Symbol used to store prior subtype checks and their results.
-final _subtypeCache = JS('', 'Symbol("_subtypeCache")');
+/// Since package:js types are all subtypes of each other, we use this var to
+/// denote *some* package:js type in our subtyping logic.
+///
+/// Used only when a concrete PackageJSType is not available i.e. when neither
+/// the object nor the target type is a PackageJSType. Avoids initializating a
+/// new PackageJSType every time. Note that we don't add it to the set of JS
+/// types, since it's not an actual JS class.
+final _pkgJSTypeForSubtyping = PackageJSType('');
 
 /// Returns a nullable (question, ?) version of [type].
 ///
@@ -1522,34 +1471,20 @@ bool _isSubtype(t1, t2, @notNull bool strictMode) => JS<bool>('!', '''(() => {
     //
     // JavaScriptObject <: package:js types
     // package:js types <: JavaScriptObject
-    //
-    // TODO: This doesn't allow package:js type <: another package:js type yet.
 
     if (${_isInterfaceSubtype(t1, JavaScriptObject, strictMode)}
-        && (${_jsInstanceOf(t2, LazyJSType)}
-            || ${_jsInstanceOf(t2, AnonymousJSType)}
-            // TODO: Since package:js types are instances of LazyJSType and
-            // AnonymousJSType, we don't have a mechanism to determine if *some*
-            // package:js type implements t2. This will possibly require keeping
-            // a map of these relationships for this subtyping check. For now,
-            // don't execute the following checks.
-            // || _isInterfaceSubtype(LazyJSType, t2, strictMode)
-            // || _isInterfaceSubtype(AnonymousJSType, t2, strictMode)
-            )) {
+        &&
+            // TODO: Since package:js types are instances of PackageJSType and
+            // we don't have a mechanism to determine if *some* package:js type
+            // implements t2. This will possibly require keeping a map of these
+            // relationships for this subtyping check. For now, this will only
+            // work if t2 is also a PackageJSType.
+            ${_isInterfaceSubtype(_pkgJSTypeForSubtyping, t2, strictMode)}) {
       return true;
     }
 
     if (${_isInterfaceSubtype(JavaScriptObject, t2, strictMode)}
-        && (${_jsInstanceOf(t1, LazyJSType)}
-            || ${_jsInstanceOf(t1, AnonymousJSType)}
-            // TODO: We don't have a check in `isInterfaceSubtype` to check that
-            // a class is a subtype of *some* package:js class. This will likely
-            // require modifying `_isInterfaceSubtype` to check if the
-            // interfaces of t1 include a package:js type. For now, don't
-            // execute the following checks.
-            // || _isInterfaceSubtype(t1, LazyJSType, strictMode)
-            // || _isInterfaceSubtype(t1, AnonymousJSType, strictMode)
-            )) {
+        && ${_isInterfaceSubtype(t1, _pkgJSTypeForSubtyping, strictMode)}) {
       return true;
     }
 
@@ -1617,10 +1552,11 @@ bool _isSubtype(t1, t2, @notNull bool strictMode) => JS<bool>('!', '''(() => {
 })()''');
 
 bool _isInterfaceSubtype(t1, t2, @notNull bool strictMode) => JS('', '''(() => {
-  // If we have lazy JS types, unwrap them.  This will effectively
-  // reduce to a prototype check below.
-  if (${_jsInstanceOf(t1, LazyJSType)}) $t1 = $t1.rawJSTypeForCheck();
-  if (${_jsInstanceOf(t2, LazyJSType)}) $t2 = $t2.rawJSTypeForCheck();
+  // Instances of PackageJSType are all subtypes of each other.
+  if (${_jsInstanceOf(t1, PackageJSType)}
+      && ${_jsInstanceOf(t2, PackageJSType)}) {
+    return true;
+  }
 
   if ($t1 === $t2) {
     return true;

tests/dartdevc/debugger/debugger_test_golden.txt

@@ -6519,7 +6519,7 @@ Value:
     {
         "style": "background-color: #d9edf7;color: black"
     },
-    "Instance of 'TestGenericClass<JSObject<ExampleJSClass>, int>'"
+    "Instance of 'TestGenericClass<ExampleJSClass, int>'"
 ]
 -----------------------------------
 Test: TestGenericClassJSInterop instance body
@@ -6616,7 +6616,7 @@ Value:
     {
         "style": "background-color: #d9edf7;color: black"
     },
-    "TestGenericClass<JSObject<ExampleJSClass>, int>"
+    "TestGenericClass<ExampleJSClass, int>"
 ]
 -----------------------------------
 Test: TestGenericClassJSInterop definition formatting body

tests/dartdevc_2/debugger/debugger_test_golden.txt

@@ -6519,7 +6519,7 @@ Value:
     {
         "style": "background-color: #d9edf7;color: black"
     },
-    "Instance of 'TestGenericClass<JSObject<ExampleJSClass>, int>'"
+    "Instance of 'TestGenericClass<ExampleJSClass, int>'"
 ]
 -----------------------------------
 Test: TestGenericClassJSInterop instance body
@@ -6616,7 +6616,7 @@ Value:
     {
         "style": "background-color: #d9edf7;color: black"
     },
-    "TestGenericClass<JSObject<ExampleJSClass>, int>"
+    "TestGenericClass<ExampleJSClass, int>"
 ]
 -----------------------------------
 Test: TestGenericClassJSInterop definition formatting body

tests/lib/html/js_typed_interop_lazy_test.dart

@@ -142,10 +142,9 @@ baz.LazyClass = function LazyClass(a) {
       expect(new Object() is! AnonClass2, isTrue);
 
       expect(<AnonClass>[] is List<AnonClass>, isTrue);
-      // TODO(jacobr): why doesn't this test pass?
-      // expect(<AnonClass>[] is List<AnonClass2>, isTrue);
+      expect(<AnonClass>[] is List<AnonClass2>, isTrue);
       expect(<int>[] is! List<AnonClass>, isTrue);
-      expect(<AnonClass>[] is! List<LazyClass>, isTrue); //# 01: ok
+      expect(<AnonClass>[] is List<LazyClass>, isTrue);
       expect(<int>[] is! List<LazyClass>, isTrue);
       expect(<LazyClass>[] is List<LazyClass>, isTrue);
 
@@ -234,10 +233,9 @@ baz.foo.NestedLazyClass = function NestedLazyClass(a) {
       expect(new Object() is! AnonClass2, isTrue);
 
       expect(<AnonClass>[] is List<AnonClass>, isTrue);
-      // TODO(jacobr): why doesn't this test pass?
-      // expect(<AnonClass>[] is List<AnonClass2>, isTrue);
+      expect(<AnonClass>[] is List<AnonClass2>, isTrue);
       expect(<int>[] is! List<AnonClass>, isTrue);
-      expect(<AnonClass>[] is! List<NestedLazyClass>, isTrue); //# 01: ok
+      expect(<AnonClass>[] is List<NestedLazyClass>, isTrue);
       expect(<int>[] is! List<NestedLazyClass>, isTrue);
       expect(<NestedLazyClass>[] is List<NestedLazyClass>, isTrue);
 

tests/lib_2/html/js_typed_interop_lazy_test.dart

@@ -144,10 +144,9 @@ baz.LazyClass = function LazyClass(a) {
       expect(new Object() is! AnonClass2, isTrue);
 
       expect(<AnonClass>[] is List<AnonClass>, isTrue);
-      // TODO(jacobr): why doesn't this test pass?
-      // expect(<AnonClass>[] is List<AnonClass2>, isTrue);
+      expect(<AnonClass>[] is List<AnonClass2>, isTrue);
       expect(<int>[] is! List<AnonClass>, isTrue);
-      expect(<AnonClass>[] is! List<LazyClass>, isTrue); //# 01: ok
+      expect(<AnonClass>[] is List<LazyClass>, isTrue);
       expect(<int>[] is! List<LazyClass>, isTrue);
       expect(<LazyClass>[] is List<LazyClass>, isTrue);
 
@@ -236,10 +235,9 @@ baz.foo.NestedLazyClass = function NestedLazyClass(a) {
       expect(new Object() is! AnonClass2, isTrue);
 
       expect(<AnonClass>[] is List<AnonClass>, isTrue);
-      // TODO(jacobr): why doesn't this test pass?
-      // expect(<AnonClass>[] is List<AnonClass2>, isTrue);
+      expect(<AnonClass>[] is List<AnonClass2>, isTrue);
       expect(<int>[] is! List<AnonClass>, isTrue);
-      expect(<AnonClass>[] is! List<NestedLazyClass>, isTrue); //# 01: ok
+      expect(<AnonClass>[] is List<NestedLazyClass>, isTrue);
       expect(<int>[] is! List<NestedLazyClass>, isTrue);
       expect(<NestedLazyClass>[] is List<NestedLazyClass>, isTrue);