Clang-tidy fabric/jni package

Summary:
Applies suggestions from clang-tidy. The automatic linter seems to be broken atm, so I used default config from Android Studio, filtering out meaningless suggestions.

Changelog: [Internal]

Reviewed By: cortinico, sammy-SC

Differential Revision: D32994138

fbshipit-source-id: e8edf53f0cb8b0eda4ff8bb8bf8f5196827efd68

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/AsyncEventBeat.h

@@ -25,8 +25,8 @@ class AsyncEventBeat final : public EventBeat, public EventBeatManagerObserver {
       jni::global_ref<jobject> javaUIManager)
       : EventBeat(ownerBox),
         eventBeatManager_(eventBeatManager),
-        runtimeExecutor_(runtimeExecutor),
-        javaUIManager_(javaUIManager) {
+        runtimeExecutor_(std::move(runtimeExecutor)),
+        javaUIManager_(std::move(javaUIManager)) {
     eventBeatManager->addObserver(*this);
   }
 

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/AsyncEventBeatV2.cpp

@@ -20,8 +20,8 @@ AsyncEventBeatV2::AsyncEventBeatV2(
     jni::global_ref<jobject> javaUIManager)
     : EventBeat(ownerBox),
       eventBeatManager_(eventBeatManager),
-      runtimeExecutor_(runtimeExecutor),
-      javaUIManager_(javaUIManager) {
+      runtimeExecutor_(std::move(runtimeExecutor)),
+      javaUIManager_(std::move(javaUIManager)) {
   eventBeatManager->addObserver(*this);
 }
 

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp

@@ -48,61 +48,61 @@ struct JMountItem : public JavaClass<JMountItem> {
 
 } // namespace
 
-CppMountItem CppMountItem::CreateMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::CreateMountItem(ShadowView const &shadowView) {
   return {CppMountItem::Type::Create, {}, {}, shadowView, -1};
 }
-CppMountItem CppMountItem::DeleteMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::DeleteMountItem(ShadowView const &shadowView) {
   return {CppMountItem::Type::Delete, {}, shadowView, {}, -1};
 }
 CppMountItem CppMountItem::InsertMountItem(
-    ShadowView parentView,
-    ShadowView shadowView,
+    ShadowView const &parentView,
+    ShadowView const &shadowView,
     int index) {
   return {CppMountItem::Type::Insert, parentView, {}, shadowView, index};
 }
 CppMountItem CppMountItem::RemoveMountItem(
-    ShadowView parentView,
-    ShadowView shadowView,
+    ShadowView const &parentView,
+    ShadowView const &shadowView,
     int index) {
   return {CppMountItem::Type::Remove, parentView, shadowView, {}, index};
 }
-CppMountItem CppMountItem::UpdatePropsMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::UpdatePropsMountItem(ShadowView const &shadowView) {
   return {CppMountItem::Type::UpdateProps, {}, {}, shadowView, -1};
 }
-CppMountItem CppMountItem::UpdateStateMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::UpdateStateMountItem(ShadowView const &shadowView) {
   return {CppMountItem::Type::UpdateState, {}, {}, shadowView, -1};
 }
-CppMountItem CppMountItem::UpdateLayoutMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::UpdateLayoutMountItem(ShadowView const &shadowView) {
   return {CppMountItem::Type::UpdateLayout, {}, {}, shadowView, -1};
 }
-CppMountItem CppMountItem::UpdateEventEmitterMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::UpdateEventEmitterMountItem(
+    ShadowView const &shadowView) {
   return {CppMountItem::Type::UpdateEventEmitter, {}, {}, shadowView, -1};
 }
-CppMountItem CppMountItem::UpdatePaddingMountItem(ShadowView shadowView) {
+CppMountItem CppMountItem::UpdatePaddingMountItem(
+    ShadowView const &shadowView) {
   return {CppMountItem::Type::UpdatePadding, {}, {}, shadowView, -1};
 }
 
 static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
-  if (mountItemType == CppMountItem::Type::Create) {
-    return 2; // tag, isLayoutable
-  } else if (mountItemType == CppMountItem::Type::Insert) {
-    return 3; // tag, parentTag, index
-  } else if (mountItemType == CppMountItem::Type::Remove) {
-    return 3; // tag, parentTag, index
-  } else if (mountItemType == CppMountItem::Type::Delete) {
-    return 1; // tag
-  } else if (mountItemType == CppMountItem::Type::UpdateProps) {
-    return 1; // tag
-  } else if (mountItemType == CppMountItem::Type::UpdateState) {
-    return 1; // tag
-  } else if (mountItemType == CppMountItem::Type::UpdatePadding) {
-    return 5; // tag, top, left, bottom, right
-  } else if (mountItemType == CppMountItem::Type::UpdateLayout) {
-    return 6; // tag, x, y, w, h, DisplayType
-  } else if (mountItemType == CppMountItem::Type::UpdateEventEmitter) {
-    return 1; // tag
-  } else {
-    return -1;
+  switch (mountItemType) {
+    case CppMountItem::Type::Create:
+      return 2; // tag, isLayoutable
+    case CppMountItem::Type::Insert:
+    case CppMountItem::Type::Remove:
+      return 3; // tag, parentTag, index
+    case CppMountItem::Type::Delete:
+    case CppMountItem::Type::UpdateProps:
+    case CppMountItem::Type::UpdateState:
+    case CppMountItem::Type::UpdateEventEmitter:
+      return 1; // tag
+    case CppMountItem::Type::UpdatePadding:
+      return 5; // tag, top, left, bottom, right
+    case CppMountItem::Type::UpdateLayout:
+      return 6; // tag, x, y, w, h, DisplayType
+    case CppMountItem::Undefined:
+    case CppMountItem::Multiple:
+      return -1;
   }
 }
 
@@ -254,7 +254,7 @@ Binding::getInspectorDataForInstance(
   result["selectedIndex"] = data.selectedIndex;
   result["props"] = data.props;
   auto hierarchy = folly::dynamic::array();
-  for (auto hierarchyItem : data.hierarchy) {
+  for (const auto &hierarchyItem : data.hierarchy) {
     hierarchy.push_back(hierarchyItem);
   }
   result["hierarchy"] = hierarchy;
@@ -634,12 +634,12 @@ void Binding::uninstallFabricUIManager() {
 }
 
 inline local_ref<ReadableMap::javaobject> castReadableMap(
-    local_ref<ReadableNativeMap::javaobject> nativeMap) {
+    local_ref<ReadableNativeMap::javaobject> const &nativeMap) {
   return make_local(reinterpret_cast<ReadableMap::javaobject>(nativeMap.get()));
 }
 
 inline local_ref<ReadableArray::javaobject> castReadableArray(
-    local_ref<ReadableNativeArray::javaobject> nativeArray) {
+    local_ref<ReadableNativeArray::javaobject> const &nativeArray) {
   return make_local(
       reinterpret_cast<ReadableArray::javaobject>(nativeArray.get()));
 }
@@ -649,7 +649,7 @@ local_ref<JString> getPlatformComponentName(const ShadowView &shadowView) {
   static std::string scrollViewComponentName = std::string("ScrollView");
 
   local_ref<JString> componentName;
-  if (scrollViewComponentName.compare(shadowView.componentName) == 0) {
+  if (scrollViewComponentName == shadowView.componentName) {
     auto newViewProps =
         std::static_pointer_cast<const ScrollViewProps>(shadowView.props);
     if (newViewProps->getProbablyMoreHorizontalThanVertical_DEPRECATED()) {
@@ -956,7 +956,7 @@ void Binding::schedulerDidFinishTransaction(
       LOG(ERROR) << "Unexpected CppMountItem type";
     }
   }
-  if (cppUpdatePropsMountItems.size() > 0) {
+  if (!cppUpdatePropsMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::UpdateProps,
         cppUpdatePropsMountItems.size(),
@@ -975,7 +975,7 @@ void Binding::schedulerDidFinishTransaction(
       (*objBufferArray)[objBufferPosition++] = newPropsReadableMap.get();
     }
   }
-  if (cppUpdateStateMountItems.size() > 0) {
+  if (!cppUpdateStateMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::UpdateState,
         cppUpdateStateMountItems.size(),
@@ -1003,7 +1003,7 @@ void Binding::schedulerDidFinishTransaction(
           (javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr);
     }
   }
-  if (cppUpdatePaddingMountItems.size() > 0) {
+  if (!cppUpdatePaddingMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::UpdatePadding,
         cppUpdatePaddingMountItems.size(),
@@ -1030,7 +1030,7 @@ void Binding::schedulerDidFinishTransaction(
       intBufferPosition += 5;
     }
   }
-  if (cppUpdateLayoutMountItems.size() > 0) {
+  if (!cppUpdateLayoutMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::UpdateLayout,
         cppUpdateLayoutMountItems.size(),
@@ -1043,10 +1043,10 @@ void Binding::schedulerDidFinishTransaction(
       auto pointScaleFactor = layoutMetrics.pointScaleFactor;
       auto frame = layoutMetrics.frame;
 
-      int x = round(scale(frame.origin.x, pointScaleFactor));
-      int y = round(scale(frame.origin.y, pointScaleFactor));
-      int w = round(scale(frame.size.width, pointScaleFactor));
-      int h = round(scale(frame.size.height, pointScaleFactor));
+      int x = (int)round(scale(frame.origin.x, pointScaleFactor));
+      int y = (int)round(scale(frame.origin.y, pointScaleFactor));
+      int w = (int)round(scale(frame.size.width, pointScaleFactor));
+      int h = (int)round(scale(frame.size.height, pointScaleFactor));
       int displayType =
           toInt(mountItem.newChildShadowView.layoutMetrics.displayType);
 
@@ -1060,7 +1060,7 @@ void Binding::schedulerDidFinishTransaction(
       intBufferPosition += 6;
     }
   }
-  if (cppUpdateEventEmitterMountItems.size() > 0) {
+  if (!cppUpdateEventEmitterMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::UpdateEventEmitter,
         cppUpdateEventEmitterMountItems.size(),
@@ -1094,7 +1094,7 @@ void Binding::schedulerDidFinishTransaction(
   // requires that the differ never produces "DELETE...CREATE" in that order for
   // the same tag. It's nice to be able to batch all similar operations together
   // for space efficiency.
-  if (cppDeleteMountItems.size() > 0) {
+  if (!cppDeleteMountItems.empty()) {
     writeIntBufferTypePreamble(
         CppMountItem::Type::Delete,
         cppDeleteMountItems.size(),

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h

@@ -31,17 +31,21 @@ class Instance;
 
 struct CppMountItem final {
 #pragma mark - Designated Initializers
-  static CppMountItem CreateMountItem(ShadowView shadowView);
-  static CppMountItem DeleteMountItem(ShadowView shadowView);
-  static CppMountItem
-  InsertMountItem(ShadowView parentView, ShadowView shadowView, int index);
-  static CppMountItem
-  RemoveMountItem(ShadowView parentView, ShadowView shadowView, int index);
-  static CppMountItem UpdatePropsMountItem(ShadowView shadowView);
-  static CppMountItem UpdateStateMountItem(ShadowView shadowView);
-  static CppMountItem UpdateLayoutMountItem(ShadowView shadowView);
-  static CppMountItem UpdateEventEmitterMountItem(ShadowView shadowView);
-  static CppMountItem UpdatePaddingMountItem(ShadowView shadowView);
+  static CppMountItem CreateMountItem(ShadowView const &shadowView);
+  static CppMountItem DeleteMountItem(ShadowView const &shadowView);
+  static CppMountItem InsertMountItem(
+      ShadowView const &parentView,
+      ShadowView const &shadowView,
+      int index);
+  static CppMountItem RemoveMountItem(
+      ShadowView const &parentView,
+      ShadowView const &shadowView,
+      int index);
+  static CppMountItem UpdatePropsMountItem(ShadowView const &shadowView);
+  static CppMountItem UpdateStateMountItem(ShadowView const &shadowView);
+  static CppMountItem UpdateLayoutMountItem(ShadowView const &shadowView);
+  static CppMountItem UpdateEventEmitterMountItem(ShadowView const &shadowView);
+  static CppMountItem UpdatePaddingMountItem(ShadowView const &shadowView);
 
 #pragma mark - Type
 
@@ -178,9 +182,8 @@ class Binding : public jni::HybridClass<Binding>,
   std::mutex javaUIManagerMutex_;
 
   // LayoutAnimations
-  virtual void onAnimationStarted() override;
-  virtual void onAllAnimationsComplete() override;
-  LayoutAnimationDriver *getAnimationDriver();
+  void onAnimationStarted() override;
+  void onAllAnimationsComplete() override;
   std::shared_ptr<LayoutAnimationDriver> animationDriver_;
   std::unique_ptr<JBackgroundExecutor> backgroundExecutor_;
 

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/CoreComponentsRegistry.h

@@ -23,7 +23,7 @@ class CoreComponentsRegistry
 
   static void registerNatives();
 
-  CoreComponentsRegistry(ComponentFactory *delegate);
+  explicit CoreComponentsRegistry(ComponentFactory *delegate);
 
   static std::shared_ptr<ComponentDescriptorProviderRegistry const>
   sharedProviderRegistry();

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventBeatManager.h

@@ -35,7 +35,8 @@ class EventBeatManager : public jni::HybridClass<EventBeatManager> {
 
   static void registerNatives();
 
-  EventBeatManager(jni::alias_ref<EventBeatManager::jhybriddata> jhybridobject);
+  explicit EventBeatManager(
+      jni::alias_ref<EventBeatManager::jhybriddata> jhybridobject);
 
   /*
    * Adds (or removes) observers.

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.cpp

@@ -19,12 +19,12 @@ EventEmitterWrapper::initHybrid(jni::alias_ref<jclass>) {
 }
 
 void EventEmitterWrapper::invokeEvent(
-    std::string eventName,
+    std::string const &eventName,
     NativeMap *payload,
     int category) {
   if (eventEmitterPointer) {
     eventEmitterPointer->dispatchEvent(
-        std::move(eventName),
+        eventName,
         payload->consume(),
         EventPriority::AsynchronousBatched,
         static_cast<RawEvent::Category>(category));
@@ -44,12 +44,11 @@ void EventEmitterWrapper::invokeEvent(
 }
 
 void EventEmitterWrapper::invokeUniqueEvent(
-    std::string eventName,
+    std::string const &eventName,
     NativeMap *payload,
     int customCoalesceKey) {
   if (eventEmitterPointer) {
-    eventEmitterPointer->dispatchUniqueEvent(
-        std::move(eventName), payload->consume());
+    eventEmitterPointer->dispatchUniqueEvent(eventName, payload->consume());
     return;
   }
   // TODO: customCoalesceKey currently unused

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/EventEmitterWrapper.h

@@ -26,9 +26,10 @@ class EventEmitterWrapper : public jni::HybridClass<EventEmitterWrapper> {
   SharedEventEmitter eventEmitter;
   EventEmitter const *eventEmitterPointer;
 
-  void invokeEvent(std::string eventName, NativeMap *params, int category);
+  void
+  invokeEvent(std::string const &eventName, NativeMap *params, int category);
   void invokeUniqueEvent(
-      std::string eventName,
+      std::string const &eventName,
       NativeMap *params,
       int customCoalesceKey);
 

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/ReactNativeConfigHolder.h

@@ -22,7 +22,7 @@ namespace react {
  */
 class ReactNativeConfigHolder : public ReactNativeConfig {
  public:
-  ReactNativeConfigHolder(jni::alias_ref<jobject> reactNativeConfig)
+  explicit ReactNativeConfigHolder(jni::alias_ref<jobject> reactNativeConfig)
       : reactNativeConfig_(make_global(reactNativeConfig)){};
 
   bool getBool(const std::string &param) const override;

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/StateWrapperImpl.h

@@ -29,10 +29,6 @@ class StateWrapperImpl : public jni::HybridClass<StateWrapperImpl> {
   jni::local_ref<ReadableMapBuffer::jhybridobject> getStateMapBufferDataImpl();
   jni::local_ref<ReadableNativeMap::jhybridobject> getStateDataImpl();
   void updateStateImpl(NativeMap *map);
-  void updateStateWithFailureCallbackImpl(
-      NativeMap *map,
-      jni::alias_ref<jobject> self,
-      int callbackRefId);
 
   State::Shared state_;
 

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/SurfaceHandlerBinding.h

@@ -28,9 +28,6 @@ class SurfaceHandlerBinding : public jni::HybridClass<SurfaceHandlerBinding> {
 
   void setDisplayMode(jint mode);
 
-  void registerScheduler(std::shared_ptr<Scheduler> scheduler);
-  void unregisterScheduler(std::shared_ptr<Scheduler> scheduler);
-
   jint getSurfaceId();
   void setSurfaceId(jint surfaceId);
   jni::local_ref<jstring> getModuleName();