allow filtering actions, and add test for removing unused var entirely

Author: zth

tests/build_tests/actions/expected/Actions_PrefixUnusedVarUnderscore_applied.res

@@ -1,3 +1,4 @@
+// actionFilter=PrefixVariableWithUnderscore
 let f = () => {
   let _x = 1
   12

tests/build_tests/actions/expected/Actions_RemoveUnusedVar_applied.res

@@ -0,0 +1,4 @@
+// actionFilter=RemoveUnusedVariable
+let f = () => {
+  12
+}

tests/build_tests/actions/fixtures/Actions_PrefixUnusedVarUnderscore.res

@@ -1,3 +1,4 @@
+// actionFilter=PrefixVariableWithUnderscore
 let f = () => {
   let x = 1
   12

tests/build_tests/actions/fixtures/Actions_RemoveUnusedVar.res

@@ -0,0 +1,5 @@
+// actionFilter=RemoveUnusedVariable
+let f = () => {
+  let x = 1
+  12
+}

tests/build_tests/actions/input.js

@@ -35,11 +35,14 @@ for (const fileName of fixtures) {
   const fullFilePath = path.join(import.meta.dirname, "fixtures", fileName);
   const cmtPath = fullFilePath.replace(".res", ".cmt");
   await bsc([...prefix, "-color", "always", fullFilePath]);
-  const { stdout, stderr } = await rescriptTools("actions", [
-    fullFilePath,
-    cmtPath,
-    "--runAll",
-  ]);
+  const firstLine =
+    (await fs.readFile(fullFilePath, "utf-8")).split("\n")[0] ?? "";
+  const actionFilter = firstLine.split("actionFilter=")[1];
+  const callArgs = [fullFilePath, cmtPath, "--runAll"];
+  if (actionFilter != null) {
+    callArgs.push("--actionFilter", actionFilter);
+  }
+  const { stdout, stderr } = await rescriptTools("actions", callArgs);
   if (stderr.length > 0) {
     console.error(stderr.toString());
   }

tools/bin/main.ml

@@ -111,12 +111,25 @@ let main () =
     Reanalyze.cli ()
   | "actions" :: file :: opts ->
     let run_all_on_file = List.mem "--runAll" opts in
+    let rec extract_arg_with_value target_arg opts =
+      match opts with
+      | arg :: value :: _ when arg = target_arg -> Some value
+      | _ :: rest -> extract_arg_with_value target_arg rest
+      | [] -> None
+    in
     let cmtPath =
       match opts with
       | path :: _ when String.ends_with ~suffix:".cmt" path -> Some path
       | _ -> None
     in
-    if run_all_on_file then Tools.Actions.runActionsOnFile ?cmtPath file
+    let actionFilter =
+      match extract_arg_with_value "--actionFilter" opts with
+      | Some filter ->
+        Some (String.split_on_char ',' filter |> List.map String.trim)
+      | None -> None
+    in
+    if run_all_on_file then
+      Tools.Actions.runActionsOnFile ?actionFilter ?cmtPath file
     else Tools.Actions.extractActionsFromFile ?cmtPath file
   | "extract-embedded" :: extPointNames :: filename :: _ ->
     logAndExit

tools/src/tools.ml

@@ -1321,7 +1321,23 @@ module Actions = struct
             Ast_mapper.default_mapper.structure mapper items);
         value_bindings =
           (fun mapper bindings ->
-            (* TODO: Implement removing binding action *)
+            let remove_unused_variables_action_locs =
+              List.filter_map
+                (fun (action : Cmt_utils.cmt_action) ->
+                  match action.action with
+                  | RemoveUnusedVariable -> Some action.loc
+                  | _ -> None)
+                actions
+            in
+            let bindings =
+              bindings
+              |> List.filter_map (fun (binding : Parsetree.value_binding) ->
+                     if
+                       List.mem binding.pvb_pat.ppat_loc
+                         remove_unused_variables_action_locs
+                     then None
+                     else Some binding)
+            in
             Ast_mapper.default_mapper.value_bindings mapper bindings);
         pat =
           (fun mapper pattern ->
@@ -1492,9 +1508,20 @@ module Actions = struct
                            }
                        | _ -> None)
             in
+            let mapped_expr =
+              match mapped_expr with
+              | None -> Ast_mapper.default_mapper.expr mapper expr
+              | Some expr -> expr
+            in
+            (* We sometimes need to do some post-transformation cleanup. 
+            E.g if all let bindings was removed from `Pexp_let`, we need to remove the entire Pexp_let.*)
             match mapped_expr with
-            | None -> Ast_mapper.default_mapper.expr mapper expr
-            | Some expr -> expr);
+            | {pexp_desc = Pexp_let (_, [], cont); pexp_attributes} ->
+              {
+                cont with
+                pexp_attributes = cont.pexp_attributes @ pexp_attributes;
+              }
+            | _ -> mapped_expr);
       }
     in
     if Filename.check_suffix path ".res" then
@@ -1511,7 +1538,8 @@ module Actions = struct
            "error: failed to apply actions to %s because it is not a .res file"
            path)
 
-  let runActionsOnFile ?cmtPath entryPointFile =
+  let runActionsOnFile ?(actionFilter : string list option) ?cmtPath
+      entryPointFile =
     let path =
       match Filename.is_relative entryPointFile with
       | true -> Unix.realpath entryPointFile
@@ -1529,7 +1557,33 @@ module Actions = struct
          be found. try to build the project"
         path
     | Some {cmt_possible_actions} -> (
-      match applyActionsToFile path cmt_possible_actions with
+      let possible_actions =
+        match actionFilter with
+        | None -> cmt_possible_actions
+        | Some filter ->
+          cmt_possible_actions
+          |> List.filter (fun (action : Cmt_utils.cmt_action) ->
+                 match action.action with
+                 | Cmt_utils.ApplyFunction _ -> List.mem "ApplyFunction" filter
+                 | ApplyCoercion _ -> List.mem "ApplyCoercion" filter
+                 | RemoveSwitchCase -> List.mem "RemoveSwitchCase" filter
+                 | RemoveOpen -> List.mem "RemoveOpen" filter
+                 | RemoveAwait -> List.mem "RemoveAwait" filter
+                 | AddAwait -> List.mem "AddAwait" filter
+                 | ReplaceWithVariantConstructor _ ->
+                   List.mem "ReplaceWithVariantConstructor" filter
+                 | ReplaceWithPolymorphicVariantConstructor _ ->
+                   List.mem "ReplaceWithPolymorphicVariantConstructor" filter
+                 | RewriteObjectToRecord ->
+                   List.mem "RewriteObjectToRecord" filter
+                 | RewriteArrayToTuple -> List.mem "RewriteArrayToTuple" filter
+                 | RewriteIdent _ -> List.mem "RewriteIdent" filter
+                 | PrefixVariableWithUnderscore ->
+                   List.mem "PrefixVariableWithUnderscore" filter
+                 | RemoveUnusedVariable ->
+                   List.mem "RemoveUnusedVariable" filter)
+      in
+      match applyActionsToFile path possible_actions with
       | Ok applied -> print_endline applied
       | Error e ->
         print_endline e;