From 4e7d09ceff35757ffe29368701806d91c7c75fd1 Mon Sep 17 00:00:00 2001 From: Gijs Weterings Date: Thu, 3 Apr 2025 08:01:59 -0700 Subject: [PATCH] FIx nullsafe FIXMEs for DialogModule and mark nullsafe (#50353) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50353 Gone trough all the FIXMEs added in the previous diff by the nullsafe tool, marked the class as nullsafe and ensured no remaining violations. Changelog: [Android][Fixed] Made DialogModule.java nullsafe Reviewed By: cortinico Differential Revision: D71979604 fbshipit-source-id: 556b2b10ef0879bf7cd4eac6eaf3c9c3cc1c5413 --- .../react/modules/dialog/DialogModule.java | 17 +++++++------- .../react/modules/dialog/DialogModuleTest.kt | 22 ++++++++++++++----- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java index 9d3f15ad6a2..f08db0be348 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java @@ -12,12 +12,13 @@ import android.content.DialogInterface; import android.content.DialogInterface.OnClickListener; import android.content.DialogInterface.OnDismissListener; import android.os.Bundle; -import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.fragment.app.FragmentActivity; import androidx.fragment.app.FragmentManager; import com.facebook.common.logging.FLog; import com.facebook.fbreact.specs.NativeDialogManagerAndroidSpec; +import com.facebook.infer.annotation.Assertions; +import com.facebook.infer.annotation.Nullsafe; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.LifecycleEventListener; import com.facebook.react.bridge.ReactApplicationContext; @@ -29,6 +30,7 @@ import com.facebook.react.common.MapBuilder; import com.facebook.react.module.annotations.ReactModule; import java.util.Map; +@Nullsafe(Nullsafe.Mode.LOCAL) @ReactModule(name = NativeDialogManagerAndroidSpec.NAME) public class DialogModule extends NativeDialogManagerAndroidSpec implements LifecycleEventListener { @@ -59,12 +61,13 @@ public class DialogModule extends NativeDialogManagerAndroidSpec implements Life super(reactContext); } + @Nullsafe(Nullsafe.Mode.LOCAL) private class FragmentManagerHelper { - private final @NonNull FragmentManager mFragmentManager; + private final FragmentManager mFragmentManager; private @Nullable Object mFragmentToShow; - public FragmentManagerHelper(@NonNull FragmentManager fragmentManager) { + public FragmentManagerHelper(FragmentManager fragmentManager) { mFragmentManager = fragmentManager; } @@ -110,6 +113,7 @@ public class DialogModule extends NativeDialogManagerAndroidSpec implements Life } } + @Nullsafe(Nullsafe.Mode.LOCAL) /* package */ class AlertFragmentListener implements OnClickListener, OnDismissListener { private final Callback mCallback; @@ -130,8 +134,7 @@ public class DialogModule extends NativeDialogManagerAndroidSpec implements Life } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] - public void onDismiss(DialogInterface dialog) { + public void onDismiss(@Nullable DialogInterface dialog) { if (!mCallbackConsumed) { if (getReactApplicationContext().hasActiveReactInstance()) { mCallback.invoke(ACTION_DISMISSED); @@ -199,11 +202,9 @@ public class DialogModule extends NativeDialogManagerAndroidSpec implements Life } if (options.hasKey(KEY_ITEMS)) { ReadableArray items = options.getArray(KEY_ITEMS); - // NULLSAFE_FIXME[Nullable Dereference] + Assertions.assertNotNull(items); CharSequence[] itemsArray = new CharSequence[items.size()]; - // NULLSAFE_FIXME[Nullable Dereference] for (int i = 0; i < items.size(); i++) { - // NULLSAFE_FIXME[Nullable Dereference] itemsArray[i] = items.getString(i); } args.putCharSequenceArray(AlertFragment.ARG_ITEMS, itemsArray); diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt index 5191ce48a33..94df33110da 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/modules/dialog/DialogModuleTest.kt @@ -67,7 +67,7 @@ class DialogModuleTest { putBoolean("cancelable", false) } - dialogModule.showAlert(options, null, null) + dialogModule.showAlert(options, SimpleCallback(), SimpleCallback()) shadowOf(getMainLooper()).idle() val fragment = getFragment() @@ -85,14 +85,16 @@ class DialogModuleTest { fun testCallbackPositive() { val options = JavaOnlyMap().apply { putString("buttonPositive", "OK") } + val errorCallback = SimpleCallback() val actionCallback = SimpleCallback() - dialogModule.showAlert(options, null, actionCallback) + dialogModule.showAlert(options, errorCallback, actionCallback) shadowOf(getMainLooper()).idle() val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_POSITIVE).performClick() shadowOf(getMainLooper()).idle() + assertThat(errorCallback.calls).isEqualTo(0) assertThat(actionCallback.calls).isEqualTo(1) assertThat(actionCallback.args?.get(0)).isEqualTo(DialogModule.ACTION_BUTTON_CLICKED) assertThat(actionCallback.args?.get(1)).isEqualTo(DialogInterface.BUTTON_POSITIVE) @@ -102,14 +104,16 @@ class DialogModuleTest { fun testCallbackNegative() { val options = JavaOnlyMap().apply { putString("buttonNegative", "Cancel") } + val errorCallback = SimpleCallback() val actionCallback = SimpleCallback() - dialogModule.showAlert(options, null, actionCallback) + dialogModule.showAlert(options, errorCallback, actionCallback) shadowOf(getMainLooper()).idle() val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_NEGATIVE).performClick() shadowOf(getMainLooper()).idle() + assertThat(errorCallback.calls).isEqualTo(0) assertThat(actionCallback.calls).isEqualTo(1) assertThat(actionCallback.args?.get(0)).isEqualTo(DialogModule.ACTION_BUTTON_CLICKED) assertThat(actionCallback.args?.get(1)).isEqualTo(DialogInterface.BUTTON_NEGATIVE) @@ -119,14 +123,16 @@ class DialogModuleTest { fun testCallbackNeutral() { val options = JavaOnlyMap().apply { putString("buttonNeutral", "Later") } + val errorCallback = SimpleCallback() val actionCallback = SimpleCallback() - dialogModule.showAlert(options, null, actionCallback) + dialogModule.showAlert(options, errorCallback, actionCallback) shadowOf(getMainLooper()).idle() val dialog = getFragment().dialog as AlertDialog dialog.getButton(DialogInterface.BUTTON_NEUTRAL).performClick() shadowOf(getMainLooper()).idle() + assertThat(errorCallback.calls).isEqualTo(0) assertThat(actionCallback.calls).isEqualTo(1) assertThat(actionCallback.args?.get(0)).isEqualTo(DialogModule.ACTION_BUTTON_CLICKED) assertThat(actionCallback.args?.get(1)).isEqualTo(DialogInterface.BUTTON_NEUTRAL) @@ -136,13 +142,15 @@ class DialogModuleTest { fun testCallbackDismiss() { val options = JavaOnlyMap() + val errorCallback = SimpleCallback() val actionCallback = SimpleCallback() - dialogModule.showAlert(options, null, actionCallback) + dialogModule.showAlert(options, errorCallback, actionCallback) shadowOf(getMainLooper()).idle() getFragment().dialog?.dismiss() shadowOf(getMainLooper()).idle() + assertThat(errorCallback.calls).isEqualTo(0) assertThat(actionCallback.calls).isEqualTo(1) assertThat(actionCallback.args?.get(0)).isEqualTo(DialogModule.ACTION_DISMISSED) } @@ -153,13 +161,15 @@ class DialogModuleTest { val options = JavaOnlyMap() + val errorCallback = SimpleCallback() val actionCallback = SimpleCallback() - dialogModule.showAlert(options, null, actionCallback) + dialogModule.showAlert(options, errorCallback, actionCallback) shadowOf(getMainLooper()).idle() getFragment().dialog?.dismiss() shadowOf(getMainLooper()).idle() + assertThat(errorCallback.calls).isEqualTo(0) assertThat(actionCallback.calls).isEqualTo(1) assertThat(actionCallback.args?.get(0)).isEqualTo(DialogModule.ACTION_DISMISSED) }