diff --git a/pkg/compiler/lib/src/commandline_options.dart b/pkg/compiler/lib/src/commandline_options.dart index 0c2fe1746830..4cdb791b986a 100644 --- a/pkg/compiler/lib/src/commandline_options.dart +++ b/pkg/compiler/lib/src/commandline_options.dart @@ -89,6 +89,9 @@ class Flags { static const String nativeNullAssertions = '--native-null-assertions'; static const String noNativeNullAssertions = '--no-native-null-assertions'; + static const String interopNullAssertions = '--interop-null-assertions'; + static const String noInteropNullAssertions = '--no-interop-null-assertions'; + static const String noSourceMaps = '--no-source-maps'; static const String omitLateNames = '--omit-late-names'; diff --git a/pkg/compiler/lib/src/common/elements.dart b/pkg/compiler/lib/src/common/elements.dart index 96807dd3a3b9..3b328fd5da81 100644 --- a/pkg/compiler/lib/src/common/elements.dart +++ b/pkg/compiler/lib/src/common/elements.dart @@ -11,6 +11,7 @@ import '../elements/entities.dart'; import '../elements/names.dart'; import '../elements/types.dart'; import '../inferrer/abstract_value_domain.dart'; +import '../ir/element_map.dart'; import '../js_backend/native_data.dart' show NativeBasicData; import '../js_model/locals.dart'; import '../universe/selector.dart' show Selector; @@ -863,6 +864,9 @@ abstract class CommonElements { FunctionEntity _findRtiFunction(String name) => _findLibraryMember(rtiLibrary, name)!; + late final FunctionEntity interopNullAssertion = + _findRtiFunction('_interopNullAssertion'); + late final FunctionEntity setArrayType = _findRtiFunction('_setArrayType'); late final FunctionEntity findType = _findRtiFunction('findType'); @@ -1279,6 +1283,8 @@ class JCommonElements extends CommonElements { // interface, the first should only be used during resolution and the latter in // both resolution and codegen. abstract class ElementEnvironment { + IrToElementMap get elementMap; + /// Returns the main library for the compilation. LibraryEntity? get mainLibrary; diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart index 6c254c23a503..d5b73b436bfb 100644 --- a/pkg/compiler/lib/src/dart2js.dart +++ b/pkg/compiler/lib/src/dart2js.dart @@ -452,6 +452,8 @@ Future compile(List argv, _OneOption(Flags.enableNullAssertions, passThrough), _OneOption(Flags.nativeNullAssertions, passThrough), _OneOption(Flags.noNativeNullAssertions, passThrough), + _OneOption(Flags.interopNullAssertions, passThrough), + _OneOption(Flags.noInteropNullAssertions, passThrough), _OneOption(Flags.trustTypeAnnotations, setTrustTypeAnnotations), _OneOption(Flags.trustPrimitives, passThrough), _OneOption(Flags.trustJSInteropTypeAnnotations, ignoreOption), diff --git a/pkg/compiler/lib/src/js_backend/backend_impact.dart b/pkg/compiler/lib/src/js_backend/backend_impact.dart index feef277b76b7..d515db9308a3 100644 --- a/pkg/compiler/lib/src/js_backend/backend_impact.dart +++ b/pkg/compiler/lib/src/js_backend/backend_impact.dart @@ -546,6 +546,7 @@ class BackendImpacts { // TODO(sra): Split into refined impacts. late final BackendImpact newRtiImpact = BackendImpact( staticUses: [ + if (_options.interopNullAssertions) _commonElements.interopNullAssertion, _commonElements.findType, _commonElements.instanceType, _commonElements.arrayInstanceType, diff --git a/pkg/compiler/lib/src/js_backend/native_data.dart b/pkg/compiler/lib/src/js_backend/native_data.dart index adc02f629c6a..2b3e2940da2f 100644 --- a/pkg/compiler/lib/src/js_backend/native_data.dart +++ b/pkg/compiler/lib/src/js_backend/native_data.dart @@ -9,11 +9,15 @@ import 'package:kernel/ast.dart' as ir; import '../common.dart'; import '../common/elements.dart' show ElementEnvironment; import '../elements/entities.dart'; +import '../elements/names.dart'; +import '../elements/types.dart'; import '../ir/annotations.dart'; import '../js_model/js_to_frontend_map.dart' show identity, JsToFrontendMap; import '../kernel/element_map.dart'; import '../native/behavior.dart' show NativeBehavior; import '../serialization/serialization.dart'; +import '../universe/call_structure.dart'; +import '../universe/selector.dart'; import '../util/util.dart'; class NativeBasicDataBuilder { @@ -438,12 +442,93 @@ class NativeDataBuilder { } /// Closes this builder and creates the resulting [NativeData] object. - NativeData close() => NativeData( - _nativeBasicData, - _nativeMemberName, - _nativeMethodBehavior, - _nativeFieldLoadBehavior, - _nativeFieldStoreBehavior); + NativeData close(DiagnosticReporter reporter) { + final data = NativeData( + _nativeBasicData, + _nativeMemberName, + _nativeMethodBehavior, + _nativeFieldLoadBehavior, + _nativeFieldStoreBehavior, {}); + + if (reporter.options.interopNullAssertions) { + // We can enforce the return type nullability of an interop API in two + // ways: by putting the null check on the invocation in the caller, or by + // putting the null check on the return value in the callee body + // (generally an interceptor method). It is only safe to do the latter if + // all interop bindings that share the interceptor method have consistent + // nullabilities. + + final environment = _nativeBasicData._env; + final dartTypes = environment.elementMap.commonElements.dartTypes; + + bool returnTypeIsNonNullable(FunctionEntity member, + {required bool callthrough}) { + final memberType = environment.getFunctionType(member); + final functionType = + callthrough ? memberType.returnType as FunctionType : memberType; + return dartTypes.isNonNullableIfSound(functionType.returnType); + } + + // Intercepted methods keyed by selector. + final jsNameMap = >{}; + for (final (member as FunctionEntity) + in _nativeBasicData._jsInteropMembers.keys) { + if (!member.isInstanceMember) continue; + if (!member.isFunction && !member.isGetter) continue; + + // The program builder uses the unescaped name for interceptor methods. + // We can only perform null checks in the interceptor method body if all + // the intercepted methods with the same name have consistent return + // type nullabilities. + // We use a public name because the interceptor will not distinguish + // methods from different libraries, even if they have a leading + // underscore. + final name = + PublicName(data.computeUnescapedJSInteropName(member.name!)); + + void addAllPossibleInvocations(FunctionType type) { + final requiredPositionalCount = type.parameterTypes.length; + final optionalPositionalCount = type.optionalParameterTypes.length; + // We do not yet know which invocations are actually live in the + // program, so we conservatively allow for any number of optional + // arguments to be passed. Named parameters are not supported. + for (var i = 0; i <= optionalPositionalCount; i++) { + (jsNameMap[Selector.call(name, + CallStructure.unnamed(requiredPositionalCount + i))] ??= []) + .add(member); + } + } + + if (member.isGetter) { + (jsNameMap[Selector.getter(name)] ??= []).add(member); + final returnType = environment.getFunctionType(member).returnType; + if (returnType is FunctionType) { + addAllPossibleInvocations(returnType); + } + } else if (member.isFunction) { + final functionType = environment.getFunctionType(member); + addAllPossibleInvocations(functionType); + } + } + + jsNameMap.forEach((selector, members) { + final canCheckInCallee = members.every((FunctionEntity member) => + returnTypeIsNonNullable(member, + callthrough: + member.isGetter && selector.kind == SelectorKind.CALL)); + data.interopNullChecks[selector] = canCheckInCallee + ? InteropNullCheckKind.calleeCheck + : InteropNullCheckKind.callerCheck; + }); + } + + return data; + } +} + +enum InteropNullCheckKind { + calleeCheck, + callerCheck, } /// Additional element information for native classes and methods and js-interop @@ -475,12 +560,17 @@ class NativeData implements NativeBasicData { /// Cache for [NativeBehavior]s for writing to native fields. final Map _nativeFieldStoreBehavior; + /// A map from selectors for interop members to the type of null check + /// required when `--interop-null-assertions` is passed. + final Map interopNullChecks; + NativeData( this._nativeBasicData, this._nativeMemberName, this._nativeMethodBehavior, this._nativeFieldLoadBehavior, - this._nativeFieldStoreBehavior); + this._nativeFieldStoreBehavior, + this.interopNullChecks); factory NativeData.fromIr(KernelToElementMap map, IrAnnotationData data) { NativeBasicData nativeBasicData = NativeBasicData.fromIr(map, data); @@ -517,7 +607,7 @@ class NativeData implements NativeBasicData { }); return NativeData(nativeBasicData, nativeMemberName, nativeMethodBehavior, - nativeFieldLoadBehavior, nativeFieldStoreBehavior); + nativeFieldLoadBehavior, nativeFieldStoreBehavior, const {}); } /// Deserializes a [NativeData] object from [source]. @@ -537,9 +627,11 @@ class NativeData implements NativeBasicData { Map nativeFieldStoreBehavior = source.readMemberMap( (MemberEntity member) => NativeBehavior.readFromDataSource(source)); + Map interopNullChecks = source + .readSelectorMap((_) => source.readEnum(InteropNullCheckKind.values)); source.end(tag); return NativeData(nativeBasicData, nativeMemberName, nativeMethodBehavior, - nativeFieldLoadBehavior, nativeFieldStoreBehavior); + nativeFieldLoadBehavior, nativeFieldStoreBehavior, interopNullChecks); } /// Serializes this [NativeData] to [sink]. @@ -565,6 +657,9 @@ class NativeData implements NativeBasicData { behavior.writeToDataSink(sink); }); + sink.writeSelectorMap( + interopNullChecks, sink.writeEnum); + sink.end(tag); } @@ -846,7 +941,7 @@ class NativeData implements NativeBasicData { Map nativeFieldStoreBehavior = map .toBackendMemberMap(_nativeFieldStoreBehavior, _convertNativeBehavior); return NativeData(nativeBasicData, nativeMemberName, nativeMethodBehavior, - nativeFieldLoadBehavior, nativeFieldStoreBehavior); + nativeFieldLoadBehavior, nativeFieldStoreBehavior, interopNullChecks); } } diff --git a/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart b/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart index ade581bd1175..7c33d5bdfd9e 100644 --- a/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart +++ b/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart @@ -426,6 +426,9 @@ class ProgramBuilder { interceptorClass?.isChecks.addAll(_jsInteropIsChecks); interceptorTypeData?.classChecks.addAll(_jsInteropTypeChecks); + late final interopNullAssert = _task.emitter + .staticFunctionAccess(_commonElements.interopNullAssertion); + Set stubNames = {}; librariesMap.forEach((LibraryEntity library, List classElements, _memberElement, _typeElement) { @@ -443,9 +446,14 @@ class ProgramBuilder { for (Selector selector in selectors) { js.Name stubName = _namer.invocationName(selector); if (stubNames.add(stubName.key)) { - interceptorClass!.callStubs.add(_buildStubMethod(stubName, - js.js('function(obj) { return obj.# }', [jsName]), - element: member)); + final code = _options.interopNullAssertions && + _nativeData.interopNullChecks[selector] == + InteropNullCheckKind.calleeCheck + ? js.js('function(obj) { return #(obj.#) }', + [interopNullAssert, jsName]) + : js.js('function(obj) { return obj.# }', [jsName]); + interceptorClass!.callStubs + .add(_buildStubMethod(stubName, code, element: member)); } } } @@ -511,11 +519,16 @@ class ProgramBuilder { // functions. The behavior of this solution matches JavaScript // behavior implicitly binding this only when JavaScript // would. - interceptorClass!.callStubs.add(_buildStubMethod( - stubName, - js.js('function(receiver, #) { return receiver.#(#) }', - [parameters, jsName, parameters]), - element: member)); + final code = _options.interopNullAssertions && + _nativeData.interopNullChecks[selector] == + InteropNullCheckKind.calleeCheck + ? js.js( + 'function(receiver, #) { return #(receiver.#(#)) }', + [parameters, interopNullAssert, jsName, parameters]) + : js.js('function(receiver, #) { return receiver.#(#) }', + [parameters, jsName, parameters]); + interceptorClass!.callStubs + .add(_buildStubMethod(stubName, code, element: member)); } } } diff --git a/pkg/compiler/lib/src/js_model/element_map_impl.dart b/pkg/compiler/lib/src/js_model/element_map_impl.dart index 12c0b1ab52f7..b693389a1115 100644 --- a/pkg/compiler/lib/src/js_model/element_map_impl.dart +++ b/pkg/compiler/lib/src/js_model/element_map_impl.dart @@ -2074,6 +2074,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap { class JsElementEnvironment extends ElementEnvironment implements JElementEnvironment { + @override final JsKernelToElementMap elementMap; JsElementEnvironment(this.elementMap); diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart index 870c9d9b89c2..845d5a6580d9 100644 --- a/pkg/compiler/lib/src/kernel/element_map_impl.dart +++ b/pkg/compiler/lib/src/kernel/element_map_impl.dart @@ -1619,6 +1619,7 @@ class KernelToElementMap implements IrToElementMap { class KernelElementEnvironment extends ElementEnvironment implements KElementEnvironment { + @override final KernelToElementMap elementMap; KernelElementEnvironment(this.elementMap); diff --git a/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart b/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart index 26231749e9aa..913659742fdc 100644 --- a/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart +++ b/pkg/compiler/lib/src/kernel/transformations/global/js_get_flag_lowering.dart @@ -51,28 +51,18 @@ class JsGetFlagLowering { ..fileOffset = node.fileOffset; } - bool? _getFlagValue(String flagName) { - switch (flagName) { - case 'DEV_COMPILER': - return false; - case 'MINIFIED': - return _options.enableMinification; - case 'MUST_RETAIN_METADATA': - return false; - case 'USE_CONTENT_SECURITY_POLICY': - return _options.features.useContentSecurityPolicy.isEnabled; - case 'VARIANCE': - return _options.enableVariance; - case 'LEGACY': - return _options.useLegacySubtyping; - case 'EXTRA_NULL_SAFETY_CHECKS': - return _options.experimentNullSafetyChecks; - case 'PRINT_LEGACY_STARS': - return _options.printLegacyStars; - default: - return null; - } - } + bool? _getFlagValue(String flagName) => switch (flagName) { + 'DEV_COMPILER' => false, + 'MINIFIED' => _options.enableMinification, + 'MUST_RETAIN_METADATA' => false, + 'USE_CONTENT_SECURITY_POLICY' => + _options.features.useContentSecurityPolicy.isEnabled, + 'VARIANCE' => _options.enableVariance, + 'LEGACY' => _options.useLegacySubtyping, + 'EXTRA_NULL_SAFETY_CHECKS' => _options.experimentNullSafetyChecks, + 'PRINT_LEGACY_STARS' => _options.printLegacyStars, + _ => null, + }; Never _unsupportedFlag(Object? flag) => throw UnsupportedError('Unexpected JS_GET_FLAG argument: $flag'); diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart index a0238945f07d..b7a0a1bfb673 100644 --- a/pkg/compiler/lib/src/options.dart +++ b/pkg/compiler/lib/src/options.dart @@ -508,6 +508,11 @@ class CompilerOptions implements DiagnosticOptions { bool nativeNullAssertions = false; bool _noNativeNullAssertions = false; + /// Whether to generate code asserting that return values of JS-interop APIs + /// with non-nullable return types are not null. + bool interopNullAssertions = false; + bool _noInteropNullAssertions = false; + /// Whether to generate a source-map file together with the output program. bool generateSourceMap = true; @@ -868,6 +873,9 @@ class CompilerOptions implements DiagnosticOptions { ..nativeNullAssertions = _hasOption(options, Flags.nativeNullAssertions) .._noNativeNullAssertions = _hasOption(options, Flags.noNativeNullAssertions) + ..interopNullAssertions = _hasOption(options, Flags.interopNullAssertions) + .._noInteropNullAssertions = + _hasOption(options, Flags.noInteropNullAssertions) ..experimentalTrackAllocations = _hasOption(options, Flags.experimentalTrackAllocations) ..experimentStartupFunctions = @@ -984,13 +992,19 @@ class CompilerOptions implements DiagnosticOptions { throw ArgumentError("Missing required ${Flags.platformBinaries}"); } if (_soundNullSafety && _noSoundNullSafety) { - throw ArgumentError("'${Flags.soundNullSafety}' incompatible with " + throw ArgumentError("'${Flags.soundNullSafety}' is incompatible with " "'${Flags.noSoundNullSafety}'"); } if (nativeNullAssertions && _noNativeNullAssertions) { - throw ArgumentError("'${Flags.nativeNullAssertions}' incompatible with " + throw ArgumentError( + "'${Flags.nativeNullAssertions}' is incompatible with " "'${Flags.noNativeNullAssertions}'"); } + if (interopNullAssertions && _noInteropNullAssertions) { + throw ArgumentError( + "'${Flags.interopNullAssertions}' is incompatible with " + "'${Flags.noInteropNullAssertions}'"); + } if (nullSafetyMode == NullSafetyMode.sound && experimentNullSafetyChecks) { throw ArgumentError('${Flags.experimentNullSafetyChecks} is incompatible ' 'with sound null safety.'); @@ -1081,6 +1095,10 @@ class CompilerOptions implements DiagnosticOptions { nativeNullAssertions = true; } + if (_noInteropNullAssertions) { + interopNullAssertions = false; + } + if (_mergeFragmentsThreshold != null) { mergeFragmentsThreshold = _mergeFragmentsThreshold; } diff --git a/pkg/compiler/lib/src/serialization/serialization.dart b/pkg/compiler/lib/src/serialization/serialization.dart index 7067c6d59b4f..2fa6a49b40c4 100644 --- a/pkg/compiler/lib/src/serialization/serialization.dart +++ b/pkg/compiler/lib/src/serialization/serialization.dart @@ -23,6 +23,7 @@ import '../js_model/elements.dart'; import '../js_model/locals.dart'; import '../js_model/type_recipe.dart' show TypeRecipe; import '../universe/record_shape.dart' show RecordShape; +import '../universe/selector.dart'; import '../options.dart'; import 'deferrable.dart'; diff --git a/pkg/compiler/lib/src/serialization/sink.dart b/pkg/compiler/lib/src/serialization/sink.dart index 720589d9ff7d..fb694aef3777 100644 --- a/pkg/compiler/lib/src/serialization/sink.dart +++ b/pkg/compiler/lib/src/serialization/sink.dart @@ -876,6 +876,17 @@ class DataSinkWriter { writeMap(map, writeLocal, f); } + /// Writes the [map] from selectors to [V] values to this data sink, calling + /// [f] to write each value to the data sink. + /// + /// This is a convenience method to be used together with + /// [DataSourceReader.readSelectorMap]. + void writeSelectorMap(Map map, void f(V value)) { + writeMap(map, (Selector selector) { + selector.writeToDataSink(this); + }, f); + } + /// Writes the constant [value] to this data sink. void writeConstant(ConstantValue value) { _writeDataKind(DataKind.constant); diff --git a/pkg/compiler/lib/src/serialization/source.dart b/pkg/compiler/lib/src/serialization/source.dart index e956506f6f75..36188be41068 100644 --- a/pkg/compiler/lib/src/serialization/source.dart +++ b/pkg/compiler/lib/src/serialization/source.dart @@ -1152,6 +1152,31 @@ class DataSourceReader { return map; } + /// Reads a map from selectors to [V] values from this data source, calling + /// [f] to read each value from the data source. + /// + /// This is a convenience method to be used together with + /// [DataSinkWriter.writeSelectorMap]. + Map readSelectorMap(V f(Selector selector)) => + readSelectorMapOrNull(f) ?? {}; + + /// Reads a map from selectors to [V] values from this data source, calling + /// [f] to read each value from the data source. + /// 'null' is returned instead of an empty map. + /// + /// This is a convenience method to be used together with + /// [DataSinkWriter.writeSelectorMap]. + Map? readSelectorMapOrNull(V f(Selector selector)) { + int count = readInt(); + if (count == 0) return null; + Map map = {}; + for (int i = 0; i < count; i++) { + final selector = Selector.readFromDataSource(this); + map[selector] = f(selector); + } + return map; + } + /// Reads a constant value from this data source. ConstantValue readConstant() { _checkDataKind(DataKind.constant); diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart index ea3728f6808a..b19aa2f07b92 100644 --- a/pkg/compiler/lib/src/ssa/builder.dart +++ b/pkg/compiler/lib/src/ssa/builder.dart @@ -1340,6 +1340,7 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault push(HInvokeExternal(stubTarget, nativeInputs, returnType, closedWorld.nativeData.getNativeMethodBehavior(stubTarget), sourceInformation: sourceInformation)); + _maybeAddInteropNullAssertionForMember(stubTarget, nativeInputs.length); } else if (stubTarget.isInstanceMember) { if (stubTarget.enclosingClass!.isClosure) { push(HInvokeClosure( @@ -1904,16 +1905,27 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault push(HInvokeExternal( targetElement as FunctionEntity, inputs, returnType, nativeBehavior, sourceInformation: null)); - HInstruction value = pop(); + HInstruction value; // TODO(johnniwinther): Provide source information. - if (options.nativeNullAssertions) { + if (options.nativeNullAssertions && nodeIsInWebLibrary(functionNode)) { + value = pop(); DartType type = _getDartTypeIfValid(functionNode.returnType); - if (dartTypes.isNonNullableIfSound(type) && - nodeIsInWebLibrary(functionNode)) { + if (dartTypes.isNonNullableIfSound(type)) { push(HNullCheck(value, _abstractValueDomain.excludeNull(returnType), sticky: true)); value = pop(); } + } else if (_nativeData.isJsInteropMember(targetElement)) { + if (targetElement.isInstanceMember) { + _maybeAddInteropNullAssertionForMember( + targetElement as FunctionEntity, inputs.length); + } else { + _maybeAddInteropNullAssertionForStatic( + _getDartTypeIfValid(functionNode.returnType)); + } + value = pop(); + } else { + value = pop(); } if (targetElement.isSetter) { _closeAndGotoExit(HGoto()); @@ -5313,6 +5325,44 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault } } + void _maybeAddInteropNullAssertionForMember( + FunctionEntity member, int argumentCount) { + if (options.interopNullAssertions) { + final name = + PublicName(_nativeData.computeUnescapedJSInteropName(member.name!)); + _addInteropNullAssertionForSelector( + Selector.call(name, CallStructure.unnamed(argumentCount))); + } + } + + void _maybeAddInteropNullAssertionForSelector(Selector selector) { + if (options.interopNullAssertions && + _nativeData.interopNullChecks[selector] == + InteropNullCheckKind.callerCheck) { + _addInteropNullAssertion(); + } + } + + void _addInteropNullAssertionForSelector(Selector selector) { + if (_nativeData.interopNullChecks[selector] == + InteropNullCheckKind.callerCheck) { + _addInteropNullAssertion(); + } + } + + void _maybeAddInteropNullAssertionForStatic(DartType returnType) { + if (options.interopNullAssertions && + closedWorld.dartTypes.isNonNullableIfSound(returnType)) { + _addInteropNullAssertion(); + } + } + + void _addInteropNullAssertion() { + final value = pop(); + _pushStaticInvocation(_commonElements.interopNullAssertion, [value], + value.instructionType, const []); + } + void _handleJsStringConcat( ir.StaticInvocation invocation, SourceInformation? sourceInformation) { if (_unexpectedForeignArguments(invocation, @@ -5515,8 +5565,7 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault List arguments, AbstractValue typeMask, List typeArguments) { - push(_invokeJsInteropFunction(target, arguments)); - return; + _invokeJsInteropFunction(target, arguments); } void _pushDynamicInvocation( @@ -5619,9 +5668,12 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault invoke.isBoundsSafe = node.isBoundsSafe; } push(invoke); + if (element != null) { + _maybeAddInteropNullAssertionForSelector(selector); + } } - HInstruction _invokeJsInteropFunction( + void _invokeJsInteropFunction( FunctionEntity element, List arguments) { assert(closedWorld.nativeData.isJsInteropMember(element)); @@ -5671,9 +5723,10 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault var nativeBehavior = NativeBehavior()..codeTemplate = codeTemplate; registry.registerNativeMethod(element); // TODO(efortuna): Source information. - return HForeignCode( + push(HForeignCode( codeTemplate, _abstractValueDomain.dynamicType, filteredArguments, - nativeBehavior: nativeBehavior); + nativeBehavior: nativeBehavior)); + return; } // Strip off trailing arguments that were not specified. @@ -5711,8 +5764,9 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault _typeInferenceMap.typeFromNativeBehavior(nativeBehavior, closedWorld); // TODO(efortuna): Add source information. - return HInvokeExternal(element, inputs, instructionType, nativeBehavior, - sourceInformation: null); + push(HInvokeExternal(element, inputs, instructionType, nativeBehavior, + sourceInformation: null)); + _maybeAddInteropNullAssertionForStatic(type); } @override diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart index ae6824257b1e..599aa022936c 100644 --- a/pkg/compiler/lib/src/ssa/optimize.dart +++ b/pkg/compiler/lib/src/ssa/optimize.dart @@ -1004,16 +1004,24 @@ class SsaInstructionSimplifier extends HBaseVisitor HInstruction maybeAddNativeReturnNullCheck( HInstruction node, HInstruction replacement, FunctionEntity method) { - if (_options.nativeNullAssertions) { + if (_options.nativeNullAssertions && memberEntityIsInWebLibrary(method)) { FunctionType type = _closedWorld.elementEnvironment.getFunctionType(method); - if (_closedWorld.dartTypes.isNonNullableIfSound(type.returnType) && - memberEntityIsInWebLibrary(method)) { + if (_closedWorld.dartTypes.isNonNullableIfSound(type.returnType)) { node.block!.addBefore(node, replacement); replacement = HNullCheck(replacement, _abstractValueDomain.excludeNull(replacement.instructionType), sticky: true); } + } else if (_options.interopNullAssertions && + _nativeData.interopNullChecks.containsKey(Selector.getter(PublicName( + _nativeData.computeUnescapedJSInteropName(method.name!))))) { + node.block!.addBefore(node, replacement); + final replacementType = _options.experimentNullSafetyChecks + ? replacement.instructionType + : _abstractValueDomain.excludeNull(replacement.instructionType); + replacement = HInvokeStatic(commonElements.interopNullAssertion, + [replacement], replacementType, const []); } return replacement; } diff --git a/pkg/compiler/lib/src/universe/resolution_world_builder.dart b/pkg/compiler/lib/src/universe/resolution_world_builder.dart index 63b9610bc52e..7f08f8cf06aa 100644 --- a/pkg/compiler/lib/src/universe/resolution_world_builder.dart +++ b/pkg/compiler/lib/src/universe/resolution_world_builder.dart @@ -1000,7 +1000,7 @@ class ResolutionWorldBuilder extends WorldBuilder implements World { elementEnvironment: elementEnvironment as KElementEnvironment, dartTypes: _dartTypes, commonElements: _commonElements as KCommonElements, - nativeData: _nativeDataBuilder.close(), + nativeData: _nativeDataBuilder.close(reporter), interceptorData: _interceptorDataBuilder.close(), backendUsage: backendUsage, noSuchMethodData: _noSuchMethodRegistry.close(), diff --git a/sdk/lib/_internal/js_shared/lib/rti.dart b/sdk/lib/_internal/js_shared/lib/rti.dart index 07b5ad763d98..3f61c5289e15 100644 --- a/sdk/lib/_internal/js_shared/lib/rti.dart +++ b/sdk/lib/_internal/js_shared/lib/rti.dart @@ -52,6 +52,23 @@ void _onExtraNullSafetyError(TypeError error, StackTrace trace) { } } +class _InteropNullAssertionError extends _Error implements TypeError { + _InteropNullAssertionError() + : super('Non-nullable interop API returned null value.'); +} + +/// Called from generated code. +Object? _interopNullAssertion(Object? value) { + if (JS_GET_FLAG('EXTRA_NULL_SAFETY_CHECKS')) { + if (value == null) { + _onExtraNullSafetyError(_InteropNullAssertionError(), StackTrace.current); + } + return value; + } else { + return value!; + } +} + /// The name of a property on the constructor function of Dart Object /// and interceptor types, used for caching Rti types. const constructorRtiCachePropertyName = r'$ccache'; diff --git a/tests/web/js_interop_non_null_asserts_disabled_test.dart b/tests/web/js_interop_non_null_asserts_disabled_test.dart index 9adff00d7805..bd1005747cc9 100644 --- a/tests/web/js_interop_non_null_asserts_disabled_test.dart +++ b/tests/web/js_interop_non_null_asserts_disabled_test.dart @@ -12,7 +12,5 @@ import 'js_interop_non_null_asserts_utils.dart'; void main() { - topLevelMemberTests(checksEnabled: false); - anonymousClassTests(checksEnabled: false); - namedClassTests(checksEnabled: false); + runTests(checksEnabled: false); } diff --git a/tests/web/js_interop_non_null_asserts_enabled_test.dart b/tests/web/js_interop_non_null_asserts_enabled_test.dart index 5afd3ab9041c..94ef16fb1465 100644 --- a/tests/web/js_interop_non_null_asserts_enabled_test.dart +++ b/tests/web/js_interop_non_null_asserts_enabled_test.dart @@ -17,7 +17,5 @@ import 'js_interop_non_null_asserts_utils.dart'; void main() { - topLevelMemberTests(checksEnabled: true); - anonymousClassTests(checksEnabled: true); - namedClassTests(checksEnabled: true); + runTests(checksEnabled: true); } diff --git a/tests/web/js_interop_non_null_asserts_utils.dart b/tests/web/js_interop_non_null_asserts_utils.dart index b088bd9987d6..493ccde6292b 100644 --- a/tests/web/js_interop_non_null_asserts_utils.dart +++ b/tests/web/js_interop_non_null_asserts_utils.dart @@ -5,78 +5,282 @@ import 'package:expect/expect.dart'; import 'package:js/js.dart'; +const dart2js = const bool.fromEnvironment('dart.library._dart2js_only'); +const ddc = const bool.fromEnvironment('dart.library._ddc_only'); + +dynamic obj; + @JS() external dynamic eval(String code); @JS() +@pragma('dart2js:never-inline') external String returnsNull(); @JS() +@pragma('dart2js:prefer-inline') +external String returnsNullInlined(); + +@JS() +@pragma('dart2js:never-inline') external String returnsUndefined(); @JS() +@pragma('dart2js:prefer-inline') +external String returnsUndefinedInlined(); + +@JS() +@pragma('dart2js:never-inline') external int get getsNull; @JS() +@pragma('dart2js:prefer-inline') +external int get getsNullInlined; + +@JS() +@pragma('dart2js:never-inline') external int get undefinedGetter; @JS() +@pragma('dart2js:prefer-inline') +external int get undefinedGetterInlined; + +@JS() +@pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsNull; @JS() +@pragma('dart2js:prefer-inline') +external double Function() get getterFunctionReturnsNullInlined; + +@JS() +@pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsUndefined; @JS() +@pragma('dart2js:prefer-inline') +external double Function() get getterFunctionReturnsUndefinedInlined; + +@JS() +@pragma('dart2js:never-inline') external bool nullField; @JS() +@pragma('dart2js:prefer-inline') +external bool nullFieldInlined; + +@JS() +@pragma('dart2js:never-inline') external bool undefinedField; +@JS() +@pragma('dart2js:prefer-inline') +external bool undefinedFieldInlined; + @JS() @anonymous class SomeClass { + @pragma('dart2js:never-inline') external String returnsNull(); + + @pragma('dart2js:prefer-inline') + external String returnsNullInlined(); + + @pragma('dart2js:never-inline') external String returnsUndefined(); + + @pragma('dart2js:prefer-inline') + external String returnsUndefinedInlined(); + + @pragma('dart2js:never-inline') external int get getsNull; + + @pragma('dart2js:prefer-inline') + external int get getsNullInlined; + + @pragma('dart2js:never-inline') external int get undefinedGetter; + + @pragma('dart2js:prefer-inline') + external int get undefinedGetterInlined; + + @pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsNull; + + @pragma('dart2js:prefer-inline') + external double Function() get getterFunctionReturnsNullInlined; + + @pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsUndefined; + + @pragma('dart2js:prefer-inline') + external double Function() get getterFunctionReturnsUndefinedInlined; + + @pragma('dart2js:never-inline') external bool nullField; + + @pragma('dart2js:prefer-inline') + external bool nullFieldInlined; + + @pragma('dart2js:never-inline') external bool undefinedField; + + @pragma('dart2js:prefer-inline') + external bool undefinedFieldInlined; } @JS() @anonymous class AnotherClass { + @pragma('dart2js:never-inline') external static String staticReturnsNull(); + + @pragma('dart2js:prefer-inline') + external static String staticReturnsNullInlined(); + + @pragma('dart2js:never-inline') external static String staticReturnsUndefined(); + + @pragma('dart2js:prefer-inline') + external static String staticReturnsUndefinedInlined(); + + @pragma('dart2js:never-inline') external static int get staticGetsNull; + + @pragma('dart2js:prefer-inline') + external static int get staticGetsNullInlined; + + @pragma('dart2js:never-inline') external static int get staticUndefinedGetter; + + @pragma('dart2js:prefer-inline') + external static int get staticUndefinedGetterInlined; + + @pragma('dart2js:never-inline') external static double Function() get staticGetterFunctionReturnsNull; + + @pragma('dart2js:prefer-inline') + external static double Function() get staticGetterFunctionReturnsNullInlined; + + @pragma('dart2js:never-inline') external static double Function() get staticGetterFunctionReturnsUndefined; + + @pragma('dart2js:prefer-inline') + external static double Function() + get staticGetterFunctionReturnsUndefinedInlined; + + @pragma('dart2js:never-inline') external static bool staticNullField; + + @pragma('dart2js:prefer-inline') + external static bool staticNullFieldInlined; + + @pragma('dart2js:never-inline') external static bool staticUndefinedField; + + @pragma('dart2js:prefer-inline') + external static bool staticUndefinedFieldInlined; } @JS() class NamedClass { + @pragma('dart2js:never-inline') external String returnsNull(); + + @pragma('dart2js:prefer-inline') + external String returnsNullInlined(); + + @pragma('dart2js:never-inline') external String returnsUndefined(); + + @pragma('dart2js:prefer-inline') + external String returnsUndefinedInlined(); + + @pragma('dart2js:never-inline') external static String staticReturnsNull(); + + @pragma('dart2js:prefer-inline') + external static String staticReturnsNullInlined(); + + @pragma('dart2js:never-inline') external static String staticReturnsUndefined(); + + @pragma('dart2js:prefer-inline') + external static String staticReturnsUndefinedInlined(); + + @pragma('dart2js:never-inline') external int get getsNull; + + @pragma('dart2js:prefer-inline') + external int get getsNullInlined; + + @pragma('dart2js:never-inline') external int get undefinedGetter; + + @pragma('dart2js:prefer-inline') + external int get undefinedGetterInlined; + + @pragma('dart2js:never-inline') external static int get staticGetsNull; + + @pragma('dart2js:prefer-inline') + external static int get staticGetsNullInlined; + + @pragma('dart2js:never-inline') external static int get staticUndefinedGetter; + + @pragma('dart2js:prefer-inline') + external static int get staticUndefinedGetterInlined; + + @pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsNull; + + @pragma('dart2js:prefer-inline') + external double Function() get getterFunctionReturnsNullInlined; + + @pragma('dart2js:never-inline') external double Function() get getterFunctionReturnsUndefined; + + @pragma('dart2js:prefer-inline') + external double Function() get getterFunctionReturnsUndefinedInlined; + + @pragma('dart2js:never-inline') external static double Function() get staticGetterFunctionReturnsNull; + + @pragma('dart2js:prefer-inline') + external static double Function() get staticGetterFunctionReturnsNullInlined; + + @pragma('dart2js:never-inline') external static double Function() get staticGetterFunctionReturnsUndefined; + + @pragma('dart2js:prefer-inline') + external static double Function() + get staticGetterFunctionReturnsUndefinedInlined; + + @pragma('dart2js:never-inline') external bool nullField; + + @pragma('dart2js:prefer-inline') + external bool nullFieldInlined; + + @pragma('dart2js:never-inline') external bool undefinedField; + + @pragma('dart2js:prefer-inline') + external bool undefinedFieldInlined; + + @pragma('dart2js:never-inline') external static bool staticNullField; + + @pragma('dart2js:prefer-inline') + external static bool staticNullFieldInlined; + + @pragma('dart2js:never-inline') external static bool staticUndefinedField; + @pragma('dart2js:prefer-inline') + external static bool staticUndefinedFieldInlined; + external factory NamedClass.createNamedClass(); } @@ -84,6 +288,10 @@ class NamedClass { external SomeClass createSomeClass(); void topLevelMemberTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + eval(r'self.returnsNull = function() { return null; }'); Expect.throwsTypeErrorWhen(checksEnabled, () => returnsNull()); @@ -119,28 +327,107 @@ void topLevelMemberTests({required bool checksEnabled}) { Expect.throwsTypeErrorWhen(checksEnabled, () => undefinedField); } -void anonymousClassTests({required bool checksEnabled}) { +void inlinedTopLevelMemberTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + eval(r'self.returnsNullInlined = function() { return null; }'); + Expect.throwsTypeErrorWhen(checksEnabled, () => returnsNullInlined()); + + eval(r'self.returnsUndefinedInlined = function() { return void 0; }'); + Expect.throwsTypeErrorWhen(checksEnabled, () => returnsUndefinedInlined()); + + var functionTearoff = returnsNullInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + functionTearoff = returnsUndefinedInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + + eval(r'self.getsNullInlined = null;'); + Expect.throwsTypeErrorWhen(checksEnabled, () => getsNullInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => undefinedGetterInlined); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + eval(r'self.getterFunctionReturnsNullInlined = function() { return null; };'); + Expect.isNull(getterFunctionReturnsNullInlined()); + var getterFunction = getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + + eval(r'self.getterFunctionReturnsUndefinedInlined = function() { ' + 'return void 0; };'); + Expect.isNull(getterFunctionReturnsUndefinedInlined()); + getterFunction = getterFunctionReturnsUndefinedInlined; + Expect.isNull(getterFunction()); + + eval(r'self.nullFieldInlined = null;'); + Expect.throwsTypeErrorWhen(checksEnabled, () => nullFieldInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => undefinedFieldInlined); +} + +void anonymousClassSetup() { eval(r'''self.createSomeClass = function() { return { "returnsNull": function() { return null; }, + "returnsNullInlined": function() { return null; }, "returnsUndefined": function() { return void 0; }, + "returnsUndefinedInlined": function() { return void 0; }, "getsNull" : null, + "getsNullInlined" : null, "jsGetsNull": { get: function() { return null; } }, + "jsGetsNullInlined": { get: function() { return null; } }, "jsGetsUndefined": { get: function() { return void 0; } }, + "jsGetsUndefinedInlined": { get: function() { return void 0; } }, "getterFunctionReturnsNull": function() { return null; }, + "getterFunctionReturnsNullInlined": function() { return null; }, "getterFunctionReturnsUndefined": function() { return void 0; }, + "getterFunctionReturnsUndefinedInlined": function() { return void 0; }, "nullField" : null, + "nullFieldInlined" : null, }; }; '''); + eval(r'''self.AnotherClass = class AnotherClass { + static staticReturnsNull() { return null; } + static staticReturnsNullInlined() { return null; } + static staticReturnsUndefined() { return void 0; } + static staticReturnsUndefinedInlined() { return void 0; } + static get staticGetsNull() { return null; } + static get staticGetsNullInlined() { return null; } + static get staticGetterFunctionReturnsNull() { + return function() { return null; }; + } + static get staticGetterFunctionReturnsNullInlined() { + return function() { return null; }; + } + static get staticGetterFunctionReturnsUndefined() { + return function() { return void 0; }; + } + static get staticGetterFunctionReturnsUndefinedInlined() { + return function() { return void 0; }; + } + };'''); +} + +void anonymousClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + var x = createSomeClass(); Expect.throwsTypeErrorWhen(checksEnabled, () => x.returnsNull()); Expect.throwsTypeErrorWhen(checksEnabled, () => x.returnsUndefined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. var functionTearoff = x.returnsNull; - Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); functionTearoff = x.returnsUndefined; - Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(checksEnabled, () => x.getsNull); Expect.throwsTypeErrorWhen(checksEnabled, () => x.undefinedGetter); @@ -163,18 +450,6 @@ void anonymousClassTests({required bool checksEnabled}) { Expect.throwsTypeErrorWhen(checksEnabled, () => x.nullField); Expect.throwsTypeErrorWhen(checksEnabled, () => x.undefinedField); - eval(r'''self.AnotherClass = class AnotherClass { - static staticReturnsNull() { return null; } - static staticReturnsUndefined() { return void 0; } - static get staticGetsNull() { return null; } - static get staticGetterFunctionReturnsNull() { - return function() { return null; }; - } - static get staticGetterFunctionReturnsUndefined() { - return function() { return void 0; }; - } - };'''); - Expect.throwsTypeErrorWhen( checksEnabled, () => AnotherClass.staticReturnsNull()); Expect.throwsTypeErrorWhen( @@ -203,36 +478,221 @@ void anonymousClassTests({required bool checksEnabled}) { checksEnabled, () => AnotherClass.staticUndefinedField); } -void namedClassTests({required bool checksEnabled}) { +void inlinedAnonymousClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + var x = createSomeClass(); + Expect.throwsTypeErrorWhen(checksEnabled, () => x.returnsNullInlined()); + Expect.throwsTypeErrorWhen(checksEnabled, () => x.returnsUndefinedInlined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = x.returnsNullInlined; + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + functionTearoff = x.returnsUndefinedInlined; + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + + Expect.throwsTypeErrorWhen(checksEnabled, () => x.getsNullInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => x.undefinedGetterInlined); + + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + checksEnabled, () => x.getterFunctionReturnsNullInlined()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => x.getterFunctionReturnsUndefinedInlined()); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = x.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = x.getterFunctionReturnsUndefinedInlined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen(checksEnabled, () => x.nullFieldInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => x.undefinedFieldInlined); + + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticReturnsNullInlined()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticReturnsUndefinedInlined()); + functionTearoff = AnotherClass.staticReturnsNullInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + functionTearoff = AnotherClass.staticReturnsUndefinedInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticGetsNullInlined); + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticUndefinedGetterInlined); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + Expect.isNull(AnotherClass.staticGetterFunctionReturnsNullInlined()); + Expect.isNull(AnotherClass.staticGetterFunctionReturnsUndefinedInlined()); + getterFunction = AnotherClass.staticGetterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = AnotherClass.staticGetterFunctionReturnsUndefinedInlined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticNullFieldInlined); + Expect.throwsTypeErrorWhen( + checksEnabled, () => AnotherClass.staticUndefinedFieldInlined); +} + +// DDC does not perform checks for dynamic invocations. +void dynamicAnonymousClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + obj = createSomeClass(); + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.returnsNull()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsUndefined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = obj.returnsNull; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + functionTearoff = obj.returnsUndefined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.getsNull); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedGetter); + + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsNull()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsUndefined()); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = obj.getterFunctionReturnsNull; + Expect.isNull(getterFunction()); + getterFunction = obj.getterFunctionReturnsUndefined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.nullField); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedField); +} + +// DDC does not perform checks for dynamic invocations. +void dynamicInlinedAnonymousClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + obj = createSomeClass(); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsNullInlined()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsUndefinedInlined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = obj.returnsNullInlined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + functionTearoff = obj.returnsUndefinedInlined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getsNullInlined); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedGetterInlined); + + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsNullInlined()); + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, + () => obj.getterFunctionReturnsUndefinedInlined()); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = obj.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = obj.getterFunctionReturnsUndefinedInlined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.nullFieldInlined); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedFieldInlined); +} + +void namedClassSetup() { eval(r'''self.NamedClass = class NamedClass { returnsNull() { return null; } + returnsNullInlined() { return null; } returnsUndefined() { return void 0; } + returnsUndefinedInlined() { return void 0; } static staticReturnsNull() { return null; } + static staticReturnsNullInlined() { return null; } static staticReturnsUndefined() { return void 0; } + static staticReturnsUndefinedInlined() { return void 0; } get getsNull() { return null; } + get getsNullInlined() { return null; } static get staticGetsNull() { return null; } + static get staticGetsNullInlined() { return null; } get getterFunctionReturnsNull() { return function() { return null; }; } + get getterFunctionReturnsNullInlined() { + return function() { return null; }; + } get getterFunctionReturnsUndefined() { return function() { return void 0; }; } + get getterFunctionReturnsUndefinedInlined() { + return function() { return void 0; }; + } static get staticGetterFunctionReturnsNull() { return function() { return null; }; } + static get staticGetterFunctionReturnsNullInlined() { + return function() { return null; }; + } static get staticGetterFunctionReturnsUndefined() { return function() { return void 0; }; } + static get staticGetterFunctionReturnsUndefinedInlined() { + return function() { return void 0; }; + } static createNamedClass() { return new NamedClass(); } };'''); +} + +void namedClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); var y = NamedClass.createNamedClass(); Expect.throwsTypeErrorWhen(checksEnabled, () => y.returnsNull()); Expect.throwsTypeErrorWhen(checksEnabled, () => y.returnsUndefined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. var functionTearoff = y.returnsNull; - Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); functionTearoff = y.returnsUndefined; - Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen( checksEnabled, () => NamedClass.staticReturnsNull()); Expect.throwsTypeErrorWhen( @@ -274,3 +734,230 @@ void namedClassTests({required bool checksEnabled}) { Expect.throwsTypeErrorWhen( checksEnabled, () => NamedClass.staticUndefinedField); } + +void inlinedNamedClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + var y = NamedClass.createNamedClass(); + Expect.throwsTypeErrorWhen(checksEnabled, () => y.returnsNullInlined()); + Expect.throwsTypeErrorWhen(checksEnabled, () => y.returnsUndefinedInlined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = y.returnsNullInlined; + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + functionTearoff = y.returnsUndefinedInlined; + Expect.throwsTypeErrorWhen(ddc && checksEnabled, () => functionTearoff()); + + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticReturnsNullInlined()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticReturnsUndefinedInlined()); + functionTearoff = NamedClass.staticReturnsNullInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + functionTearoff = NamedClass.staticReturnsUndefinedInlined; + Expect.throwsTypeErrorWhen(checksEnabled, () => functionTearoff()); + Expect.throwsTypeErrorWhen(checksEnabled, () => y.getsNullInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => y.undefinedGetterInlined); + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + checksEnabled, () => y.getterFunctionReturnsNullInlined()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => y.getterFunctionReturnsUndefinedInlined()); + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticGetsNullInlined); + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticUndefinedGetterInlined); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = y.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = y.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + Expect.isNull(NamedClass.staticGetterFunctionReturnsNullInlined()); + Expect.isNull(NamedClass.staticGetterFunctionReturnsUndefinedInlined()); + getterFunction = NamedClass.staticGetterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = NamedClass.staticGetterFunctionReturnsUndefinedInlined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen(checksEnabled, () => y.nullFieldInlined); + Expect.throwsTypeErrorWhen(checksEnabled, () => y.undefinedFieldInlined); + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticNullFieldInlined); + Expect.throwsTypeErrorWhen( + checksEnabled, () => NamedClass.staticUndefinedFieldInlined); +} + +// DDC does not perform checks for dynamic invocations. +void dynamicNamedClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + obj = NamedClass.createNamedClass(); + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.returnsNull()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsUndefined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = obj.returnsNull; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + functionTearoff = obj.returnsUndefined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.getsNull); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedGetter); + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsNull()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsUndefined()); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = obj.getterFunctionReturnsNull; + Expect.isNull(getterFunction()); + getterFunction = obj.getterFunctionReturnsNull; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, () => obj.nullField); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedField); +} + +// DDC does not perform checks for dynamic invocations. +void dynamicInlinedNamedClassTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + obj = NamedClass.createNamedClass(); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsNullInlined()); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.returnsUndefinedInlined()); + + // In dart2js, a tearoff of an interop method is indistinguishable from a + // getter returning the underlying function. + var functionTearoff = obj.returnsNullInlined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + functionTearoff = obj.returnsUndefinedInlined; + Expect.throwsTypeErrorWhen(false, () => functionTearoff()); + + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getsNullInlined); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedGetterInlined); + // Immediate invocations of instance getters are seen as function calls so + // the results get checked. + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.getterFunctionReturnsNullInlined()); + Expect.throwsTypeErrorWhen(dart2js && checksEnabled, + () => obj.getterFunctionReturnsUndefinedInlined()); + + // At the time this test was written, getters that return function types don't + // get the same wrappers as function tearoffs so there isn't an opportunity to + // check the return values so they can still leak null or undefined through + // from the JavaScript side. + var getterFunction = obj.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + getterFunction = obj.getterFunctionReturnsNullInlined; + Expect.isNull(getterFunction()); + + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.nullFieldInlined); + Expect.throwsTypeErrorWhen( + dart2js && checksEnabled, () => obj.undefinedFieldInlined); +} + +@JS() +class CollisionA { + @pragma('dart2js:prefer-inline') + external String foo([int? x]); + + @pragma('dart2js:never-inline') + external String bar([int? x]); + + external factory CollisionA.create(); +} + +@JS() +class CollisionB { + @pragma('dart2js:prefer-inline') + external String? foo(); + + @pragma('dart2js:never-inline') + external String? bar(); + + external factory CollisionB.create(); +} + +void collisionClassSetup() { + eval(r''' + self.CollisionA = class CollisionA { + foo(x) { return null; } + bar(x) { return null; } + static create() { return new CollisionA(); } + }; + + self.CollisionB = class CollisionB { + foo() { return null; } + bar() { return null; } + static create() { return new CollisionB(); } + };'''); +} + +void collisionTests({required bool checksEnabled}) { + // Provide at least one nullable value to [Expect.isNull] so that dart2js + // doesn't discard the branch. + Expect.isNull(null); + + var a = CollisionA.create(); + var b = CollisionB.create(); + + // In dart2js, the receiver type is erased to `LegacyJavaScriptObject`, so we + // cannot identify which class's method is being invoked and cannot emit a + // null check. DDC is able to identify the member being invoked. + Expect.throwsTypeErrorWhen(checksEnabled && ddc, () => a.foo()); + Expect.throwsTypeErrorWhen(checksEnabled && ddc, () => a.bar()); + + Expect.isNull(b.foo()); + Expect.isNull(b.bar()); + + // Because `foo$1` and `bar$1` only exist for [CollisionA] and not + // [CollisionB], dart2js still emits these checks in the callee. + Expect.throwsTypeErrorWhen(checksEnabled, () => a.foo(42)); + Expect.throwsTypeErrorWhen(checksEnabled, () => a.bar(42)); +} + +void runTests({required bool checksEnabled}) { + topLevelMemberTests(checksEnabled: checksEnabled); + inlinedTopLevelMemberTests(checksEnabled: checksEnabled); + + anonymousClassSetup(); + anonymousClassTests(checksEnabled: checksEnabled); + inlinedAnonymousClassTests(checksEnabled: checksEnabled); + dynamicAnonymousClassTests(checksEnabled: checksEnabled); + dynamicInlinedAnonymousClassTests(checksEnabled: checksEnabled); + + namedClassSetup(); + namedClassTests(checksEnabled: checksEnabled); + inlinedNamedClassTests(checksEnabled: checksEnabled); + dynamicNamedClassTests(checksEnabled: checksEnabled); + dynamicInlinedNamedClassTests(checksEnabled: checksEnabled); + + collisionClassSetup(); + collisionTests(checksEnabled: checksEnabled); +}