From 9ba137b426f0a28e76d0797fdc4cd126b32a2d25 Mon Sep 17 00:00:00 2001
From: Catherine Gasnier <catg@meta.com>
Date: Mon, 16 Sep 2024 04:10:51 -0700
Subject: [PATCH] Dead fixme removal also removes dead HH_IGNOREs

Summary: What it says on the tin

Differential Revision: D62394047

fbshipit-source-id: c3a06eec7c26429b81f83bd889455722231b3e7a
---
 hphp/hack/src/providers/fixme_provider.ml     | 11 +++++--
 hphp/hack/test/integration/test_fresh_init.py | 33 +++++++++++++++++--
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/hphp/hack/src/providers/fixme_provider.ml b/hphp/hack/src/providers/fixme_provider.ml
index 6bfb5fdfb0704e..2892d45ab83082 100644
--- a/hphp/hack/src/providers/fixme_provider.ml
+++ b/hphp/hack/src/providers/fixme_provider.ml
@@ -252,8 +252,11 @@ module UnusedFixmes = struct
        && (code < 5000 || Error_codes.Warning.of_enum code |> Option.is_some)
 
   let get_unused_fixmes_in_file
-      codes (applied_fixmes : FileToLineToCodesMap.t) (fn : Relative_path.t) acc
-      =
+      get_fixmes
+      codes
+      (applied_fixmes : FileToLineToCodesMap.t)
+      (fn : Relative_path.t)
+      acc =
     match get_fixmes fn with
     | None -> acc
     | Some fixme_map ->
@@ -277,7 +280,9 @@ module UnusedFixmes = struct
       ~(files : 'files) : Pos.t list =
     let applied_fixmes = FileToLineToCodesMap.make applied_fixmes in
     fold files ~init:[] ~f:(fun fn _ acc ->
-        get_unused_fixmes_in_file codes applied_fixmes fn acc)
+        acc
+        |> get_unused_fixmes_in_file get_fixmes codes applied_fixmes fn
+        |> get_unused_fixmes_in_file get_ignores codes applied_fixmes fn)
 end
 
 (*****************************************************************************)
diff --git a/hphp/hack/test/integration/test_fresh_init.py b/hphp/hack/test/integration/test_fresh_init.py
index 8b4de33bd98518..cbeba7ba95836d 100644
--- a/hphp/hack/test/integration/test_fresh_init.py
+++ b/hphp/hack/test/integration/test_fresh_init.py
@@ -81,7 +81,7 @@ def test_remove_dead_fixmes(self) -> None:
             f.write(
                 """<?hh // strict
                 function expect_int(int $_): void {}
-                function foo(?string $s): void {
+                function foo(?string $s, ?int $i): void {
                   /* HH_FIXME[4089] We can delete this one */
                   /* HH_FIXME[4110] We need to keep this one */
                   /* HH_FIXME[4099] We can delete this one */
@@ -94,10 +94,29 @@ def test_remove_dead_fixmes(self) -> None:
                   /* HH_FIXME[4099] We can delete this one */
                   /* HH_FIXME[4098] We can delete this one */
                   print "done\n";
+                  /* HH_IGNORE[12003] We can delete this one */
+                  /* HH_IGNORE[4110] We can delete this one */
+                  /* HH_IGNORE[12001] We can delete this one */
+                  print "sauerkraut";
+                  if (/* HH_IGNORE[12004] We can delete this one */   $s) {
+                    print "hello";
+                  } else if ($s /* HH_FIXME[4011] We can delete this one */) {
+                    print "world";
+                  } else if (/* HH_IGNORE[12003] We need to keep this one */   $s) {
+                    print "hello";
+                  }
+                  /* HH_IGNORE[12003] We can delete this one */
+                  /* HH_IGNORE[12001] We need to keep this one */
+                  /* HH_FIXME[4099] We can delete this one */
+                  /* HH_IGNORE[12004] We can delete this one */
+                  $s === $i;
                 }
             """
             )
 
+        # Allow to print full diff in case of failure of the assert
+        self.maxDiff = None
+
         self.test_driver.start_hh_server(
             changed_files=["foo_4.php"], args=["--no-load"]
         )
@@ -111,7 +130,7 @@ def test_remove_dead_fixmes(self) -> None:
                 out,
                 """<?hh // strict
                 function expect_int(int $_): void {}
-                function foo(?string $s): void {
+                function foo(?string $s, ?int $i): void {
                   /* HH_FIXME[4110] We need to keep this one */
                   expect_int($s);
                   if ($s) {
@@ -120,6 +139,16 @@ def test_remove_dead_fixmes(self) -> None:
                     print "world";
                   }
                   print "done\n";
+                  print "sauerkraut";
+                  if ($s) {
+                    print "hello";
+                  } else if ($s ) {
+                    print "world";
+                  } else if (/* HH_IGNORE[12003] We need to keep this one */   $s) {
+                    print "hello";
+                  }
+                  /* HH_IGNORE[12001] We need to keep this one */
+                  $s === $i;
                 }
             """,
             )