Skip to content

Commit

Permalink
[dart:js_interop] Allow dart:html types in external signatures
Browse files Browse the repository at this point in the history
Closes #54482

This is generally useful for users working around some limitations of
dart:html. While we want to encourage users to use package:web,
dart:html is not available on dart2wasm and users can use dart:html in
other ways already e.g. in an interop extension type, so it doesn't make
sense to disallow this. We also allow type parameters that extend these
types as well.

In order to make this a bit more performant (subtyping checks may be
expensive), code is refactored to cache more readily and separate the
notion of an allowed representation type vs interop extension type. We
also define the notion of a "core" interop type, which will be useful
when we want to efficiently query what interop type users are using
underneath the possible layers of extension types.

A few missing tests around typed_data are added and the error around
invalid types is reworded to include this change and be more consise.

Change-Id: I256b0cce4355d2a21853b0c5bf641166cafc523e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347224
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Srujan Gaddam <[email protected]>
  • Loading branch information
srujzs authored and Commit Queue committed Jan 25, 2024
1 parent 75b1973 commit f04e42b
Show file tree
Hide file tree
Showing 10 changed files with 257 additions and 193 deletions.
36 changes: 13 additions & 23 deletions pkg/_js_interop_checks/lib/js_interop_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -915,46 +915,36 @@ class JsInteropChecks extends RecursiveVisitor {
return false;
}

