Enable Clang Tidy (#43299)

Summary: X-link: facebook/litho#976 X-link: facebook/yoga#1586 Pull Request resolved: facebook/react-native#43299 Add the React Clang Tidy config to Yoga, run the auto fixes, and make some manual mechanical tweaks. Notably, the automatic changes to the infra for generating a Yoga tree from JSON capture make it 70% faster. Before: {F1463947076} After: {F1463946802} This also cleans up all the no-op shallow const parameters in headers. {F1463943386} Not all checks are available in all environments, but that is okay, as Clang Tidy will gracefully skip them. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D54461054 fbshipit-source-id: dbd2d9ce51afd3174d1f2c6d439fa7d08baff46f
tungdo194 · Mar 4, 2024 · 9ce360b · 9ce360b
1 parent f5c8bc1
commit 9ce360b
Show file tree

Hide file tree

Showing 25 changed files with 240 additions and 238 deletions.
diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/ScopedGlobalRef.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/ScopedGlobalRef.h
@@ -62,7 +62,7 @@ class ScopedGlobalRef {
    *
    * @param globalRef the global reference to wrap. Can be NULL.
    */
-  ScopedGlobalRef(T globalRef) : mGlobalRef(globalRef) {}
+  explicit ScopedGlobalRef(T globalRef) : mGlobalRef(globalRef) {}
 
   /**
    * Equivalent to ScopedGlobalRef(NULL)
@@ -72,12 +72,12 @@ class ScopedGlobalRef {
   /**
    * Move construction is allowed.
    */
-  ScopedGlobalRef(ScopedGlobalRef&& s) : mGlobalRef(s.release()) {}
+  ScopedGlobalRef(ScopedGlobalRef&& s) noexcept : mGlobalRef(s.release()) {}
 
   /**
    * Move assignment is allowed.
    */
-  ScopedGlobalRef& operator=(ScopedGlobalRef&& s) {
+  ScopedGlobalRef& operator=(ScopedGlobalRef&& s) noexcept {
     reset(s.release());
     return *this;
   }

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/ScopedLocalRef.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/ScopedLocalRef.h
@@ -70,12 +70,13 @@ class ScopedLocalRef {
   /**
    * Move construction is allowed.
    */
-  ScopedLocalRef(ScopedLocalRef&& s) : mEnv(s.mEnv), mLocalRef(s.release()) {}
+  ScopedLocalRef(ScopedLocalRef&& s) noexcept
+      : mEnv(s.mEnv), mLocalRef(s.release()) {}
 
   /**
    * Move assignment is allowed.
    */
-  ScopedLocalRef& operator=(ScopedLocalRef&& s) {
+  ScopedLocalRef& operator=(ScopedLocalRef&& s) noexcept {
     reset(s.release());
     mEnv = s.mEnv;
     return *this;

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.h
@@ -36,7 +36,7 @@ class YGNodeEdges {
     BORDER = 4,
   };
 
-  YGNodeEdges(YGNodeRef node) {
+  explicit YGNodeEdges(YGNodeRef node) {
     auto context = YGNodeContext{};
     context.asVoidPtr = YGNodeGetContext(node);
     edges_ = context.edgesSet;

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp
@@ -54,7 +54,9 @@ static void jni_YGConfigSetExperimentalFeatureEnabledJNI(
     jboolean enabled) {
   const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
   YGConfigSetExperimentalFeatureEnabled(
-      config, static_cast<YGExperimentalFeature>(feature), enabled);
+      config,
+      static_cast<YGExperimentalFeature>(feature),
+      static_cast<bool>(enabled));
 }
 
 static void jni_YGConfigSetUseWebDefaultsJNI(
@@ -63,7 +65,7 @@ static void jni_YGConfigSetUseWebDefaultsJNI(
     jlong nativePointer,
     jboolean useWebDefaults) {
   const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
-  YGConfigSetUseWebDefaults(config, useWebDefaults);
+  YGConfigSetUseWebDefaults(config, static_cast<bool>(useWebDefaults));
 }
 
 static void jni_YGConfigSetPointScaleFactorJNI(
@@ -161,7 +163,7 @@ static void jni_YGConfigSetLoggerJNI(
   auto context =
       reinterpret_cast<ScopedGlobalRef<jobject>*>(YGConfigGetContext(config));
 
-  if (logger) {
+  if (logger != nullptr) {
     if (context == nullptr) {
       context = new ScopedGlobalRef<jobject>();
       YGConfigSetContext(config, context);
@@ -225,14 +227,15 @@ static void jni_YGNodeSetIsReferenceBaselineJNI(
     jlong nativePointer,
     jboolean isReferenceBaseline) {
   YGNodeSetIsReferenceBaseline(
-      _jlong2YGNodeRef(nativePointer), isReferenceBaseline);
+      _jlong2YGNodeRef(nativePointer), static_cast<bool>(isReferenceBaseline));
 }
 
 static jboolean jni_YGNodeIsReferenceBaselineJNI(
     JNIEnv* /*env*/,
     jobject /*obj*/,
     jlong nativePointer) {
-  return YGNodeIsReferenceBaseline(_jlong2YGNodeRef(nativePointer));
+  return static_cast<jboolean>(
+      YGNodeIsReferenceBaseline(_jlong2YGNodeRef(nativePointer)));
 }
 
 static void jni_YGNodeRemoveAllChildrenJNI(
@@ -340,7 +343,7 @@ static void jni_YGNodeCalculateLayoutJNI(
   try {
     PtrJNodeMapVanilla* layoutContext = nullptr;
     auto map = PtrJNodeMapVanilla{};
-    if (nativePointers) {
+    if (nativePointers != nullptr) {
       map = PtrJNodeMapVanilla{nativePointers, javaNodes};
       layoutContext = &map;
     }
@@ -356,7 +359,7 @@ static void jni_YGNodeCalculateLayoutJNI(
     YGTransferLayoutOutputsRecursive(env, obj, root);
   } catch (const YogaJniException& jniException) {
     ScopedLocalRef<jthrowable> throwable = jniException.getThrowable();
-    if (throwable.get()) {
+    if (throwable.get() != nullptr) {
       env->Throw(throwable.get());
     }
   } catch (const std::logic_error& ex) {
@@ -626,8 +629,8 @@ static YGSize YGJNIMeasureFunc(
 
     uint32_t wBits = 0xFFFFFFFF & (measureResult >> 32);
     uint32_t hBits = 0xFFFFFFFF & measureResult;
-    float measuredWidth = std::bit_cast<float>(wBits);
-    float measuredHeight = std::bit_cast<float>(hBits);
+    auto measuredWidth = std::bit_cast<float>(wBits);
+    auto measuredHeight = std::bit_cast<float>(hBits);
 
     return YGSize{measuredWidth, measuredHeight};
   } else {
@@ -645,7 +648,7 @@ static void jni_YGNodeSetHasMeasureFuncJNI(
     jboolean hasMeasureFunc) {
   YGNodeSetMeasureFunc(
       _jlong2YGNodeRef(nativePointer),
-      hasMeasureFunc ? YGJNIMeasureFunc : nullptr);
+      static_cast<bool>(hasMeasureFunc) ? YGJNIMeasureFunc : nullptr);
 }
 
 static float YGJNIBaselineFunc(YGNodeConstRef node, float width, float height) {
@@ -669,7 +672,7 @@ static void jni_YGNodeSetHasBaselineFuncJNI(
     jboolean hasBaselineFunc) {
   YGNodeSetBaselineFunc(
       _jlong2YGNodeRef(nativePointer),
-      hasBaselineFunc ? YGJNIBaselineFunc : nullptr);
+      static_cast<bool>(hasBaselineFunc) ? YGJNIBaselineFunc : nullptr);
 }
 
 static void jni_YGNodeSetAlwaysFormsContainingBlockJNI(
@@ -678,7 +681,8 @@ static void jni_YGNodeSetAlwaysFormsContainingBlockJNI(
     jlong nativePointer,
     jboolean alwaysFormsContainingBlock) {
   YGNodeSetAlwaysFormsContainingBlock(
-      _jlong2YGNodeRef(nativePointer), alwaysFormsContainingBlock);
+      _jlong2YGNodeRef(nativePointer),
+      static_cast<bool>(alwaysFormsContainingBlock));
 }
 
 static jlong

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YogaJniException.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YogaJniException.cpp
@@ -25,7 +25,7 @@ YogaJniException::YogaJniException(jthrowable throwable) {
   throwable_ = newGlobalRef(getCurrentEnv(), throwable);
 }
 
-YogaJniException::YogaJniException(YogaJniException&& rhs)
+YogaJniException::YogaJniException(YogaJniException&& rhs) noexcept
     : throwable_(std::move(rhs.throwable_)) {}
 
 YogaJniException::YogaJniException(const YogaJniException& rhs) {

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YogaJniException.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YogaJniException.h
@@ -22,9 +22,9 @@ class YogaJniException : public std::exception {
 
   explicit YogaJniException(jthrowable throwable);
 
-  YogaJniException(YogaJniException&& rhs);
+  YogaJniException(YogaJniException&& rhs) noexcept;
 
-  YogaJniException(const YogaJniException& other);
+  YogaJniException(const YogaJniException& rhs);
 
   ScopedLocalRef<jthrowable> getThrowable() const noexcept;
 

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/common.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/common.cpp
@@ -16,7 +16,7 @@ void registerNatives(
     size_t numMethods) {
   jclass clazz = env->FindClass(className);
 
-  assertNoPendingJniExceptionIf(env, !clazz);
+  assertNoPendingJniExceptionIf(env, clazz == nullptr);
 
   auto result =
       env->RegisterNatives(clazz, methods, static_cast<int32_t>(numMethods));
@@ -31,7 +31,7 @@ jmethodID getStaticMethodId(
     const char* methodDescriptor) {
   jmethodID methodId =
       env->GetStaticMethodID(clazz, methodName, methodDescriptor);
-  assertNoPendingJniExceptionIf(env, !methodId);
+  assertNoPendingJniExceptionIf(env, methodId == nullptr);
   return methodId;
 }
 
@@ -41,7 +41,7 @@ jmethodID getMethodId(
     const char* methodName,
     const char* methodDescriptor) {
   jmethodID methodId = env->GetMethodID(clazz, methodName, methodDescriptor);
-  assertNoPendingJniExceptionIf(env, !methodId);
+  assertNoPendingJniExceptionIf(env, methodId == nullptr);
   return methodId;
 }
 
@@ -51,7 +51,7 @@ jfieldID getFieldId(
     const char* fieldName,
     const char* fieldSignature) {
   jfieldID fieldId = env->GetFieldID(clazz, fieldName, fieldSignature);
-  assertNoPendingJniExceptionIf(env, !fieldId);
+  assertNoPendingJniExceptionIf(env, fieldId == nullptr);
   return fieldId;
 }
 
@@ -82,24 +82,24 @@ callStaticObjectMethod(JNIEnv* env, jclass clazz, jmethodID methodId, ...) {
   va_start(args, methodId);
   jobject result = env->CallStaticObjectMethodV(clazz, methodId, args);
   va_end(args);
-  assertNoPendingJniExceptionIf(env, !result);
+  assertNoPendingJniExceptionIf(env, result == nullptr);
   return make_local_ref(env, result);
 }
 
 ScopedGlobalRef<jobject> newGlobalRef(JNIEnv* env, jobject obj) {
   jobject result = env->NewGlobalRef(obj);
 
-  if (!result) {
+  if (result == nullptr) {
     logErrorMessageAndDie("Could not obtain global reference from object");
   }
 
   return make_global_ref(result);
 }
 
 ScopedGlobalRef<jthrowable> newGlobalRef(JNIEnv* env, jthrowable obj) {
-  jthrowable result = static_cast<jthrowable>(env->NewGlobalRef(obj));
+  auto result = static_cast<jthrowable>(env->NewGlobalRef(obj));
 
-  if (!result) {
+  if (result == nullptr) {
     logErrorMessageAndDie("Could not obtain global reference from object");
   }
 

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/corefunctions.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/corefunctions.cpp
@@ -12,9 +12,9 @@
 namespace facebook::yoga::vanillajni {
 
 namespace {
-JavaVM* globalVm = NULL;
+JavaVM* globalVm = nullptr;
 struct JavaVMInitializer {
-  JavaVMInitializer(JavaVM* vm) {
+  explicit JavaVMInitializer(JavaVM* vm) {
     if (!vm) {
       logErrorMessageAndDie(
           "You cannot pass a NULL JavaVM to ensureInitialized");
@@ -27,7 +27,7 @@ struct JavaVMInitializer {
 jint ensureInitialized(JNIEnv** env, JavaVM* vm) {
   static JavaVMInitializer init(vm);
 
-  if (!env) {
+  if (env == nullptr) {
     logErrorMessageAndDie(
         "Need to pass a valid JNIEnv pointer to vanillajni initialization "
         "routine");
@@ -43,7 +43,7 @@ jint ensureInitialized(JNIEnv** env, JavaVM* vm) {
 
 // TODO why we need JNIEXPORT for getCurrentEnv ?
 JNIEXPORT JNIEnv* getCurrentEnv() {
-  JNIEnv* env;
+  JNIEnv* env = nullptr;
   jint ret = globalVm->GetEnv((void**)&env, JNI_VERSION_1_6);
   if (ret != JNI_OK) {
     logErrorMessageAndDie(
@@ -68,7 +68,7 @@ void assertNoPendingJniException(JNIEnv* env) {
   }
 
   auto throwable = env->ExceptionOccurred();
-  if (!throwable) {
+  if (throwable == nullptr) {
     logErrorMessageAndDie("Unable to get pending JNI exception.");
   }
   env->ExceptionClear();

diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/yogajni.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/yogajni.cpp
@@ -10,8 +10,8 @@
 
 using namespace facebook::yoga;
 
-jint JNI_OnLoad(JavaVM* vm, void*) {
-  JNIEnv* env;
+jint JNI_OnLoad(JavaVM* vm, void* /*unused*/) {
+  JNIEnv* env = nullptr;
   jint ret = vanillajni::ensureInitialized(&env, vm);
   YGJNIVanilla::registerNatives(env);
   return ret;

diff --git a/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp b/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp
@@ -216,12 +216,12 @@ void YGNodeSetChildren(
   auto owner = resolveRef(ownerRef);
   auto children = reinterpret_cast<yoga::Node* const*>(childrenRefs);
 
-  if (!owner) {
+  if (owner == nullptr) {
     return;
   }
 
   const std::vector<yoga::Node*> childrenVector = {children, children + count};
-  if (childrenVector.size() == 0) {
+  if (childrenVector.empty()) {
     if (owner->getChildCount() > 0) {
       for (auto* child : owner->getChildren()) {
         child->setLayout({});

diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/AbsoluteLayout.h b/packages/react-native/ReactCommon/yoga/yoga/algorithm/AbsoluteLayout.h
@@ -13,16 +13,16 @@
 namespace facebook::yoga {
 
 void layoutAbsoluteChild(
-    const yoga::Node* const containingNode,
-    const yoga::Node* const node,
-    yoga::Node* const child,
-    const float containingBlockWidth,
-    const float containingBlockHeight,
-    const SizingMode widthMode,
-    const Direction direction,
+    const yoga::Node* containingNode,
+    const yoga::Node* node,
+    yoga::Node* child,
+    float containingBlockWidth,
+    float containingBlockHeight,
+    SizingMode widthMode,
+    Direction direction,
     LayoutData& layoutMarkerData,
-    const uint32_t depth,
-    const uint32_t generationCount);
+    uint32_t depth,
+    uint32_t generationCount);
 
 void layoutAbsoluteDescendants(
     yoga::Node* containingNode,