Enable Clang Tidy (#1586)

Summary:
X-link: https://github.com/facebook/litho/pull/976

Pull Request resolved: https://github.com/facebook/yoga/pull/1586

X-link: https://github.com/facebook/react-native/pull/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
This commit is contained in:
Nick Gerleman
2024-03-04 02:28:02 -08:00
committed by Facebook GitHub Bot
parent 47a56db5f6
commit b959c79a2a
43 changed files with 537 additions and 306 deletions

View File

@@ -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;
}

View File

@@ -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;

View File

@@ -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;

View File

@@ -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

View File

@@ -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) {

View File

@@ -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;

View File

@@ -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,14 +82,14 @@ 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");
}
@@ -97,9 +97,9 @@ ScopedGlobalRef<jobject> newGlobalRef(JNIEnv* env, jobject obj) {
}
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");
}

View File

@@ -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();

View File

@@ -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;