Add more clang tidy rules

Summary: changelog: [internal] Add more clang tidy rules to prevent common class of bugs. Reviewed By: javache Differential Revision: D39245194 fbshipit-source-id: 5521c5c4653d7005b96ebba494e810ba5075afbc
facebook · Sep 6, 2022 · ce50c43 · ce50c43
1 parent 010cdcd
commit ce50c43
Show file tree

Hide file tree

Showing 73 changed files with 412 additions and 331 deletions.
diff --git a/ReactCommon/react/renderer/.clang-tidy b/ReactCommon/react/renderer/.clang-tidy
@@ -40,5 +40,147 @@ performance-noexcept-move-constructor,
 performance-type-promotion-in-math-fn,
 performance-unnecessary-copy-initialization,
 performance-unnecessary-value-param,
+cppcoreguidelines-macro-usage,
+cppcoreguidelines-narrowing-conversions,
+cppcoreguidelines-no-malloc,
+cppcoreguidelines-pro-bounds-pointer-arithmetic,
+cppcoreguidelines-pro-type-const-cast,
+cppcoreguidelines-pro-type-cstyle-cast,
+cppcoreguidelines-pro-type-member-init,
+cppcoreguidelines-pro-type-reinterpret-cast,
+cppcoreguidelines-pro-type-union-access,
+cppcoreguidelines-pro-type-vararg,
+cppcoreguidelines-slicing,
+cppcoreguidelines-special-member-functions,
+readability-avoid-const-params-in-decls,
+readability-braces-around-statements,
+readability-const-return-type,
+readability-container-size-empty,
+readability-deleted-default,
+readability-delete-null-pointer,
+readability-implicit-bool-conversion,
+readability-inconsistent-declaration-parameter-name,
+readability-isolate-declaration,
+readability-misplaced-array-index,
+readability-named-parameter,
+readability-non-const-parameter,
+readability-redundant-control-flow,
+readability-redundant-declaration,
+readability-redundant-function-ptr-dereference,
+readability-redundant-preprocessor,
+readability-redundant-smartptr-get,
+readability-redundant-string-cstr,
+readability-redundant-string-init,
+readability-simplify-boolean-expr,
+readability-simplify-subscript-expr,
+readability-static-accessed-through-instance,
+readability-static-definition-in-anonymous-namespace,
+readability-string-compare,
+readability-uniqueptr-delete-release,
+misc-definitions-in-headers,
+misc-new-delete-overloads,
+misc-non-copyable-objects,
+misc-static-assert,
+misc-throw-by-value-catch-by-reference,
+misc-unconventional-assign-operator,
+misc-uniqueptr-reset-release,
+misc-unused-alias-decls,
+misc-unused-parameters,
+misc-unused-using-decls,
+bugprone-argument-comment,
+bugprone-assert-side-effect,
+bugprone-bool-pointer-implicit-conversion,
+bugprone-copy-constructor-init,
+bugprone-dangling-handle,
+bugprone-exception-escape,
+bugprone-fold-init-type,
+bugprone-forward-declaration-namespace,
+bugprone-forwarding-reference-overload,
+bugprone-inaccurate-erase,
+bugprone-incorrect-roundings,
+bugprone-integer-division,
+bugprone-macro-parentheses,
+bugprone-macro-repeated-side-effects,
+bugprone-misplaced-operator-in-strlen-in-alloc,
+bugprone-misplaced-widening-cast,
+bugprone-move-forwarding-reference,
+bugprone-multiple-statement-macro,
+bugprone-parent-virtual-call,
+bugprone-sizeof-container,
+bugprone-sizeof-expression,
+bugprone-string-constructor,
+bugprone-string-integer-assignment,
+bugprone-string-literal-with-embedded-nul,
+bugprone-suspicious-enum-usage,
+bugprone-suspicious-memset-usage,
+bugprone-suspicious-missing-comma,
+bugprone-suspicious-semicolon,
+bugprone-suspicious-string-compare,
+bugprone-swapped-arguments,
+bugprone-terminating-continue,
+bugprone-throw-keyword-missing,
+bugprone-too-small-loop-variable,
+bugprone-undefined-memory-manipulation,
+bugprone-undelegated-constructor,
+bugprone-unused-return-value,
+bugprone-use-after-move,
+bugprone-virtual-near-miss,
+clang-analyzer-apiModeling.StdCLibraryFunctions,
+clang-analyzer-apiModeling.TrustNonnull,
+clang-analyzer-apiModeling.google.GTest,
+clang-analyzer-core.CallAndMessage,
+clang-analyzer-core.DivideZero,
+clang-analyzer-core.DynamicTypePropagation,
+clang-analyzer-core.NonNullParamChecker,
+clang-analyzer-core.NonnilStringConstants,
+clang-analyzer-core.NullDereference,
+clang-analyzer-core.StackAddressEscape,
+clang-analyzer-core.UndefinedBinaryOperatorResult,
+clang-analyzer-core.VLASize,
+clang-analyzer-core.builtin.BuiltinFunctions,
+clang-analyzer-core.builtin.NoReturnFunctions,
+clang-analyzer-core.uninitialized.ArraySubscript,
+clang-analyzer-core.uninitialized.Assign,
+clang-analyzer-core.uninitialized.Branch,
+clang-analyzer-core.uninitialized.CapturedBlockVariable,
+clang-analyzer-core.uninitialized.UndefReturn,
+clang-analyzer-cplusplus.InnerPointer,
+clang-analyzer-cplusplus.Move,
+clang-analyzer-cplusplus.NewDelete,
+clang-analyzer-cplusplus.NewDeleteLeaks,
+clang-analyzer-cplusplus.SelfAssignment,
+clang-analyzer-deadcode.DeadStores,
+clang-analyzer-optin.cplusplus.VirtualCall,
+clang-analyzer-optin.mpi.MPI-Checker,
+clang-analyzer-optin.performance.GCDAntipattern,
+clang-analyzer-optin.performance.Padding,
+clang-analyzer-optin.portability.UnixAPI,
+clang-analyzer-nullability.NullPassedToNonnull,
+clang-analyzer-nullability.NullReturnedFromNonnull,
+clang-analyzer-nullability.NullableDereferenced,
+clang-analyzer-nullability.NullablePassedToNonnull,
+clang-analyzer-nullability.NullableReturnedFromNonnull,
+clang-analyzer-security.FloatLoopCounter,
+clang-analyzer-security.insecureAPI.UncheckedReturn,
+clang-analyzer-security.insecureAPI.bcmp,
+clang-analyzer-security.insecureAPI.bcopy,
+clang-analyzer-security.insecureAPI.bzero,
+clang-analyzer-security.insecureAPI.getpw,
+clang-analyzer-security.insecureAPI.gets,
+clang-analyzer-security.insecureAPI.mkstemp,
+clang-analyzer-security.insecureAPI.mktemp,
+clang-analyzer-security.insecureAPI.rand,
+clang-analyzer-security.insecureAPI.strcpy,
+clang-analyzer-security.insecureAPI.vfork,
+clang-analyzer-unix.API,
+clang-analyzer-unix.Malloc,
+clang-analyzer-unix.MallocSizeof,
+clang-analyzer-unix.MismatchedDeallocator,
+clang-analyzer-unix.Vfork,
+clang-analyzer-unix.cstring.BadSizeArg,
+clang-analyzer-unix.cstring.NullArg,
+clang-analyzer-valist.CopyToSelf,
+clang-analyzer-valist.Uninitialized,
+clang-analyzer-valist.Unterminated,
 '
 ...