/// Return whether [type] can be used on a `dart:js_interop` external member
/// or in the signature of a function that is converted via `toJS`.
bool _isAllowedExternalType(DartType type) {
// TODO(joshualitt): We allow only JS types on external JS interop APIs with
// two exceptions: `void` and `Null`. Both of these exceptions exist largely
// to support passing Dart functions to JS as callbacks. Furthermore, both
// of these types mean no actual values needs to be returned to JS. That
// said, for completeness, we may restrict these two types someday, and
// provide JS types equivalents, but likely only if we have implicit
// conversions between Dart types and JS types.

if (type is VoidType || type is NullType) return true;
if (type is TypeParameterType || type is StructuralParameterType) {
final bound = type.nonTypeVariableBound;
final isStaticInteropBound =
bound is InterfaceType && hasStaticInteropAnnotation(bound.classNode);
final isInteropExtensionTypeBound = bound is ExtensionType &&
_extensionIndex
.isInteropExtensionType(bound.extensionTypeDeclaration);
// If it can be used as a representation type of an interop extension
// type, it is okay to be used as a bound.
// TODO(srujzs): We may want to support type parameters with primitive
// bounds that are themselves allowed e.g. `num`. If so, we should handle
// that change in dart2wasm.
if (isStaticInteropBound || isInteropExtensionTypeBound) {
return true;
}
if (_extensionIndex.isAllowedRepresentationType(bound)) return true;
}
// If it can be used as a representation type of an interop extension type,
// it is okay to be used on an external member.
if (_extensionIndex.isAllowedRepresentationType(type)) return true;
if (type is InterfaceType) {
final cls = type.classNode;
// Primitive types are okay.
if (cls == _coreTypes.boolClass ||
cls == _coreTypes.numClass ||
cls == _coreTypes.doubleClass ||
cls == _coreTypes.intClass ||
cls == _coreTypes.stringClass) {
return true;
}
if (hasStaticInteropAnnotation(cls)) return true;
}
if (type is ExtensionType) {
if (_extensionIndex
.isInteropExtensionType(type.extensionTypeDeclaration)) {
return true;
}
} else if (type is ExtensionType) {
// Extension types that wrap other allowed types are also okay. Interop
// extension types are handled above, so this is essentially for extension
// types on primtives.
return _isAllowedExternalType(type.extensionTypeErasure);
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,18 @@ class JsUtilOptimizer extends Transformer {
final Procedure _setPropertyUncheckedTarget;

/// Dynamic members in js_util that interop allowed.
static final Iterable<String> _allowedInteropJsUtilMembers = <String>[
static const List<String> _allowedInteropJsUtilMembers = [
'callConstructor',
'callMethod',
'getProperty',
'jsify',
'newObject',
'setProperty'
];

final Procedure _allowInteropTarget;
final Iterable<Procedure> _allowedInteropJsUtilTargets;
final Procedure _jsTarget;
final Procedure _allowInteropTarget;
final Procedure _listEmptyFactory;

final CoreTypes _coreTypes;
Expand Down Expand Up @@ -690,7 +691,7 @@ class JsUtilOptimizer extends Transformer {
/// member in question if needed. We only process JS interop extension types and
/// extensions on either JS interop or @Native classes.
class ExtensionIndex {
final CoreTypes _coreTypes;
final Map<Reference, Reference?> _coreInteropTypeIndex = {};
final Map<Reference, Annotatable> _extensionAnnotatableIndex = {};
final Map<Reference, Extension> _extensionIndex = {};
final Map<Reference, ExtensionMemberDescriptor> _extensionMemberIndex = {};
Expand All @@ -699,13 +700,15 @@ class ExtensionIndex {
final Map<Reference, ExtensionTypeMemberDescriptor>
_extensionTypeMemberIndex = {};
final Map<Reference, Reference> _extensionTypeTearOffIndex = {};
final Map<Reference, bool> _interopExtensionTypeIndex = {};
final Class? _javaScriptObject;
final Set<Library> _processedExtensionLibraries = {};
final Set<Library> _processedExtensionTypeLibraries = {};
final Set<Reference> _shouldTrustType = {};
final TypeEnvironment _typeEnvironment;

ExtensionIndex(this._coreTypes, this._typeEnvironment);
ExtensionIndex(CoreTypes coreTypes, this._typeEnvironment)
: _javaScriptObject = coreTypes.index
.tryGetClass('dart:_interceptors', 'JavaScriptObject');

/// If unprocessed, for all extension members in [library] whose on-type is a
/// JS interop or `@Native` class, does the following:
Expand Down Expand Up @@ -786,50 +789,71 @@ class ExtensionIndex {
return _extensionTearOffIndex[member.reference];
}

/// Caches and returns whether the ultimate representation type that
/// corresponds to [extensionType]'s representation type is an interop type
/// that can be statically interoperable.
/// Returns whether [extensionType] is an "interop extension type".
///
/// This currently allows the interface type to be:
/// - all package:js classes
/// - dart:js_interop types
/// - @Native types that implement JavaScriptObject
/// - extension types that wrap any of the above
/// Interop extension types have either another interop extension type or a
/// "core" interop type (see below) as their representation type. Extension
/// types can only declare external JS interop members if they are interop
/// extension types.
bool isInteropExtensionType(ExtensionTypeDeclaration extensionType) {
final reference = extensionType.reference;
if (_interopExtensionTypeIndex.containsKey(reference)) {
return _interopExtensionTypeIndex[reference]!;
}
// Check if this is an dart:js_interop JS type or recursively an extension
// type on one.
DartType repType = ExtensionType(extensionType, Nullability.nonNullable);
while (repType is ExtensionType) {
final declaration = repType.extensionTypeDeclaration;
return getCoreInteropType(
// Nullability is irrelevant for this purpose.
ExtensionType(extensionType, Nullability.undetermined)) != null;
}

/// Returns the "core" interop type of [type], unwrapping extension types as
/// needed and caching along the way.
///
/// A type is a "core" interop type if it is:
/// - a `dart:js_interop` extension type
/// - a `@staticInterop` type
/// - an `@Native` type that <: `JavaScriptObject`. Note that this excludes
/// `dart:typed_data`, as typed list factories return a type that is
/// <: `JavaScriptObject`, but the typed lists themselves are not such a
/// type. This is expected and intended since unlike `dart:html`,
/// `dart:typed_data` can be used in dart2wasm, and since we do not want
/// typed lists to be considered interoperable there, it makes sense to
/// exclude them here.
///
/// If [type] is allowed and is an extension type, it is an interop extension
/// type as well.
///
/// Returns `null` if there is no [type] that neither wraps nor is a "core"
/// interop type.
Reference? getCoreInteropType(DartType type) {
if (type is ExtensionType) {
final declaration = type.extensionTypeDeclaration;
final reference = declaration.reference;
if (_coreInteropTypeIndex.containsKey(reference)) {
return _coreInteropTypeIndex[reference];
}
if (declaration.enclosingLibrary.importUri.toString() ==
'dart:js_interop') {
return true;
return _coreInteropTypeIndex[reference] = reference;
}
repType = declaration.declaredRepresentationType;
}
if (repType is InterfaceType) {
final cls = repType.classNode;
final javaScriptObject = _coreTypes.index
.tryGetClass('dart:_interceptors', 'JavaScriptObject');
// Note that we recurse instead of using the erasure, as JS types are
// extension types.
return _coreInteropTypeIndex[reference] =
getCoreInteropType(declaration.declaredRepresentationType);
} else if (type is InterfaceType) {
final cls = type.classNode;
final reference = cls.reference;
if (hasStaticInteropAnnotation(cls) ||
(javaScriptObject != null &&
(_javaScriptObject != null &&
hasNativeAnnotation(cls) &&
_typeEnvironment.isSubtypeOf(
repType,
InterfaceType(javaScriptObject, Nullability.nullable),
type,
InterfaceType(_javaScriptObject!, Nullability.nullable),
SubtypeCheckMode.withNullabilities))) {
_interopExtensionTypeIndex[reference] = true;
return true;
return _coreInteropTypeIndex[reference] = reference;
}
}
_interopExtensionTypeIndex[reference] = false;
return false;
return null;
}

bool isAllowedRepresentationType(DartType type) =>
getCoreInteropType(type) != null;

/// If unprocessed, for all extension type members in [library] whose
/// extension type is static interop, does the following:
///
Expand Down
9 changes: 5 additions & 4 deletions pkg/front_end/lib/src/fasta/fasta_codes_cfe_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4114,8 +4114,9 @@ const Template<Message Function(DartType _type, bool isNonNullableByDefault)>
Message Function(DartType _type, bool isNonNullableByDefault)>(
"JsInteropStaticInteropExternalTypeViolation",
problemMessageTemplate:
r"""Type '#type' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type.""",
correctionMessageTemplate: r"""Use one of the valid types instead.""",
r"""Type '#type' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.""",
correctionMessageTemplate:
r"""Use one of these valid types instead: JS types from 'dart:js_interop', '@staticInterop' types, 'dart:html' types when compiling to JS, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop or 'dart:html' type.""",
withArguments:
_withArgumentsJsInteropStaticInteropExternalTypeViolation);

Expand All @@ -4134,9 +4135,9 @@ Message _withArgumentsJsInteropStaticInteropExternalTypeViolation(
String type = typeParts.join();
return new Message(codeJsInteropStaticInteropExternalTypeViolation,
problemMessage:
"""Type '${type}' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type.""" +
"""Type '${type}' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.""" +
labeler.originMessages,
correctionMessage: """Use one of the valid types instead.""",
correctionMessage: """Use one of these valid types instead: JS types from 'dart:js_interop', '@staticInterop' types, 'dart:html' types when compiling to JS, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop or 'dart:html' type.""",
arguments: {'type': _type});
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/front_end/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5917,8 +5917,8 @@ JsInteropNonStaticWithStaticInteropSupertype:
correctionMessage: "Try marking '#name' as a `@staticInterop` class, or don't inherit '#name2'."

JsInteropStaticInteropExternalTypeViolation:
problemMessage: "Type '#type' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type."
correctionMessage: "Use one of the valid types instead."
problemMessage: "Type '#type' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'."
correctionMessage: "Use one of these valid types instead: JS types from 'dart:js_interop', '@staticInterop' types, 'dart:html' types when compiling to JS, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop or 'dart:html' type."

JsInteropStaticInteropGenerativeConstructor:
problemMessage: "`@staticInterop` classes should not contain any generative constructors."
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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.

// Like allowed_external_member_type_test, but tests `@Native` types. The only
// valid ones are those that <: `JavaScriptObject` and users can reference,
// which end up only being the types in the SDK web libraries.

@JS()
library allowed_external_member_native_type_test;

import 'dart:html';
import 'dart:js_interop';
import 'dart:svg';
import 'dart:typed_data';

@JS()
external void documentTest(Document _);

@JS()
external void elementTypeParamTest<T extends Element>(T _);

@JS()
external GeometryElement geometryElementTest();

// Not an `@Native` type.
@JS()
external void platformTest(Platform _);
// ^
// [web] Type 'Platform' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

// While the factory returns an `@Native` type that implements
// `JavaScriptObject`, the public interface is not such a type.
@JS()
external void uint8ListTest(Uint8List _);
// ^
// [web] Type 'Uint8List' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

void main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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.

@JS()
library allowed_external_member_type_test;

import 'dart:js_interop';

@JS()
@staticInterop
class JSClass {
external factory JSClass(List<int> baz);
// ^
// [web] Type 'List<int>' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external factory JSClass.other(Object blu);
// ^
// [web] Type 'Object' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external static dynamic foo();
// ^
// [web] Type 'dynamic' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external static Function get fooGet;
// ^
// [web] Type 'Function' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external static set fooSet(void Function() bar);
// ^
// [web] Type 'void Function()' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.
}

extension JSClassExtension on JSClass {
external dynamic extFoo();
// ^
// [web] Type 'dynamic' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external JSClass extFoo2(List<Object?> bar);
// ^
// [web] Type 'List<Object?>' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external Function get extFooGet;
// ^
// [web] Type 'Function' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

external set extFooSet(void Function() bar);
// ^
// [web] Type 'void Function()' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.
}

@JS()
extension type ExtensionType(JSObject _) {}

@JS()
external void jsFunctionTest(JSFunction foo);

@JS()
external void useStaticInteropClass(JSClass foo);

@JS()
external void useStaticInteropExtensionType(ExtensionType foo);

void declareTypeParameter<T extends JSAny?>() {}

T declareAndUseTypeParameter<T extends JSAny?>(T t) => t;

T declareAndUseInvalidTypeParameter<T>(T t) => t;

void main() {
((double foo) => 4.0.toJS).toJS;

((JSNumber foo) => 4.0).toJS;

((List foo) => 4.0).toJS;
// ^
// [web] Type 'List<dynamic>' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

((JSNumber foo) => () {}).toJS;
// ^
// [web] Type 'Null Function()' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

((((JSNumber foo) => 4.0) as dynamic) as Function).toJS;
// ^
// [web] `Function.toJS` requires a statically known function type, but Type 'Function' is not a precise function type, e.g., `void Function()`.

void typeParametersTest<T extends JSAny, U extends ExtensionType,
V extends JSClass, W, Y>() {
((T t) => t).toJS;
((U u) => u).toJS;
((V v) => v).toJS;
((W w) => w as Y).toJS;
// ^
// [web] Type 'W' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.
// [web] Type 'Y' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.

declareTypeParameter.toJS;
// ^
// [web] Functions converted via `toJS` cannot declare type parameters.
declareAndUseTypeParameter.toJS;
// ^
// [web] Functions converted via `toJS` cannot declare type parameters.
declareAndUseInvalidTypeParameter.toJS;
// ^
// [web] Functions converted via `toJS` cannot declare type parameters.
// [web] Type 'T' is not a valid type in the signature of 'dart:js_interop' external APIs or APIs converted via 'toJS'.
}
}
Loading

0 comments on commit f04e42b

Please sign in to comment.