From adbcaef1e1632eff29570cce878b65594096a694 Mon Sep 17 00:00:00 2001 From: Gijs Weterings Date: Tue, 18 Mar 2025 06:08:17 -0700 Subject: [PATCH] Fix Nullsafe FIXMEs for DevSupportManagerBase.java (#50068) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50068 Note: this involved tightening up some signatures in Kotlin files 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 DevSupportManagerBase.java nullsafe Reviewed By: mdvacca Differential Revision: D71126381 fbshipit-source-id: 01c08e2fc61eff885911fa9c295d504fa7ea334c --- .../react/devsupport/DevServerHelper.java | 4 +- .../devsupport/DevSupportManagerBase.java | 78 +++++++++---------- .../devsupport/ReleaseDevSupportManager.kt | 20 ++--- .../interfaces/DevBundleDownloadListener.kt | 2 +- .../interfaces/DevSupportManager.kt | 18 ++--- 5 files changed, 59 insertions(+), 63 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java index 5411029777a..da0cac909f4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java @@ -327,7 +327,7 @@ public class DevServerHelper { DevBundleDownloadListener callback, File outputFile, String bundleURL, - BundleDownloader.BundleInfo bundleInfo) { + @Nullable BundleDownloader.BundleInfo bundleInfo) { mBundleDownloader.downloadBundleFromURL(callback, outputFile, bundleURL, bundleInfo); } @@ -335,7 +335,7 @@ public class DevServerHelper { DevBundleDownloadListener callback, File outputFile, String bundleURL, - BundleDownloader.BundleInfo bundleInfo, + @Nullable BundleDownloader.BundleInfo bundleInfo, Request.Builder requestBuilder) { mBundleDownloader.downloadBundleFromURL( callback, outputFile, bundleURL, bundleInfo, requestBuilder); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java index a1b207897ce..0d68f1a2d25 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java @@ -34,6 +34,7 @@ import androidx.annotation.Nullable; import androidx.annotation.UiThread; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; +import com.facebook.infer.annotation.Nullsafe; import com.facebook.react.R; import com.facebook.react.bridge.DefaultJSExceptionHandler; import com.facebook.react.bridge.JSBundleLoader; @@ -74,6 +75,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +@Nullsafe(Nullsafe.Mode.LOCAL) public abstract class DevSupportManagerBase implements DevSupportManager { public interface CallbackWithBundleLoader { @@ -222,16 +224,13 @@ public abstract class DevSupportManagerBase implements DevSupportManager { if (e instanceof JavascriptException) { FLog.e(ReactConstants.TAG, "Exception in native call from JS", e); - showNewError( - // NULLSAFE_FIXME[Nullable Dereference] - e.getMessage().toString(), new StackFrame[] {}, JSEXCEPTION_ERROR_COOKIE, ErrorType.JS); + showNewError(e.getMessage(), new StackFrame[] {}, JSEXCEPTION_ERROR_COOKIE, ErrorType.JS); } else { showNewJavaError(message.toString(), e); } } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void showNewJavaError(@Nullable String message, Throwable e) { FLog.e(ReactConstants.TAG, "Exception in native call", e); showNewError( @@ -244,19 +243,17 @@ public abstract class DevSupportManagerBase implements DevSupportManager { * called. */ @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void addCustomDevOption(String optionName, DevOptionHandler optionHandler) { mCustomDevOptions.put(optionName, optionHandler); } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] - public void showNewJSError(String message, ReadableArray details, int errorCookie) { + public void showNewJSError( + @Nullable String message, @Nullable ReadableArray details, int errorCookie) { showNewError(message, StackTraceHelper.convertJsStackTrace(details), errorCookie, ErrorType.JS); } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void registerErrorCustomizer(ErrorCustomizer errorCustomizer) { if (mErrorCustomizers == null) { mErrorCustomizers = new ArrayList<>(); @@ -265,7 +262,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public Pair processErrorCustomizers(Pair errorInfo) { if (mErrorCustomizers != null) { for (ErrorCustomizer errorCustomizer : mErrorCustomizers) { @@ -287,14 +283,14 @@ public abstract class DevSupportManagerBase implements DevSupportManager { mRedBoxSurfaceDelegate.hide(); } - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public @Nullable View createRootView(String appKey) { return mReactInstanceDevHelper.createRootView(appKey); } - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] - public void destroyRootView(View rootView) { - mReactInstanceDevHelper.destroyRootView(rootView); + public void destroyRootView(@Nullable View rootView) { + if (rootView != null) { + mReactInstanceDevHelper.destroyRootView(rootView); + } } private void hideDevOptionsDialog() { @@ -526,14 +522,12 @@ public abstract class DevSupportManagerBase implements DevSupportManager { mDevOptionsDialog = new AlertDialog.Builder(context) .setCustomTitle(header) - // NULLSAFE_FIXME[Not Vetted Third-Party] .setAdapter( adapter, (dialog, which) -> { optionHandlers[which].onOptionSelected(); mDevOptionsDialog = null; }) - // NULLSAFE_FIXME[Not Vetted Third-Party] .setOnCancelListener(dialog -> mDevOptionsDialog = null) .create(); mDevOptionsDialog.show(); @@ -544,11 +538,10 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } } - private String getJSExecutorDescription() { + private @Nullable String getJSExecutorDescription() { try { return getReactInstanceDevHelper().getJavaScriptExecutorFactory().toString(); } catch (IllegalStateException e) { - // NULLSAFE_FIXME[Return Not Nullable] return null; } } @@ -634,19 +627,20 @@ public abstract class DevSupportManagerBase implements DevSupportManager { if (mIsDevSupportEnabled && mJSBundleDownloadedFile.exists()) { try { String packageName = mApplicationContext.getPackageName(); - PackageInfo thisPackage = - // NULLSAFE_FIXME[Nullable Dereference] - mApplicationContext.getPackageManager().getPackageInfo(packageName, 0); - if (mJSBundleDownloadedFile.lastModified() > thisPackage.lastUpdateTime) { - // Base APK has not been updated since we downloaded JS, but if app is using exopackage - // it may only be a single dex that has been updated. We check for exopackage dir update - // time in that case. - File exopackageDir = - new File(String.format(Locale.US, EXOPACKAGE_LOCATION_FORMAT, packageName)); - if (exopackageDir.exists()) { - return mJSBundleDownloadedFile.lastModified() > exopackageDir.lastModified(); + PackageManager packageManager = mApplicationContext.getPackageManager(); + if (packageManager != null) { + PackageInfo thisPackage = packageManager.getPackageInfo(packageName, 0); + if (mJSBundleDownloadedFile.lastModified() > thisPackage.lastUpdateTime) { + // Base APK has not been updated since we downloaded JS, but if app is using exopackage + // it may only be a single dex that has been updated. We check for exopackage dir update + // time in that case. + File exopackageDir = + new File(String.format(Locale.US, EXOPACKAGE_LOCATION_FORMAT, packageName)); + if (exopackageDir.exists()) { + return mJSBundleDownloadedFile.lastModified() > exopackageDir.lastModified(); + } + return true; } - return true; } } catch (PackageManager.NameNotFoundException e) { // Ignore this error and just fallback to loading JS from assets @@ -675,8 +669,10 @@ public abstract class DevSupportManagerBase implements DevSupportManager { if (mCurrentReactContext != null) { try { URL sourceUrl = new URL(getSourceUrl()); - // NULLSAFE_FIXME[Nullable Dereference] - String path = sourceUrl.getPath().substring(1); // strip initial slash in path + String path = sourceUrl.getPath(); + if (path != null) { + path = path.substring(1); // strip initial slash in path + } String host = sourceUrl.getHost(); String scheme = sourceUrl.getProtocol(); int port = sourceUrl.getPort() != -1 ? sourceUrl.getPort() : sourceUrl.getDefaultPort(); @@ -798,7 +794,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void onFailure(Exception cause) { UiThreadUtil.runOnUiThread( DevSupportManagerBase.this::hideSplitBundleDevLoadingView); @@ -807,7 +802,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { }, bundleFile, bundleUrl, - // NULLSAFE_FIXME[Parameter Not Nullable] null); }); } @@ -836,7 +830,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public @Nullable File downloadBundleResourceFromUrlSync( final String resourceURL, final File outputFile) { return mDevServerHelper.downloadBundleResourceFromUrlSync(resourceURL, outputFile); @@ -904,7 +897,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void onFailure(final Exception cause) { hideDevLoadingView(); if (mBundleDownloadListener != null) { @@ -991,10 +983,16 @@ public abstract class DevSupportManagerBase implements DevSupportManager { // start shake gesture detector if (!mIsShakeDetectorStarted) { - mShakeDetector.start( - // NULLSAFE_FIXME[Parameter Not Nullable] - (SensorManager) mApplicationContext.getSystemService(Context.SENSOR_SERVICE)); - mIsShakeDetectorStarted = true; + SensorManager sensorManager = + (SensorManager) mApplicationContext.getSystemService(Context.SENSOR_SERVICE); + if (sensorManager != null) { + mShakeDetector.start(sensorManager); + mIsShakeDetectorStarted = true; + } else { + FLog.w( + ReactConstants.TAG, + "Couldn't register shake gesture detector, sensor service is null"); + } } // register reload app broadcast receiver @@ -1078,7 +1076,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public void setPackagerLocationCustomizer( DevSupportManager.PackagerLocationCustomizer packagerLocationCustomizer) { mPackagerLocationCustomizer = packagerLocationCustomizer; @@ -1090,7 +1087,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager { } @Override - // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation] public @Nullable SurfaceDelegate createSurfaceDelegate(String moduleName) { if (mSurfaceDelegateFactory == null) { return null; diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.kt index 5bd677e6fc8..cba0fc062d6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.kt @@ -36,11 +36,11 @@ public open class ReleaseDevSupportManager : DevSupportManager { private val defaultJSExceptionHandler: DefaultJSExceptionHandler = DefaultJSExceptionHandler() - public override fun showNewJavaError(message: String?, e: Throwable?): Unit = Unit + public override fun showNewJavaError(message: String?, e: Throwable): Unit = Unit public override fun addCustomDevOption( - optionName: String?, - optionHandler: DevOptionHandler? + optionName: String, + optionHandler: DevOptionHandler ): Unit = Unit public override fun showNewJSError( @@ -49,7 +49,7 @@ public open class ReleaseDevSupportManager : DevSupportManager { errorCookie: Int ): Unit = Unit - public override fun createRootView(appKey: String?): View? = null + public override fun createRootView(appKey: String): View? = null public override fun destroyRootView(rootView: View?): Unit = Unit @@ -111,7 +111,7 @@ public open class ReleaseDevSupportManager : DevSupportManager { public override fun downloadBundleResourceFromUrlSync( resourceURL: String, - outputFile: File? + outputFile: File ): File? = null public override val lastErrorTitle: String? @@ -125,14 +125,14 @@ public open class ReleaseDevSupportManager : DevSupportManager { public override val lastErrorCookie: Int = 0 - public override fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer?): Unit = Unit + public override fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer): Unit = Unit public override fun processErrorCustomizers( - errorInfo: Pair>? - ): Pair>? = errorInfo + errorInfo: Pair> + ): Pair> = errorInfo public override fun setPackagerLocationCustomizer( - packagerLocationCustomizer: PackagerLocationCustomizer? + packagerLocationCustomizer: PackagerLocationCustomizer ): Unit = Unit public override fun handleException(e: Exception) { @@ -145,7 +145,7 @@ public open class ReleaseDevSupportManager : DevSupportManager { public override val currentReactContext: ReactContext? get() = null - public override fun createSurfaceDelegate(moduleName: String?): SurfaceDelegate? = null + public override fun createSurfaceDelegate(moduleName: String): SurfaceDelegate? = null public override fun openDebugger(): Unit = Unit diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevBundleDownloadListener.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevBundleDownloadListener.kt index 95f6c642b7b..0dcf6a9c51d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevBundleDownloadListener.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevBundleDownloadListener.kt @@ -12,5 +12,5 @@ public interface DevBundleDownloadListener { public fun onProgress(status: String?, done: Int?, total: Int?) - public fun onFailure(cause: Exception?) + public fun onFailure(cause: Exception) } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt index 4c637b84e9b..40351ddaaa4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt @@ -38,11 +38,11 @@ public interface DevSupportManager : JSExceptionHandler { public var devSupportEnabled: Boolean - public fun showNewJavaError(message: String?, e: Throwable?) + public fun showNewJavaError(message: String?, e: Throwable) - public fun addCustomDevOption(optionName: String?, optionHandler: DevOptionHandler?) + public fun addCustomDevOption(optionName: String, optionHandler: DevOptionHandler) - public fun createRootView(appKey: String?): View? + public fun createRootView(appKey: String): View? public fun destroyRootView(rootView: View?) @@ -78,15 +78,15 @@ public interface DevSupportManager : JSExceptionHandler { public fun toggleElementInspector() - public fun downloadBundleResourceFromUrlSync(resourceURL: String, outputFile: File?): File? + public fun downloadBundleResourceFromUrlSync(resourceURL: String, outputFile: File): File? - public fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer?) + public fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer) public fun processErrorCustomizers( - errorInfo: Pair>? - ): Pair>? + errorInfo: Pair> + ): Pair> - public fun setPackagerLocationCustomizer(packagerLocationCustomizer: PackagerLocationCustomizer?) + public fun setPackagerLocationCustomizer(packagerLocationCustomizer: PackagerLocationCustomizer) /** * Create the surface delegate that the provided module should use to interact with @@ -94,7 +94,7 @@ public interface DevSupportManager : JSExceptionHandler { * @param moduleName the module name that helps decide which surface it should interact with * @return a [SurfaceDelegate] instance */ - public fun createSurfaceDelegate(moduleName: String?): SurfaceDelegate? + public fun createSurfaceDelegate(moduleName: String): SurfaceDelegate? /** Attempt to open the JS debugger on the host machine. */ public fun openDebugger()