diff --git a/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp b/ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp
@@ -1183,11 +1183,11 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
     AnimationKeyFrame const &keyframe,
     ShadowViewMutation::List &mutationsList,
     bool interrupted,
-    const std::string &logPrefix) const {
+    const std::string & /*logPrefix*/) const {
   if (skipInvalidatedKeyFrames_ && keyframe.invalidated) {
     return;
   }
-  if (keyframe.finalMutationsForKeyFrame.size() > 0) {
+  if (!keyframe.finalMutationsForKeyFrame.empty()) {
     // TODO: modularize this segment, it is repeated 2x in KeyFrameManager
     // as well.
     ShadowView prev = keyframe.viewPrev;

diff --git a/ReactCommon/react/renderer/animations/tests/LayoutAnimationTest.cpp b/ReactCommon/react/renderer/animations/tests/LayoutAnimationTest.cpp
@@ -64,7 +64,7 @@ static void testShadowNodeTreeLifeCycleLayoutAnimations(
 
   // Create a RuntimeExecutor
   RuntimeExecutor runtimeExecutor =
-      [](std::function<void(jsi::Runtime &)> const &) {};
+      [](std::function<void(jsi::Runtime &)> const & /*unused*/) {};
 
   // Create component descriptor registry for animation driver
   auto providerRegistry =
@@ -158,7 +158,7 @@ static void testShadowNodeTreeLifeCycleLayoutAnimations(
       // If tree randomization produced no changes in the form of mutations,
       // don't bother trying to animate because this violates a bunch of our
       // assumptions in this test
-      if (originalMutations.size() == 0) {
+      if (originalMutations.empty()) {
         continue;
       }
 

diff --git a/ReactCommon/react/renderer/componentregistry/ComponentDescriptorProviderRegistry.cpp b/ReactCommon/react/renderer/componentregistry/ComponentDescriptorProviderRegistry.cpp
@@ -11,7 +11,7 @@ namespace facebook {
 namespace react {
 
 void ComponentDescriptorProviderRegistry::add(
-    ComponentDescriptorProvider provider) const {
+    const ComponentDescriptorProvider &provider) const {
   std::unique_lock<butter::shared_mutex> lock(mutex_);
 
   /*

diff --git a/ReactCommon/react/renderer/componentregistry/ComponentDescriptorProviderRegistry.h b/ReactCommon/react/renderer/componentregistry/ComponentDescriptorProviderRegistry.h
@@ -33,7 +33,7 @@ class ComponentDescriptorProviderRegistry final {
    * `ComponentDescriptorRegistry`s accordingly.
    * The methods can be called on any thread.
    */
-  void add(ComponentDescriptorProvider provider) const;
+  void add(const ComponentDescriptorProvider &provider) const;
 
   /*
    * ComponenDescriptorRegistry will call the `request` in case if a component

diff --git a/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.cpp b/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.cpp
@@ -50,7 +50,7 @@ void ComponentDescriptorRegistry::add(
 }
 
 void ComponentDescriptorRegistry::registerComponentDescriptor(
-    SharedComponentDescriptor componentDescriptor) const {
+    const SharedComponentDescriptor &componentDescriptor) const {
   ComponentHandle componentHandle = componentDescriptor->getComponentHandle();
   _registryByHandle[componentHandle] = componentDescriptor;
 
@@ -119,11 +119,7 @@ bool ComponentDescriptorRegistry::hasComponentDescriptorAt(
   std::shared_lock<butter::shared_mutex> lock(mutex_);
 
   auto iterator = _registryByHandle.find(componentHandle);
-  if (iterator == _registryByHandle.end()) {
-    return false;
-  }
-
-  return true;
+  return iterator != _registryByHandle.end();
 }
 
 ShadowNode::Shared ComponentDescriptorRegistry::createNode(
@@ -136,8 +132,7 @@ ShadowNode::Shared ComponentDescriptorRegistry::createNode(
   auto const &componentDescriptor = this->at(unifiedComponentName);
 
   auto const fragment = ShadowNodeFamilyFragment{tag, surfaceId, nullptr};
-  auto family =
-      componentDescriptor.createFamily(fragment, std::move(eventTarget));
+  auto family = componentDescriptor.createFamily(fragment, eventTarget);
   auto const props = componentDescriptor.cloneProps(
       PropsParserContext{surfaceId, *contextContainer_.get()},
       nullptr,
@@ -155,7 +150,7 @@ ShadowNode::Shared ComponentDescriptorRegistry::createNode(
 }
 
 void ComponentDescriptorRegistry::setFallbackComponentDescriptor(
-    SharedComponentDescriptor descriptor) {
+    const SharedComponentDescriptor &descriptor) {
   _fallbackComponentDescriptor = descriptor;
   registerComponentDescriptor(descriptor);
 }

diff --git a/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.h b/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.h
@@ -62,14 +62,15 @@ class ComponentDescriptorRegistry {
       folly::dynamic const &props,
       SharedEventTarget const &eventTarget) const;
 
-  void setFallbackComponentDescriptor(SharedComponentDescriptor descriptor);
+  void setFallbackComponentDescriptor(
+      const SharedComponentDescriptor &descriptor);
   ComponentDescriptor::Shared getFallbackComponentDescriptor() const;
 
  private:
   friend class ComponentDescriptorProviderRegistry;
 
   void registerComponentDescriptor(
-      SharedComponentDescriptor componentDescriptor) const;
+      const SharedComponentDescriptor &componentDescriptor) const;
 
   /*
    * Creates a `ComponentDescriptor` using specified

diff --git a/ReactCommon/react/renderer/componentregistry/native/NativeComponentRegistryBinding.cpp b/ReactCommon/react/renderer/componentregistry/native/NativeComponentRegistryBinding.cpp
@@ -20,7 +20,7 @@ namespace react {
  */
 NativeComponentRegistryBinding::NativeComponentRegistryBinding(
     const HasComponentProviderFunctionType &&hasComponentProvider)
-    : hasComponentProvider_(std::move(hasComponentProvider)) {}
+    : hasComponentProvider_(hasComponentProvider) {}
 
 void NativeComponentRegistryBinding::install(
     jsi::Runtime &runtime,
@@ -61,7 +61,7 @@ jsi::Value NativeComponentRegistryBinding::jsProxy(
 
   bool result = hasComponent(moduleName);
 
-  return jsi::Value(result);
+  return {result};
 }
 
 } // namespace react

diff --git a/...rogressbar/android/react/renderer/components/progressbar/AndroidProgressBarShadowNode.cpp b/...rogressbar/android/react/renderer/components/progressbar/AndroidProgressBarShadowNode.cpp
@@ -25,7 +25,7 @@ void AndroidProgressBarShadowNode::setAndroidProgressBarMeasurementsManager(
 #pragma mark - LayoutableShadowNode
 
 Size AndroidProgressBarShadowNode::measureContent(
-    LayoutContext const &layoutContext,
+    LayoutContext const & /*layoutContext*/,
     LayoutConstraints const &layoutConstraints) const {
   return measurementsManager_->measure(
       getSurfaceId(), getConcreteProps(), layoutConstraints);

diff --git a/ReactCommon/react/renderer/components/root/RootProps.cpp b/ReactCommon/react/renderer/components/root/RootProps.cpp
@@ -26,8 +26,8 @@ RootProps::RootProps(
 // If that's a problem and the context is necesary here, refactor RootShadowNode
 // first.
 RootProps::RootProps(
-    const PropsParserContext &context,
-    RootProps const &sourceProps,
+    const PropsParserContext & /*context*/,
+    RootProps const & /*sourceProps*/,
     LayoutConstraints const &layoutConstraints,
     LayoutContext const &layoutContext)
     : ViewProps(),

diff --git a/ReactCommon/react/renderer/components/slider/SliderShadowNode.cpp b/ReactCommon/react/renderer/components/slider/SliderShadowNode.cpp
@@ -84,7 +84,7 @@ ImageSource SliderShadowNode::getThumbImageSource() const {
 #pragma mark - LayoutableShadowNode
 
 Size SliderShadowNode::measureContent(
-    LayoutContext const &layoutContext,
+    LayoutContext const & /*layoutContext*/,
     LayoutConstraints const &layoutConstraints) const {
   if (SliderMeasurementsManager::shouldMeasureSlider()) {
     return measurementsManager_->measure(getSurfaceId(), layoutConstraints);

diff --git a/.../switch/androidswitch/react/renderer/components/androidswitch/AndroidSwitchShadowNode.cpp b/.../switch/androidswitch/react/renderer/components/androidswitch/AndroidSwitchShadowNode.cpp
@@ -22,7 +22,7 @@ void AndroidSwitchShadowNode::setAndroidSwitchMeasurementsManager(
 #pragma mark - LayoutableShadowNode
 
 Size AndroidSwitchShadowNode::measureContent(
-    LayoutContext const &layoutContext,
+    LayoutContext const & /*layoutContext*/,
     LayoutConstraints const &layoutConstraints) const {
   return measurementsManager_->measure(getSurfaceId(), layoutConstraints);
 }

diff --git a/ReactCommon/react/renderer/components/text/BaseTextProps.cpp b/ReactCommon/react/renderer/components/text/BaseTextProps.cpp
@@ -219,7 +219,7 @@ BaseTextProps::BaseTextProps(
 void BaseTextProps::setProp(
     const PropsParserContext &context,
     RawPropsPropNameHash hash,
-    const char *propName,
+    const char * /*propName*/,
     RawValue const &value) {
   static auto defaults = TextAttributes{};
 

diff --git a/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp b/ReactCommon/react/renderer/components/text/BaseTextShadowNode.cpp
@@ -34,7 +34,7 @@ void BaseTextShadowNode::buildAttributedString(
     // RawShadowNode
     auto rawTextShadowNode =
         traitCast<RawTextShadowNode const *>(childNode.get());
-    if (rawTextShadowNode) {
+    if (rawTextShadowNode != nullptr) {
       auto fragment = AttributedString::Fragment{};
       fragment.string = rawTextShadowNode->getConcreteProps().text;
       fragment.textAttributes = baseTextAttributes;
@@ -50,7 +50,7 @@ void BaseTextShadowNode::buildAttributedString(
 
     // TextShadowNode
     auto textShadowNode = traitCast<TextShadowNode const *>(childNode.get());
-    if (textShadowNode) {
+    if (textShadowNode != nullptr) {
       auto localTextAttributes = baseTextAttributes;
       localTextAttributes.apply(
           textShadowNode->getConcreteProps().textAttributes);

diff --git a/...ndroidtextinput/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp b/...ndroidtextinput/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp
@@ -168,7 +168,7 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() {
 #pragma mark - LayoutableShadowNode
 
 Size AndroidTextInputShadowNode::measureContent(
-    LayoutContext const &layoutContext,
+    LayoutContext const & /*layoutContext*/,
     LayoutConstraints const &layoutConstraints) const {
   if (getStateData().cachedAttributedStringId != 0) {
     return textLayoutManager_

diff --git a/ReactCommon/react/renderer/components/textinput/iostextinput/TextInputEventEmitter.h b/ReactCommon/react/renderer/components/textinput/iostextinput/TextInputEventEmitter.h
@@ -45,8 +45,8 @@ class TextInputEventEmitter : public ViewEventEmitter {
   void onSelectionChange(TextInputMetrics const &textInputMetrics) const;
   void onEndEditing(TextInputMetrics const &textInputMetrics) const;
   void onSubmitEditing(TextInputMetrics const &textInputMetrics) const;
-  void onKeyPress(KeyPressMetrics const &textInputMetrics) const;
-  void onKeyPressSync(KeyPressMetrics const &textInputMetrics) const;
+  void onKeyPress(KeyPressMetrics const &keyPressMetrics) const;
+  void onKeyPressSync(KeyPressMetrics const &keyPressMetrics) const;
   void onScroll(TextInputMetrics const &textInputMetrics) const;
 
  private:

diff --git a/ReactCommon/react/renderer/components/textinput/iostextinput/TextInputState.cpp b/ReactCommon/react/renderer/components/textinput/iostextinput/TextInputState.cpp
@@ -12,8 +12,8 @@ namespace react {
 
 #ifdef ANDROID
 TextInputState::TextInputState(
-    TextInputState const &previousState,
-    folly::dynamic const &data){};
+    TextInputState const & /*previousState*/,
+    folly::dynamic const & /*data*/){};
 
 /*
  * Empty implementation for Android because it doesn't use this class.

diff --git a/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp b/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp
@@ -194,7 +194,7 @@ AccessibilityProps::AccessibilityProps(
 void AccessibilityProps::setProp(
     const PropsParserContext &context,
     RawPropsPropNameHash hash,
-    const char *propName,
+    const char * /*propName*/,
     RawValue const &value) {
   switch (hash) {
     RAW_SET_PROP_SWITCH_CASE_BASIC(accessible, false);
@@ -217,7 +217,7 @@ void AccessibilityProps::setProp(
     RAW_SET_PROP_SWITCH_CASE(testId, "testID", std::string{""});
     case CONSTEXPR_RAW_PROPS_KEY_HASH("accessibilityRole"): {
       AccessibilityTraits traits = AccessibilityTraits::None;
-      std::string roleString = "";
+      std::string roleString;
       if (value.hasValue()) {
         fromRawValue(context, value, traits);
         fromRawValue(context, value, roleString);

diff --git a/ReactCommon/react/renderer/components/view/PointerEvent.cpp b/ReactCommon/react/renderer/components/view/PointerEvent.cpp
@@ -12,7 +12,7 @@ namespace react {
 
 #if RN_DEBUG_STRING_CONVERTIBLE
 
-std::string getDebugName(PointerEvent const &pointerEvent) {
+std::string getDebugName(PointerEvent const & /*pointerEvent*/) {
   return "PointerEvent";
 }