Skip to content

Commit

Permalink
Dead fixme removal also removes dead HH_IGNOREs
Browse files Browse the repository at this point in the history
Summary: What it says on the tin

Differential Revision: D62394047

fbshipit-source-id: c3a06eec7c26429b81f83bd889455722231b3e7a
  • Loading branch information
Catherine Gasnier authored and facebook-github-bot committed Sep 16, 2024
1 parent 44563a2 commit 9ba137b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
11 changes: 8 additions & 3 deletions hphp/hack/src/providers/fixme_provider.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand All @@ -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

(*****************************************************************************)
Expand Down
33 changes: 31 additions & 2 deletions hphp/hack/test/integration/test_fresh_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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"]
)
Expand All @@ -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) {
Expand All @@ -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;
}
""",
)
Expand Down

0 comments on commit 9ba137b

Please sign in to comment.