diff --git a/pkg/_js_interop_checks/lib/src/transformations/static_interop_class_eraser.dart b/pkg/_js_interop_checks/lib/src/transformations/static_interop_class_eraser.dart new file mode 100644 index 00000000000..f1caddb8580 --- /dev/null +++ b/pkg/_js_interop_checks/lib/src/transformations/static_interop_class_eraser.dart @@ -0,0 +1,183 @@ +// Copyright (c) 2021, 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. + +import 'package:kernel/ast.dart'; +import 'package:kernel/clone.dart'; +import 'package:kernel/core_types.dart'; +import 'package:kernel/kernel.dart'; +import 'package:kernel/src/replacement_visitor.dart'; +import 'package:_js_interop_checks/src/js_interop.dart'; + +class _TypeSubstitutor extends ReplacementVisitor { + final Class _baseJavaScriptObject; + _TypeSubstitutor(this._baseJavaScriptObject); + + @override + DartType? visitInterfaceType(InterfaceType type, int variance) { + if (hasStaticInteropAnnotation(type.classNode)) { + return InterfaceType(_baseJavaScriptObject, type.declaredNullability); + } + return super.visitInterfaceType(type, variance); + } +} + +/// Erases usage of `@JS` classes that are annotated with `@staticInterop` in +/// favor of `BaseJavaScriptObject`. +class StaticInteropClassEraser extends Transformer { + final Class _baseJavaScriptObject; + final CloneVisitorNotMembers _cloner = CloneVisitorNotMembers(); + late final _TypeSubstitutor _typeSubstitutor; + + StaticInteropClassEraser(CoreTypes coreTypes) + : _baseJavaScriptObject = coreTypes.index + .getClass('dart:_interceptors', 'BaseJavaScriptObject') { + _typeSubstitutor = _TypeSubstitutor(_baseJavaScriptObject); + } + + String _factoryStubName(Procedure factoryTarget) => + '${factoryTarget.name}|staticInteropFactoryStub'; + + /// Either finds or creates a static method stub to replace factories with a + /// body in a static interop class. + /// + /// Modifies [factoryTarget]'s enclosing class to include the new method. + Procedure _findOrCreateFactoryStub(Procedure factoryTarget) { + assert(factoryTarget.isFactory); + var factoryClass = factoryTarget.enclosingClass!; + assert(hasStaticInteropAnnotation(factoryClass)); + var stubName = _factoryStubName(factoryTarget); + var stubs = factoryClass.procedures + .where((procedure) => procedure.name.text == stubName); + if (stubs.isEmpty) { + // Note that the return type of the cloned function is transformed. + var functionNode = + super.visitFunctionNode(_cloner.clone(factoryTarget.function)) + as FunctionNode; + var staticMethod = Procedure( + Name(stubName), ProcedureKind.Method, functionNode, + isStatic: true, fileUri: factoryTarget.fileUri) + ..parent = factoryClass; + factoryClass.procedures.add(staticMethod); + return staticMethod; + } else { + assert(stubs.length == 1); + return stubs.first; + } + } + + @override + TreeNode visitConstructor(Constructor node) { + if (hasStaticInteropAnnotation(node.enclosingClass)) { + // Transform children of the constructor node excluding the return type. + var returnType = node.function.returnType; + var newConstructor = super.visitConstructor(node) as Constructor; + newConstructor.function.returnType = returnType; + return newConstructor; + } + return super.visitConstructor(node); + } + + @override + TreeNode visitProcedure(Procedure node) { + // Avoid changing the return types of factories, but rather cast the type of + // the invocation. + if (node.isFactory && hasStaticInteropAnnotation(node.enclosingClass!)) { + if (node.function.body != null && !node.isRedirectingFactory) { + // Bodies of factories may undergo transformation, which may result in + // type invariants breaking. For a motivating example, consider: + // + // ``` + // factory Foo.fact() => Foo.cons(); + // ``` + // + // The invocation of `cons` would have its type erased, but then it + // would no longer match the return type of `fact`, whose return type + // shouldn't get erased as it is a factory. Note that this is only an + // issue when the factory has a body that doesn't simply redirect. + // + // In order to circumvent this, we introduce a new static method that + // clones the factory body and has a return type of + // `BaseJavaScriptObject`. Invocations of the factory are turned into + // invocations of the static method. The original factory is still kept + // in order to make modular compilations work. + _findOrCreateFactoryStub(node); + return node; + } else { + // Transform children of the factory node excluding the return type and + // return type of the signature type. + var returnType = node.function.returnType; + var signatureReturnType = node.signatureType?.returnType; + var newProcedure = super.visitProcedure(node) as Procedure; + newProcedure.function.returnType = returnType; + var signatureType = newProcedure.signatureType; + if (signatureType != null && signatureReturnType != null) { + newProcedure.signatureType = FunctionType( + signatureType.positionalParameters, + signatureReturnType, + signatureType.declaredNullability, + namedParameters: signatureType.namedParameters, + typeParameters: signatureType.typeParameters, + requiredParameterCount: signatureType.requiredParameterCount, + typedefType: signatureType.typedefType); + } + return newProcedure; + } + } + return super.visitProcedure(node); + } + + @override + TreeNode visitConstructorInvocation(ConstructorInvocation node) { + if (hasStaticInteropAnnotation(node.target.enclosingClass)) { + // Add a cast so that the result gets typed as `BaseJavaScriptObject`. + var newInvocation = super.visitConstructorInvocation(node) as Expression; + return AsExpression( + newInvocation, + InterfaceType(_baseJavaScriptObject, + node.target.function.returnType.declaredNullability)) + ..fileOffset = newInvocation.fileOffset; + } + return super.visitConstructorInvocation(node); + } + + /// Transform static invocations that correspond only to factories of static + /// interop classes. + @override + TreeNode visitStaticInvocation(StaticInvocation node) { + var targetClass = node.target.enclosingClass; + if (node.target.isFactory && + targetClass != null && + hasStaticInteropAnnotation(targetClass)) { + var factoryTarget = node.target; + if (factoryTarget.function.body != null && + !factoryTarget.isRedirectingFactory) { + // Use or create the static method that replaces this factory instead. + // Note that the static method will not have been created yet in the + // case where we visit the factory later. Also note that a cast is not + // needed since the static method already has its type erased. + var args = super.visitArguments(node.arguments) as Arguments; + return StaticInvocation(_findOrCreateFactoryStub(factoryTarget), args, + isConst: node.isConst) + ..fileOffset = node.fileOffset; + } else { + // Add a cast so that the result gets typed as `BaseJavaScriptObject`. + var newInvocation = super.visitStaticInvocation(node) as Expression; + return AsExpression( + newInvocation, + InterfaceType(_baseJavaScriptObject, + node.target.function.returnType.declaredNullability)) + ..fileOffset = newInvocation.fileOffset; + } + } + return super.visitStaticInvocation(node); + } + + @override + DartType visitDartType(DartType type) { + // Variance is not a factor in our type transformation here, so just choose + // `unrelated` as a default. + var substitutedType = type.accept1(_typeSubstitutor, Variance.unrelated); + return substitutedType != null ? substitutedType : type; + } +} diff --git a/pkg/compiler/lib/src/kernel/dart2js_target.dart b/pkg/compiler/lib/src/kernel/dart2js_target.dart index d8985b78655..01f42be21c9 100644 --- a/pkg/compiler/lib/src/kernel/dart2js_target.dart +++ b/pkg/compiler/lib/src/kernel/dart2js_target.dart @@ -12,6 +12,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart' show Message, LocatedMessage; import 'package:_js_interop_checks/js_interop_checks.dart'; import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'; +import 'package:_js_interop_checks/src/transformations/static_interop_class_eraser.dart'; import 'package:kernel/ast.dart' as ir; import 'package:kernel/class_hierarchy.dart'; import 'package:kernel/core_types.dart'; @@ -142,6 +143,7 @@ class Dart2jsTarget extends Target { ChangedStructureNotifier? changedStructureNotifier}) { var nativeClasses = JsInteropChecks.getNativeClasses(component); var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy); + var staticInteropClassEraser = StaticInteropClassEraser(coreTypes); for (var library in libraries) { JsInteropChecks( coreTypes, @@ -151,6 +153,7 @@ class Dart2jsTarget extends Target { // TODO (rileyporter): Merge js_util optimizations with other lowerings // in the single pass in `transformations/lowering.dart`. jsUtilOptimizer.visitLibrary(library); + staticInteropClassEraser.visitLibrary(library); } lowering.transformLibraries(libraries, coreTypes, hierarchy, options); logger?.call("Lowering transformations performed"); diff --git a/pkg/dev_compiler/lib/src/kernel/target.dart b/pkg/dev_compiler/lib/src/kernel/target.dart index 7985eb637be..bb79eb83e96 100644 --- a/pkg/dev_compiler/lib/src/kernel/target.dart +++ b/pkg/dev_compiler/lib/src/kernel/target.dart @@ -8,6 +8,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart' show Message, LocatedMessage; import 'package:_js_interop_checks/js_interop_checks.dart'; import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart'; +import 'package:_js_interop_checks/src/transformations/static_interop_class_eraser.dart'; import 'package:kernel/class_hierarchy.dart'; import 'package:kernel/core_types.dart'; import 'package:kernel/kernel.dart'; @@ -188,6 +189,7 @@ class DevCompilerTarget extends Target { ChangedStructureNotifier? changedStructureNotifier}) { _nativeClasses ??= JsInteropChecks.getNativeClasses(component); var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy); + var staticInteropClassEraser = StaticInteropClassEraser(coreTypes); for (var library in libraries) { _CovarianceTransformer(library).transform(); JsInteropChecks( @@ -196,6 +198,7 @@ class DevCompilerTarget extends Target { _nativeClasses!) .visitLibrary(library); jsUtilOptimizer.visitLibrary(library); + staticInteropClassEraser.visitLibrary(library); } } diff --git a/tests/modular/static_interop_erasure/.packages b/tests/modular/static_interop_erasure/.packages new file mode 100644 index 00000000000..fce126f821d --- /dev/null +++ b/tests/modular/static_interop_erasure/.packages @@ -0,0 +1 @@ +js:../../../pkg/js/lib diff --git a/tests/modular/static_interop_erasure/main.dart b/tests/modular/static_interop_erasure/main.dart new file mode 100644 index 00000000000..80ccc3b7c7f --- /dev/null +++ b/tests/modular/static_interop_erasure/main.dart @@ -0,0 +1,10 @@ +// Copyright (c) 2021, 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. + +import 'static_interop.dart'; + +void main() { + setUp(); + var staticJs = StaticJSClass.factory(); +} diff --git a/tests/modular/static_interop_erasure/modules.yaml b/tests/modular/static_interop_erasure/modules.yaml new file mode 100644 index 00000000000..6d1a98874a2 --- /dev/null +++ b/tests/modular/static_interop_erasure/modules.yaml @@ -0,0 +1,9 @@ +# Copyright (c) 2021, 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. +# +# Test that modular compilation still works with static interop classes that +# contain factories. +dependencies: + main: static_interop + static_interop: js diff --git a/tests/modular/static_interop_erasure/static_interop.dart b/tests/modular/static_interop_erasure/static_interop.dart new file mode 100644 index 00000000000..a5c93c43bbc --- /dev/null +++ b/tests/modular/static_interop_erasure/static_interop.dart @@ -0,0 +1,24 @@ +// Copyright (c) 2021, 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 static_interop; + +import 'package:js/js.dart'; + +@JS() +external void eval(String code); + +@JS('JSClass') +@staticInterop +class StaticJSClass { + external StaticJSClass(); + factory StaticJSClass.factory() { + return StaticJSClass(); + } +} + +void setUp() { + eval('''function JSClass() {}'''); +} diff --git a/tests/web/native/static_interop_erasure/factory_stub_test.dart b/tests/web/native/static_interop_erasure/factory_stub_test.dart new file mode 100644 index 00000000000..71fe51681dc --- /dev/null +++ b/tests/web/native/static_interop_erasure/factory_stub_test.dart @@ -0,0 +1,73 @@ +// Copyright (c) 2021, 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. + +// Test that factory methods with bodies are stubbed correctly in static interop +// type erasure. + +@JS() +library factory_stub_test; + +import 'dart:_interceptors' show JavaScriptObject; + +import 'package:js/js.dart'; + +import '../native_testing.dart'; +import '../native_testing.dart' as native_testing; + +NativeClass makeNativeClass() native; + +@Native('NativeClass') +class NativeClass extends JavaScriptObject { + factory NativeClass() => makeNativeClass(); +} + +@JS('NativeClass') +@staticInterop +class StaticNativeClass { + external StaticNativeClass(); + factory StaticNativeClass.redirectingFactory() = StaticNativeClass; + factory StaticNativeClass.simpleFactory() => StaticNativeClass(); + factory StaticNativeClass.factoryWithParam( + StaticNativeClass staticNativeClass) => + staticNativeClass; + // This and `StaticNativeClassCopy.nestedFactory` exist to ensure that we + // cover the case where invocations on factories are visible before their + // declarations in the AST. This will test whether we correctly create the + // stub even if we haven't visited the declaration yet. + factory StaticNativeClass.nestedFactory() { + StaticNativeClassCopy.nestedFactory(); + return StaticNativeClass(); + } +} + +@JS('NativeClass') +@staticInterop +class StaticNativeClassCopy { + external StaticNativeClassCopy(); + factory StaticNativeClassCopy.nestedFactory() { + StaticNativeClass.simpleFactory(); + return StaticNativeClassCopy(); + } +} + +void main() { + nativeTesting(); + native_testing.JS('', r''' + (function(){ + function NativeClass() {} + self.NativeClass = NativeClass; + self.makeNativeClass = function(){return new NativeClass()}; + self.nativeConstructor(NativeClass); + })() + '''); + applyTestExtensions(['NativeClass']); + + // Make the Native class live. + NativeClass(); + // Invoke factories and ensure they're typed correctly through assignment. + StaticNativeClass staticNativeClass = StaticNativeClass.redirectingFactory(); + staticNativeClass = StaticNativeClass.simpleFactory(); + staticNativeClass = StaticNativeClass.factoryWithParam(staticNativeClass); + staticNativeClass = StaticNativeClass.nestedFactory(); +} diff --git a/tests/web_2/native/static_interop_erasure/factory_stub_test.dart b/tests/web_2/native/static_interop_erasure/factory_stub_test.dart new file mode 100644 index 00000000000..8aba8f48908 --- /dev/null +++ b/tests/web_2/native/static_interop_erasure/factory_stub_test.dart @@ -0,0 +1,75 @@ +// Copyright (c) 2021, 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. + +// @dart = 2.7 + +// Test that factory methods with bodies are stubbed correctly in static interop +// type erasure. + +@JS() +library factory_stub_test; + +import 'dart:_interceptors' show JavaScriptObject; + +import 'package:js/js.dart'; + +import '../native_testing.dart'; +import '../native_testing.dart' as native_testing; + +NativeClass makeNativeClass() native; + +@Native('NativeClass') +class NativeClass extends JavaScriptObject { + factory NativeClass() => makeNativeClass(); +} + +@JS('NativeClass') +@staticInterop +class StaticNativeClass { + external StaticNativeClass(); + factory StaticNativeClass.redirectingFactory() = StaticNativeClass; + factory StaticNativeClass.simpleFactory() => StaticNativeClass(); + factory StaticNativeClass.factoryWithParam( + StaticNativeClass staticNativeClass) => + staticNativeClass; + // This and `StaticNativeClassCopy.nestedFactory` exist to ensure that we + // cover the case where invocations on factories are visible before their + // declarations in the AST. This will test whether we correctly create the + // stub even if we haven't visited the declaration yet. + factory StaticNativeClass.nestedFactory() { + StaticNativeClassCopy.nestedFactory(); + return StaticNativeClass(); + } +} + +@JS('NativeClass') +@staticInterop +class StaticNativeClassCopy { + external StaticNativeClassCopy(); + factory StaticNativeClassCopy.nestedFactory() { + StaticNativeClass.simpleFactory(); + return StaticNativeClassCopy(); + } +} + +void main() { + nativeTesting(); + native_testing.JS('', r''' + (function(){ + function NativeClass() {} + self.NativeClass = NativeClass; + self.makeNativeClass = function(){return new NativeClass()}; + self.nativeConstructor(NativeClass); + })() + '''); + applyTestExtensions(['NativeClass']); + + // Make the Native class live. + NativeClass(); + // Invoke factories and ensure they're typed correctly through assignment. + StaticNativeClass staticNativeClass = StaticNativeClass.redirectingFactory(); + staticNativeClass = StaticNativeClass.simpleFactory(); + staticNativeClass = StaticNativeClass.factoryWithParam(staticNativeClass); + staticNativeClass = StaticNativeClass.nestedFactory(); +}