Enable more linting (#20187)

Author: zanderso

ci/bin/lint.dart

@@ -241,7 +241,7 @@ void main(List<String> arguments) async {
   final ProcessPool pool = ProcessPool();
 
   await for (final WorkerJob job in pool.startWorkers(jobs)) {
-    if (job.result.stdout.isEmpty) {
+    if (job.result?.stdout.isEmpty ?? true) {
       continue;
     }
     print('❌ Failures for ${job.name}:');

ci/lint.sh

@@ -42,6 +42,7 @@ fi
 
 cd "$CI_DIR"
 pub get && dart \
+  --disable-dart-dev \
   bin/lint.dart \
   --compile-commands="$COMPILE_COMMANDS" \
   --repo="$SRC_DIR/flutter" \

runtime/dart_isolate.cc

@@ -1,7 +1,6 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// FLUTTER_NOLINT
 
 #include "flutter/runtime/dart_isolate.h"
 
@@ -44,7 +43,7 @@ class DartErrorString {
   }
   char** error() { return &str_; }
   const char* str() const { return str_; }
-  operator bool() const { return str_ != nullptr; }
+  explicit operator bool() const { return str_ != nullptr; }
 
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(DartErrorString);
@@ -385,7 +384,7 @@ bool DartIsolate::LoadKernel(std::shared_ptr<const fml::Mapping> mapping,
   if (GetIsolateGroupData().GetChildIsolatePreparer() == nullptr) {
     GetIsolateGroupData().SetChildIsolatePreparer(
         [buffers = kernel_buffers_](DartIsolate* isolate) {
-          for (unsigned long i = 0; i < buffers.size(); i++) {
+          for (uint64_t i = 0; i < buffers.size(); i++) {
             bool last_piece = i + 1 == buffers.size();
             const std::shared_ptr<const fml::Mapping>& buffer = buffers.at(i);
             if (!isolate->PrepareForRunningFromKernel(buffer, last_piece)) {
@@ -676,6 +675,10 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
     );
   }
 
+  if (!parent_isolate_data) {
+    return nullptr;
+  }
+
   DartIsolateGroupData& parent_group_data =
       (*parent_isolate_data)->GetIsolateGroupData();
 
@@ -690,7 +693,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
               parent_group_data.GetIsolateShutdownCallback())));
 
   TaskRunners null_task_runners(advisory_script_uri,
-                                /* platform= */ nullptr, /* raster= */ nullptr,
+                                /* platform= */ nullptr,
+                                /* raster= */ nullptr,
                                 /* ui= */ nullptr,
                                 /* io= */ nullptr);
 
@@ -732,7 +736,8 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data,
           Dart_CurrentIsolateGroupData());
 
   TaskRunners null_task_runners((*isolate_group_data)->GetAdvisoryScriptURI(),
-                                /* platform= */ nullptr, /* raster= */ nullptr,
+                                /* platform= */ nullptr,
+                                /* raster= */ nullptr,
                                 /* ui= */ nullptr,
                                 /* io= */ nullptr);
 

runtime/dart_isolate_unittests.cc

@@ -1,7 +1,6 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// FLUTTER_NOLINT
 
 #include "flutter/fml/mapping.h"
 #include "flutter/fml/synchronization/count_down_latch.h"
@@ -19,7 +18,19 @@
 namespace flutter {
 namespace testing {
 
-using DartIsolateTest = FixtureTest;
+class DartIsolateTest : public FixtureTest {
+ public:
+  DartIsolateTest() {}
+
+  void Wait() { latch_.Wait(); }
+
+  void Signal() { latch_.Signal(); }
+
+ private:
+  fml::AutoResetWaitableEvent latch_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateTest);
+};
 
 TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
   ASSERT_FALSE(DartVMRef::IsInstanceRunning());
@@ -153,11 +164,10 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) {
 
 TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
   ASSERT_FALSE(DartVMRef::IsInstanceRunning());
-  fml::AutoResetWaitableEvent latch;
   AddNativeCallback("NotifyNative",
-                    CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
+                    CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
                       FML_LOG(ERROR) << "Hello from Dart!";
-                      latch.Signal();
+                      Signal();
                     })));
   const auto settings = CreateSettingsForFixture();
   auto vm_ref = DartVMRef::Create(settings);
@@ -173,7 +183,7 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
                            "canRegisterNativeCallback", {}, GetFixturesPath());
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
-  latch.Wait();
+  Wait();
 }
 
 TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
@@ -182,12 +192,11 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
     GTEST_SKIP();
     return;
   }
-  fml::AutoResetWaitableEvent latch;
   AddNativeCallback("NotifyNative",
-                    CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
+                    CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
                       ASSERT_TRUE(tonic::DartConverter<bool>::FromDart(
                           Dart_GetNativeArgument(args, 0)));
-                      latch.Signal();
+                      Signal();
                     })));
 
   const auto settings = CreateSettingsForFixture();
@@ -205,31 +214,52 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
 
-  latch.Wait();
+  Wait();
 }
 
-TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
-  fml::CountDownLatch latch(3);
-  fml::AutoResetWaitableEvent child_shutdown_latch;
-  fml::AutoResetWaitableEvent root_isolate_shutdown_latch;
+class DartSecondaryIsolateTest : public FixtureTest {
+ public:
+  DartSecondaryIsolateTest() : latch_(3) {}
+
+  void LatchCountDown() { latch_.CountDown(); }
+
+  void LatchWait() { latch_.Wait(); }
+
+  void ChildShutdownSignal() { child_shutdown_latch_.Signal(); }
+
+  void ChildShutdownWait() { child_shutdown_latch_.Wait(); }
+
+  void RootIsolateShutdownSignal() { root_isolate_shutdown_latch_.Signal(); }
+
+  bool RootIsolateIsSignaled() {
+    return root_isolate_shutdown_latch_.IsSignaledForTest();
+  }
+
+ private:
+  fml::CountDownLatch latch_;
+  fml::AutoResetWaitableEvent child_shutdown_latch_;
+  fml::AutoResetWaitableEvent root_isolate_shutdown_latch_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(DartSecondaryIsolateTest);
+};
+
+TEST_F(DartSecondaryIsolateTest, CanLaunchSecondaryIsolates) {
   AddNativeCallback("NotifyNative",
-                    CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
-                      latch.CountDown();
+                    CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
+                      LatchCountDown();
                     })));
   AddNativeCallback(
-      "PassMessage", CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
+      "PassMessage", CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
         auto message = tonic::DartConverter<std::string>::FromDart(
             Dart_GetNativeArgument(args, 0));
         ASSERT_EQ("Hello from code is secondary isolate.", message);
-        latch.CountDown();
+        LatchCountDown();
       })));
   auto settings = CreateSettingsForFixture();
-  settings.root_isolate_shutdown_callback = [&root_isolate_shutdown_latch]() {
-    root_isolate_shutdown_latch.Signal();
-  };
-  settings.isolate_shutdown_callback = [&child_shutdown_latch]() {
-    child_shutdown_latch.Signal();
+  settings.root_isolate_shutdown_callback = [this]() {
+    RootIsolateShutdownSignal();
   };
+  settings.isolate_shutdown_callback = [this]() { ChildShutdownSignal(); };
   auto vm_ref = DartVMRef::Create(settings);
   auto thread = CreateNewThread();
   TaskRunners task_runners(GetCurrentTestName(),  //
@@ -243,19 +273,18 @@ TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
                                       GetFixturesPath());
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
-  child_shutdown_latch.Wait();  // wait for child isolate to shutdown first
-  ASSERT_FALSE(root_isolate_shutdown_latch.IsSignaledForTest());
-  latch.Wait();  // wait for last NotifyNative called by main isolate
+  ChildShutdownWait();  // wait for child isolate to shutdown first
+  ASSERT_FALSE(RootIsolateIsSignaled());
+  LatchWait();  // wait for last NotifyNative called by main isolate
   // root isolate will be auto-shutdown
 }
 
 TEST_F(DartIsolateTest, CanRecieveArguments) {
-  fml::AutoResetWaitableEvent latch;
   AddNativeCallback("NotifyNative",
-                    CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
+                    CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
                       ASSERT_TRUE(tonic::DartConverter<bool>::FromDart(
                           Dart_GetNativeArgument(args, 0)));
-                      latch.Signal();
+                      Signal();
                     })));
 
   const auto settings = CreateSettingsForFixture();
@@ -273,7 +302,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) {
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
 
-  latch.Wait();
+  Wait();
 }
 
 }  // namespace testing

runtime/dart_service_isolate.cc

@@ -1,7 +1,6 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// FLUTTER_NOLINT
 
 #include "flutter/runtime/dart_service_isolate.h"
 
@@ -202,6 +201,7 @@ bool DartServiceIsolate::Startup(std::string server_ip,
   result = Dart_SetField(
       library, Dart_NewStringFromCString("_enableServicePortFallback"),
       Dart_NewBoolean(enable_service_port_fallback));
+  SHUTDOWN_ON_ERROR(result);
   return true;
 }
 

runtime/dart_vm.cc

@@ -1,7 +1,6 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// FLUTTER_NOLINT
 
 #include "flutter/runtime/dart_vm.h"
 
@@ -129,13 +128,15 @@ bool DartFileModifiedCallback(const char* source_url, int64_t since_ms) {
 
   const char* path = source_url + kFileUriPrefixLength;
   struct stat info;
-  if (stat(path, &info) < 0)
+  if (stat(path, &info) < 0) {
     return true;
+  }
 
   // If st_mtime is zero, it's more likely that the file system doesn't support
   // mtime than that the file was actually modified in the 1970s.
-  if (!info.st_mtime)
+  if (!info.st_mtime) {
     return true;
+  }
 
   // It's very unclear what time bases we're with here. The Dart API doesn't
   // document the time base for since_ms. Reading the code, the value varies by
@@ -383,8 +384,9 @@ DartVM::DartVM(std::shared_ptr<const DartVMData> vm_data,
   PushBackAll(&args, kDartTraceStreamsArgs, fml::size(kDartTraceStreamsArgs));
 #endif
 
-  for (size_t i = 0; i < settings_.dart_flags.size(); i++)
+  for (size_t i = 0; i < settings_.dart_flags.size(); i++) {
     args.push_back(settings_.dart_flags[i].c_str());
+  }
 
   char* flags_error = Dart_SetVMFlags(args.size(), args.data());
   if (flags_error) {