Add clang-analyzer-* and clang-diagnostic-* to .clang-tidy (flutter#3…

…1291)

.clang-tidy

@@ -1,13 +1,20 @@
 # Prefix check with "-" to ignore.
 Checks: "google-*,\
+clang-analyzer-*,\
+clang-diagnostic-*,\
 -google-objc-global-variable-declaration,\
--google-objc-avoid-throwing-exception"
+-google-objc-avoid-throwing-exception,\
+-clang-analyzer-nullability.NullPassedToNonnull,\
+-clang-analyzer-nullability.NullablePassedToNonnull,\
+-clang-analyzer-nullability.NullReturnedFromNonnull,\
+-clang-analyzer-nullability.NullableReturnedFromNonnull"
 
 # Only warnings treated as errors are reported
 # in the "ci/lint.sh" script and pre-push git hook.
 # Add checks when all warnings are fixed
 # to prevent new warnings being introduced.
 # https://github.com/flutter/flutter/issues/93279
-WarningsAsErrors: "clang-analyzer-osx.*,\
+WarningsAsErrors: "clang-analyzer-*,\
+clang-diagnostic-*,\
 google-objc-*,\
 google-explicit-constructor"

common/graphics/persistent_cache.cc

@@ -191,6 +191,9 @@ sk_sp<SkData> ParseBase64(const std::string& input) {
 }
 
 size_t PersistentCache::PrecompileKnownSkSLs(GrDirectContext* context) const {
+  // clang-tidy has trouble reasoning about some of the complicated array and
+  // pointer-arithmetic code in rapidjson.
+  // NOLINTNEXTLINE(clang-analyzer-cplusplus.PlacementNew)
   auto known_sksls = LoadSkSLs();
   // A trace must be present even if no precompilations have been completed.
   FML_TRACE_EVENT("flutter", "PersistentCache::PrecompileKnownSkSLs", "count",

display_list/display_list_benchmarks.cc

@@ -114,7 +114,7 @@ void BM_DrawLine(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -162,7 +162,7 @@ void BM_DrawRect(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -207,7 +207,7 @@ void BM_DrawOval(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -254,7 +254,7 @@ void BM_DrawCircle(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -331,7 +331,7 @@ void BM_DrawRRect(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -412,7 +412,7 @@ void BM_DrawDRRect(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -464,7 +464,7 @@ void BM_DrawArc(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -482,7 +482,7 @@ std::vector<SkPoint> GetPolygonPoints(size_t n, SkPoint center, SkScalar r) {
   float angle;
   float full_circle = 2.0f * M_PI;
   for (size_t i = 0; i < n; i++) {
-    angle = (full_circle / (float)n) * (float)i;
+    angle = (full_circle / static_cast<float>(n)) * static_cast<float>(i);
     x = center.x() + r * std::cosf(angle);
     y = center.y() + r * std::sinf(angle);
     points.push_back(SkPoint::Make(x, y));
@@ -666,7 +666,7 @@ void BM_DrawPath(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -709,12 +709,13 @@ sk_sp<SkVertices> GetTestVertices(SkPoint center,
       colors.push_back(SK_ColorCYAN);
       for (size_t i = 0; i <= outer_points.size(); i++) {
         vertices.push_back(outer_points[i % outer_points.size()]);
-        if (i % 3 == 0)
+        if (i % 3 == 0) {
           colors.push_back(SK_ColorRED);
-        else if (i % 3 == 1)
+        } else if (i % 3 == 1) {
           colors.push_back(SK_ColorGREEN);
-        else
+        } else {
           colors.push_back(SK_ColorBLUE);
+        }
       }
       break;
     case SkVertices::VertexMode::kTriangles_VertexMode:
@@ -810,7 +811,7 @@ void BM_DrawVertices(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -913,7 +914,7 @@ void BM_DrawPoints(benchmark::State& state,
 
   auto display_list = builder.Build();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -987,7 +988,7 @@ void BM_DrawImage(benchmark::State& state,
 
   auto display_list = builder.Build();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -1069,7 +1070,7 @@ void BM_DrawImageRect(benchmark::State& state,
 
   auto display_list = builder.Build();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -1153,7 +1154,7 @@ void BM_DrawImageNine(benchmark::State& state,
 
   auto display_list = builder.Build();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -1226,7 +1227,7 @@ void BM_DrawTextBlob(benchmark::State& state,
 
   auto display_list = builder.Build();
 
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -1290,7 +1291,7 @@ void BM_DrawShadow(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }
@@ -1343,7 +1344,7 @@ void BM_SaveLayer(benchmark::State& state,
   auto display_list = builder.Build();
 
   // We only want to time the actual rasterization.
-  for (auto _ : state) {
+  for ([[maybe_unused]] auto _ : state) {
     display_list->RenderTo(canvas);
     canvas_provider->GetSurface()->flushAndSubmit(true);
   }

flow/paint_utils.cc

@@ -37,8 +37,11 @@ void DrawCheckerboard(SkCanvas* canvas, const SkRect& rect) {
   canvas->save();
   canvas->clipRect(rect);
 
+  // Secure random number generation isn't needed here.
+  // NOLINTBEGIN(clang-analyzer-security.insecureAPI.rand)
   auto checkerboard_color =
       SkColorSetARGB(64, rand() % 256, rand() % 256, rand() % 256);
+  // NOLINTEND(clang-analyzer-security.insecureAPI.rand)
 
   DrawCheckerboard(canvas, checkerboard_color, 0x00000000, 12);
   canvas->restore();

fml/platform/android/jni_util.cc

@@ -52,7 +52,7 @@ JNIEnv* AttachCurrentThread() {
   } else {
     args.name = thread_name;
   }
-  jint ret = g_jvm->AttachCurrentThread(&env, &args);
+  [[maybe_unused]] jint ret = g_jvm->AttachCurrentThread(&env, &args);
   FML_DCHECK(JNI_OK == ret);
 
   FML_DCHECK(tls_jni_detach.get() == nullptr);
@@ -182,9 +182,10 @@ ScopedJavaLocalRef<jobjectArray> VectorToBufferArray(
       env->NewObjectArray(vector.size(), byte_buffer_clazz.obj(), NULL);
   ASSERT_NO_EXCEPTION();
   for (size_t i = 0; i < vector.size(); ++i) {
+    uint8_t* data = const_cast<uint8_t*>(vector[i].data());
     ScopedJavaLocalRef<jobject> item(
-        env,
-        env->NewDirectByteBuffer((void*)(vector[i].data()), vector[i].size()));
+        env, env->NewDirectByteBuffer(reinterpret_cast<void*>(data),
+                                      vector[i].size()));
     env->SetObjectArrayElement(java_array, i, item.obj());
   }
   return ScopedJavaLocalRef<jobjectArray>(env, java_array);
@@ -195,16 +196,18 @@ bool HasException(JNIEnv* env) {
 }
 
 bool ClearException(JNIEnv* env) {
-  if (!HasException(env))
+  if (!HasException(env)) {
     return false;
+  }
   env->ExceptionDescribe();
   env->ExceptionClear();
   return true;
 }
 
 bool CheckException(JNIEnv* env) {
-  if (!HasException(env))
+  if (!HasException(env)) {
     return true;
+  }
 
   jthrowable exception = env->ExceptionOccurred();
   env->ExceptionClear();

fml/platform/android/message_loop_android.cc

@@ -83,7 +83,7 @@ void MessageLoopAndroid::Terminate() {
 }
 
 void MessageLoopAndroid::WakeUp(fml::TimePoint time_point) {
-  bool result = TimerRearm(timer_fd_.get(), time_point);
+  [[maybe_unused]] bool result = TimerRearm(timer_fd_.get(), time_point);
   FML_DCHECK(result);
 }
 

fml/platform/android/scoped_java_ref.cc

@@ -13,13 +13,14 @@ namespace jni {
 static const int kDefaultLocalFrameCapacity = 16;
 
 ScopedJavaLocalFrame::ScopedJavaLocalFrame(JNIEnv* env) : env_(env) {
-  int failed = env_->PushLocalFrame(kDefaultLocalFrameCapacity);
+  [[maybe_unused]] int failed =
+      env_->PushLocalFrame(kDefaultLocalFrameCapacity);
   FML_DCHECK(!failed);
 }
 
 ScopedJavaLocalFrame::ScopedJavaLocalFrame(JNIEnv* env, int capacity)
     : env_(env) {
-  int failed = env_->PushLocalFrame(capacity);
+  [[maybe_unused]] int failed = env_->PushLocalFrame(capacity);
   FML_DCHECK(!failed);
 }
 
@@ -43,10 +44,12 @@ JNIEnv* JavaRef<jobject>::SetNewLocalRef(JNIEnv* env, jobject obj) {
   } else {
     FML_DCHECK(env == AttachCurrentThread());  // Is |env| on correct thread.
   }
-  if (obj)
+  if (obj) {
     obj = env->NewLocalRef(obj);
-  if (obj_)
+  }
+  if (obj_) {
     env->DeleteLocalRef(obj_);
+  }
   obj_ = obj;
   return env;
 }
@@ -57,10 +60,12 @@ void JavaRef<jobject>::SetNewGlobalRef(JNIEnv* env, jobject obj) {
   } else {
     FML_DCHECK(env == AttachCurrentThread());  // Is |env| on correct thread.
   }
-  if (obj)
+  if (obj) {
     obj = env->NewGlobalRef(obj);
-  if (obj_)
+  }
+  if (obj_) {
     env->DeleteGlobalRef(obj_);
+  }
   obj_ = obj;
 }
 

fml/synchronization/waitable_event_unittest.cc

@@ -15,6 +15,9 @@
 #include "flutter/fml/macros.h"
 #include "gtest/gtest.h"
 
+// rand() is only used for tests in this file.
+// NOLINTBEGIN(clang-analyzer-security.insecureAPI.rand)
+
 namespace fml {
 namespace {
 
@@ -183,3 +186,5 @@ TEST(ManualResetWaitableEventTest, SignalMultiple) {
 
 }  // namespace
 }  // namespace fml
+
+// NOLINTEND(clang-analyzer-security.insecureAPI.rand)

lib/ui/compositing/scene_builder_unittests.cc

@@ -13,6 +13,9 @@
 #include "flutter/shell/common/thread_host.h"
 #include "flutter/testing/testing.h"
 
+// CREATE_NATIVE_ENTRY is leaky by design
+// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
+
 namespace flutter {
 namespace testing {
 
@@ -156,3 +159,5 @@ TEST_F(ShellTest, EngineLayerDisposeReleasesReference) {
 
 }  // namespace testing
 }  // namespace flutter
+
+// NOLINTEND(clang-analyzer-core.StackAddressEscape)

lib/ui/hooks_unittests.cc

@@ -12,6 +12,9 @@
 #include "flutter/testing/testing.h"
 #include "third_party/dart/runtime/include/dart_api.h"
 
+// CREATE_NATIVE_ENTRY is leaky by design
+// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
+
 namespace flutter {
 namespace testing {
 
@@ -82,3 +85,5 @@ TEST_F(HooksTest, HooksUnitTests) {
 
 }  // namespace testing
 }  // namespace flutter
+
+// NOLINTEND(clang-analyzer-core.StackAddressEscape)

lib/ui/painting/image_encoding_unittests.cc

@@ -14,6 +14,9 @@
 #include "flutter/testing/testing.h"
 #include "gmock/gmock.h"
 
+// CREATE_NATIVE_ENTRY is leaky by design
+// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
+
 namespace flutter {
 namespace testing {
 
@@ -167,3 +170,5 @@ TEST_F(ShellTest, EncodeImageAccessesSyncSwitch) {
 
 }  // namespace testing
 }  // namespace flutter
+
+// NOLINTEND(clang-analyzer-core.StackAddressEscape)

lib/ui/painting/multi_frame_codec.cc

@@ -219,6 +219,9 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) {
       }));
 
   return Dart_Null();
+  // The static leak checker gets confused by the control flow, unique pointers
+  // and closures in this function.
+  // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks)
 }
 
 int MultiFrameCodec::frameCount() const {

runtime/dart_isolate.cc

@@ -1008,9 +1008,11 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup(
   {
     // Ownership of the isolate data objects has been transferred to the Dart
     // VM.
+    // NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
     std::shared_ptr<DartIsolate> embedder_isolate(*isolate_data);
     isolate_group_data.release();
     isolate_data.release();
+    // NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)
 
     success = InitializeIsolate(std::move(embedder_isolate), isolate, error);
   }