Skip to content

Kotlin: Clean up redundant constructs (2/n) #51170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

Kotlin: Clean up redundant constructs (2/n) #51170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
af5f5a9
Clean up: Redundant nullable return type
mateoguzmana May 7, 2025
b545f92
Clean up: Redundant empty construct
mateoguzmana May 7, 2025
99a7afb
Clean up: Replace empty class body
mateoguzmana May 7, 2025
53e96bb
Clean up: Unused import directive
mateoguzmana May 7, 2025
c960295
Clean up: Redundant SAM constructor
mateoguzmana May 7, 2025
b618923
Make reactChoreographer val nullable again
mateoguzmana May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-native/ReactAndroid/src/main/java/com/facebook/react/TurboReactPackage.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ package com.facebook.react
@Deprecated(
message = "Use BaseReactPackage instead",
replaceWith = ReplaceWith(expression = "BaseReactPackage"))
public abstract class TurboReactPackage : BaseReactPackage() {}
public abstract class TurboReactPackage : BaseReactPackage()
2 changes: 1 addition & 1 deletion ...src/main/java/com/facebook/react/common/annotations/internal/InteropLegacyArchitecture.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ package com.facebook.react.common.annotations.internal
*/
@Retention(AnnotationRetention.SOURCE)
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
public annotation class InteropLegacyArchitecture()
public annotation class InteropLegacyArchitecture
2 changes: 1 addition & 1 deletion ...ct-native/ReactAndroid/src/main/java/com/facebook/react/fabric/events/EventBeatManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import com.facebook.react.uimanager.events.BatchEventDispatchedListener
*/
@DoNotStrip
@SuppressLint("MissingNativeLoadLibrary")
internal final class EventBeatManager() : HybridClassBase(), BatchEventDispatchedListener {
internal class EventBeatManager : HybridClassBase(), BatchEventDispatchedListener {
init {
initHybrid()
}
Expand Down
2 changes: 0 additions & 2 deletions ...react-native/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
package com.facebook.react.modules.dialog

import android.content.DialogInterface
import android.content.DialogInterface.OnClickListener
import android.content.DialogInterface.OnDismissListener
import android.os.Bundle
import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.FragmentManager
Expand Down
27 changes: 13 additions & 14 deletions ...s/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/toast/ToastModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ internal class ToastModule(reactContext: ReactApplicationContext) :

override fun show(message: String?, durationDouble: Double) {
val duration = durationDouble.toInt()
UiThreadUtil.runOnUiThread(
Runnable { Toast.makeText(getReactApplicationContext(), message, duration).show() })
UiThreadUtil.runOnUiThread {
Toast.makeText(getReactApplicationContext(), message, duration).show()
}
}

override fun showWithGravity(message: String?, durationDouble: Double, gravityDouble: Double) {
val duration = durationDouble.toInt()
val gravity = gravityDouble.toInt()
UiThreadUtil.runOnUiThread(
Runnable {
val toast = Toast.makeText(getReactApplicationContext(), message, duration)
toast.setGravity(gravity, 0, 0)
toast.show()
})
UiThreadUtil.runOnUiThread {
val toast = Toast.makeText(getReactApplicationContext(), message, duration)
toast.setGravity(gravity, 0, 0)
toast.show()
}
}

override fun showWithGravityAndOffset(
Expand All @@ -57,12 +57,11 @@ internal class ToastModule(reactContext: ReactApplicationContext) :
val gravity = gravityDouble.toInt()
val xOffset = xOffsetDouble.toInt()
val yOffset = yOffsetDouble.toInt()
UiThreadUtil.runOnUiThread(
Runnable {
val toast = Toast.makeText(getReactApplicationContext(), message, duration)
toast.setGravity(gravity, xOffset, yOffset)
toast.show()
})
UiThreadUtil.runOnUiThread {
val toast = Toast.makeText(getReactApplicationContext(), message, duration)
toast.setGravity(gravity, xOffset, yOffset)
toast.show()
}
}

companion object {
Expand Down
4 changes: 2 additions & 2 deletions ...ative/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessCatalystInstance.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,14 @@ internal class BridgelessCatalystInstance(private val reactHost: ReactHostImpl)
public override val runtimeExecutor: RuntimeExecutor?
get() = reactHost.getRuntimeExecutor()

public override val runtimeScheduler: RuntimeScheduler?
public override val runtimeScheduler: RuntimeScheduler
get() = throw UnsupportedOperationException("Unimplemented method 'getRuntimeScheduler'")

public override fun extendNativeModules(modules: NativeModuleRegistry) {
throw UnsupportedOperationException("Unimplemented method 'extendNativeModules'")
}

public override val sourceURL: String?
public override val sourceURL: String
get() = throw UnsupportedOperationException("Unimplemented method 'getSourceURL'")

override fun addBridgeIdleDebugListener(listener: NotThreadSafeBridgeIdleDebugListener) {
Expand Down
2 changes: 1 addition & 1 deletion ...ct-native/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessReactContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ internal class BridgelessReactContext(context: Context, private val reactHost: R
override fun <T : NativeModule> hasNativeModule(nativeModuleInterface: Class<T>): Boolean =
reactHost.hasNativeModule(nativeModuleInterface)

override fun getNativeModules(): MutableCollection<NativeModule>? = reactHost.nativeModules
override fun getNativeModules(): MutableCollection<NativeModule> = reactHost.nativeModules

override fun <T : NativeModule> getNativeModule(nativeModuleInterface: Class<T>): T? =
reactHost.getNativeModule(nativeModuleInterface)
Expand Down
30 changes: 15 additions & 15 deletions packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,27 +211,27 @@ internal class ReactInstance(
// 2. genericBubblingEventTypes.
// 3. genericDirectEventTypes.
// We want to match this beahavior.
DefaultEventTypesProvider {
Arguments.makeNativeMap(UIManagerModuleConstantsHelper.getDefaultExportableEventTypes())
},
{
Arguments.makeNativeMap(UIManagerModuleConstantsHelper.getDefaultExportableEventTypes())
},
ConstantsForViewManagerProvider { viewManagerName: String ->
val viewManager =
viewManagerResolver.getViewManager(viewManagerName)
?: return@ConstantsForViewManagerProvider null
getConstantsForViewManager(viewManager, customDirectEvents)
},
ConstantsProvider {
val viewManagers: List<ViewManager<*, *>> =
ArrayList(viewManagerResolver.eagerViewManagerMap.values)
val constants = createConstants(viewManagers, customDirectEvents)

val lazyViewManagers = viewManagerResolver.lazyViewManagerNames
if (!lazyViewManagers.isEmpty()) {
constants["ViewManagerNames"] = ArrayList(lazyViewManagers)
constants["LazyViewManagersEnabled"] = true
}
Arguments.makeNativeMap(constants)
})
{
val viewManagers: List<ViewManager<*, *>> =
ArrayList(viewManagerResolver.eagerViewManagerMap.values)
val constants = createConstants(viewManagers, customDirectEvents)

val lazyViewManagers = viewManagerResolver.lazyViewManagerNames
if (!lazyViewManagers.isEmpty()) {
constants["ViewManagerNames"] = ArrayList(lazyViewManagers)
constants["LazyViewManagersEnabled"] = true
}
Arguments.makeNativeMap(constants)
})
}

val eventBeatManager = EventBeatManager()
Expand Down
2 changes: 1 addition & 1 deletion ...ReactAndroid/src/main/java/com/facebook/react/runtime/internal/bolts/ExecutorException.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ package com.facebook.react.runtime.internal.bolts
* the continuation block it self.
*/
internal class ExecutorException(e: Exception?) :
RuntimeException("An exception was thrown by an Executor", e) {}
RuntimeException("An exception was thrown by an Executor", e)
2 changes: 1 addition & 1 deletion ...e/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/CallInvokerHolderImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder
* pass it from CatalystInstance, through Java, to TurboModuleManager::initHybrid.
*/
@FrameworkAPI
public class CallInvokerHolderImpl private constructor() : HybridClassBase(), CallInvokerHolder {}
public class CallInvokerHolderImpl private constructor() : HybridClassBase(), CallInvokerHolder
2 changes: 1 addition & 1 deletion ...id/src/main/java/com/facebook/react/turbomodule/core/NativeMethodCallInvokerHolderImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHol
*/
@FrameworkAPI
public class NativeMethodCallInvokerHolderImpl private constructor() :
HybridClassBase(), NativeMethodCallInvokerHolder {}
HybridClassBase(), NativeMethodCallInvokerHolder
2 changes: 1 addition & 1 deletion ...eact-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactStylesDiffMap.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class ReactStylesDiffMap(props: ReadableMap) {

public fun getMap(name: String): ReadableMap? = backingMap.getMap(name)

public fun getDynamic(name: String): Dynamic? = backingMap.getDynamic(name)
public fun getDynamic(name: String): Dynamic = backingMap.getDynamic(name)

override fun toString(): String = "{ ${javaClass.simpleName}: $backingMap }"
}
1 change: 0 additions & 1 deletion ...ative/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package com.facebook.react.uimanager
import android.view.accessibility.AccessibilityEvent
import android.widget.ImageView
import com.facebook.react.uimanager.events.TouchEventType
import com.facebook.react.uimanager.events.TouchEventType.Companion.getJSEventName

/** Constants exposed to JS from [UIManagerModule]. */
internal object UIManagerModuleConstants {
Expand Down
11 changes: 5 additions & 6 deletions ...ve/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,11 @@ public class ViewGroupDrawingOrderHelper(private val viewGroup: ViewGroup) {
}

// Sort the views by zIndex
viewsToSort.sortWith(
Comparator { view1, view2 ->
val view1ZIndex = ViewGroupManager.getViewZIndex(view1) ?: 0
val view2ZIndex = ViewGroupManager.getViewZIndex(view2) ?: 0
view1ZIndex - view2ZIndex
})
viewsToSort.sortWith { view1, view2 ->
val view1ZIndex = ViewGroupManager.getViewZIndex(view1) ?: 0
val view2ZIndex = ViewGroupManager.getViewZIndex(view2) ?: 0
view1ZIndex - view2ZIndex
}

currentDrawingOrderIndices = IntArray(childCount)
for (i in 0 until childCount) {
Expand Down
2 changes: 1 addition & 1 deletion ...ctAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerWithGeneratedInterface.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
package com.facebook.react.uimanager

/** Marker interface to be extended by all code-generated ViewManagerInterface. */
public interface ViewManagerWithGeneratedInterface {}
public interface ViewManagerWithGeneratedInterface
2 changes: 1 addition & 1 deletion ...e/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/FabricEventDispatcher.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ internal class FabricEventDispatcher(
if (reactContext.isOnUiQueueThread()) {
maybeDispatchBatchedEvents()
} else {
reactContext.runOnUiQueueThread(Runnable { maybeDispatchBatchedEvents() })
reactContext.runOnUiQueueThread { maybeDispatchBatchedEvents() }
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...ctAndroid/src/main/java/com/facebook/react/views/drawer/events/DrawerStateChangedEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal class DrawerStateChangedEvent : Event<DrawerStateChangedEvent> {

override fun getEventName(): String = EVENT_NAME

protected override fun getEventData(): WritableMap? {
protected override fun getEventData(): WritableMap {
val eventData: WritableMap = Arguments.createMap()
eventData.putInt("drawerState", getDrawerState())
return eventData
Expand Down
2 changes: 1 addition & 1 deletion ...ndroid/src/main/java/com/facebook/react/views/safeareaview/ReactSafeAreaViewShadowNode.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ package com.facebook.react.views.safeareaview
import com.facebook.react.common.annotations.internal.LegacyArchitecture
import com.facebook.react.uimanager.LayoutShadowNode

@LegacyArchitecture internal class ReactSafeAreaViewShadowNode : LayoutShadowNode() {}
@LegacyArchitecture internal class ReactSafeAreaViewShadowNode : LayoutShadowNode()
2 changes: 1 addition & 1 deletion ...ative/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ public object ReactScrollViewHelper {
}
}

public class ReactScrollViewScrollState() {
public class ReactScrollViewScrollState {

/** Get the position after current animation is finished */
public val finalAnimatedPositionScroll: Point = Point()
Expand Down
1 change: 0 additions & 1 deletion ...native/ReactAndroid/src/main/java/com/facebook/react/views/text/DefaultStyleValuesUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package com.facebook.react.views.text

import android.content.Context
import android.content.res.ColorStateList
import androidx.core.content.res.use
Copy link
Contributor

@cortinico cortinico May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mateoguzmana just a small heads up that this triggered a lot of crashes internally. Once we remove this import, we fallback to use Kotlin's use (https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.io/use.html) which is different than this one: https://developer.android.com/reference/kotlin/androidx/core/content/res/package-summary#(android.content.res.TypedArray).use(kotlin.Function1)

Specifically the app were crashing with .IncompatibleClassChangeError: Class 'android.content.res.TypedArray' does not implement interface 'java.lang.AutoCloseable'

Copy link
Collaborator Author

@mateoguzmana mateoguzmana May 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up and for fixing it @cortinico! Apologies if it caused some extra work on your side... It looked pretty harmless as it was showing as unused with static code analysis 🥲

Not sure if there is something we can do to ensure this regression doesn't happen again, but if there is, I'm happy to help with it

Copy link
Contributor

@cortinico cortinico May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there is something we can do to ensure this regression doesn't happen again, but if there is, I'm happy to help with it

As a bare minimun having a comment on top of the import would help. Otherwise we need to suppress the IDE warning with a @Suppress annotation.

It's a bit tricky for me because I don't see a warning on that import with Android Studio so I'm a bit unsure how to suppress it

Copy link
Collaborator Author

@mateoguzmana mateoguzmana May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the code analysis again, and Android Studio suggested a way to suppress it. I've done it in #51272 and also added some context about why it is needed


/** Utility class that access default values from style */
public object DefaultStyleValuesUtil {
Expand Down
2 changes: 1 addition & 1 deletion ...ReactAndroid/src/main/java/com/facebook/react/views/text/PreparedLayoutTextViewManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ internal class PreparedLayoutTextViewManager :
stateWrapper: StateWrapper
): Any? = (stateWrapper as? ReferenceStateWrapper)?.stateDataReference

override fun getExportedCustomDirectEventTypeConstants(): MutableMap<String, Any>? {
override fun getExportedCustomDirectEventTypeConstants(): MutableMap<String, Any> {
val baseEventTypeConstants = super.getExportedCustomDirectEventTypeConstants()
val eventTypeConstants = baseEventTypeConstants ?: HashMap()
eventTypeConstants.put("topTextLayout", mapOf("registrationName" to "onTextLayout"))
Expand Down
2 changes: 1 addition & 1 deletion ...ct-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactRawTextManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal class ReactRawTextManager : ViewManager<View, ReactRawTextShadowNode>()
public override fun createViewInstance(context: ThemedReactContext): ReactTextView =
throw IllegalStateException("Attempt to create a native view for RCTRawText")

override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View? =
override fun prepareToRecycleView(reactContext: ThemedReactContext, view: View): View =
throw IllegalStateException("Attempt to recycle a native view for RCTRawText")

override fun updateExtraData(view: View, extraData: Any): Unit = Unit
Expand Down
Loading