revamp insert missing cases code action

CHANGELOG.md

@@ -17,6 +17,10 @@
 - Add support for syntax highlighting in `%raw` and `%ffi` extension points. https://github.com/rescript-lang/rescript-vscode/pull/774
 - Add completion to top level decorators. https://github.com/rescript-lang/rescript-vscode/pull/799
 
+#### :nail_care: Polish
+
+- Revamp "Insert missing cases" code action to make it apply in more cases and be much more robust. https://github.com/rescript-lang/rescript-vscode/pull/804
+
 #### :bug: Bug Fix
 
 - Fix invalid range for `definition`. https://github.com/rescript-lang/rescript-vscode/pull/781

analysis/src/Cli.ml

@@ -124,6 +124,19 @@ let main () =
     Commands.codeAction ~path
       ~pos:(int_of_string line, int_of_string col)
       ~currentFile ~debug:false
+  | [_; "codemod"; path; line; col; typ; hint] ->
+    let typ =
+      match typ with
+      | "add-missing-cases" -> Codemod.AddMissingCases
+      | _ -> raise (Failure "unsupported type")
+    in
+    let res =
+      Codemod.transform ~path
+        ~pos:(int_of_string line, int_of_string col)
+        ~debug:false ~typ ~hint
+      |> Json.escape
+    in
+    Printf.printf "\"%s\"" res
   | [_; "diagnosticSyntax"; path] -> Commands.diagnosticSyntax ~path
   | _ :: "reanalyze" :: _ ->
     let len = Array.length Sys.argv in

analysis/src/Codemod.ml

@@ -0,0 +1,52 @@
+type transformType = AddMissingCases
+
+let rec collectPatterns p =
+  match p.Parsetree.ppat_desc with
+  | Ppat_or (p1, p2) -> collectPatterns p1 @ [p2]
+  | _ -> [p]
+
+let mkFailWithExp () =
+  Ast_helper.Exp.apply
+    (Ast_helper.Exp.ident {txt = Lident "failwith"; loc = Location.none})
+    [(Nolabel, Ast_helper.Exp.constant (Pconst_string ("TODO", None)))]
+
+let transform ~path ~pos ~debug ~typ ~hint =
+  let structure, printExpr, _ = Xform.parseImplementation ~filename:path in
+  match typ with
+  | AddMissingCases -> (
+    let source = "let " ^ hint ^ " = ()" in
+    let {Res_driver.parsetree = hintStructure} =
+      Res_driver.parseImplementationFromSource ~forPrinter:false
+        ~displayFilename:"<none>" ~source
+    in
+    match hintStructure with
+    | [{pstr_desc = Pstr_value (_, [{pvb_pat = pattern}])}] -> (
+      let cases =
+        collectPatterns pattern
+        |> List.map (fun (p : Parsetree.pattern) ->
+               Ast_helper.Exp.case p (mkFailWithExp ()))
+      in
+      let result = ref None in
+      let mkIterator ~pos ~result =
+        let expr (iterator : Ast_iterator.iterator) (exp : Parsetree.expression)
+            =
+          match exp.pexp_desc with
+          | Pexp_match (e, existingCases)
+            when Pos.ofLexing exp.pexp_loc.loc_start = pos ->
+            result :=
+              Some {exp with pexp_desc = Pexp_match (e, existingCases @ cases)}
+          | _ -> Ast_iterator.default_iterator.expr iterator exp
+        in
+        {Ast_iterator.default_iterator with expr}
+      in
+      let iterator = mkIterator ~pos ~result in
+      iterator.structure iterator structure;
+      match !result with
+      | None ->
+        if debug then print_endline "Found no result";
+        exit 1
+      | Some switchExpr ->
+        printExpr ~range:(Xform.rangeOfLoc switchExpr.pexp_loc) switchExpr)
+    | _ ->
+      if debug then print_endline "Mismatch in expected structure";
+      exit 1)

analysis/src/Commands.ml

@@ -392,6 +392,14 @@ let test ~path =
                                  Printf.printf "%s\nnewText:\n%s<--here\n%s%s\n"
                                    (Protocol.stringifyRange range)
                                    indent indent newText)))
+          | "c-a" ->
+            let hint = String.sub rest 3 (String.length rest - 3) in
+            print_endline
+              ("Codemod AddMissingCases" ^ path ^ " " ^ string_of_int line ^ ":"
+             ^ string_of_int col);
+            Codemod.transform ~path ~pos:(line, col) ~debug:true
+              ~typ:AddMissingCases ~hint
+            |> print_endline
           | "dia" -> diagnosticSyntax ~path
           | "hin" ->
             (* Get all inlay Hint between line 1 and n.

analysis/tests/bsconfig.json

@@ -9,7 +9,7 @@
       "subdirs": true
     }
   ],
-  "bsc-flags": ["-w -33-44"],
+  "bsc-flags": ["-w -33-44-8"],
   "bs-dependencies": ["@rescript/react"],
   "jsx": { "version": 3 }
 }

analysis/tests/src/Codemod.res

@@ -0,0 +1,9 @@
+type someTyp = [#valid | #invalid]
+
+let ff = (v1: someTyp, v2: someTyp) => {
+  let x = switch (v1, v2) {
+  //      ^c-a (#valid, #valid) | (#invalid, _)
+  | (#valid, #invalid) => ()
+  }
+  x
+}

analysis/tests/src/expected/Codemod.res.txt

@@ -0,0 +1,8 @@
+Codemod AddMissingCasessrc/Codemod.res 3:10
+switch (v1, v2) {
+          //      ^c-a (#valid, #valid) | (#invalid, _)
+          | (#valid, #invalid) => ()
+          | (#valid, #valid) => failwith("TODO")
+          | (#invalid, _) => failwith("TODO")
+          }
+

server/src/codeActions.ts

@@ -2,6 +2,8 @@
 // actions available in the extension, but they are derived via the analysis
 // OCaml binary.
 import * as p from "vscode-languageserver-protocol";
+import * as utils from "./utils";
+import { fileURLToPath } from "url";
 
 export type filesCodeActions = {
   [key: string]: { range: p.Range; codeAction: p.CodeAction }[];
@@ -82,6 +84,15 @@ let insertBeforeEndingChar = (
   ];
 };
 
+let replaceText = (range: p.Range, newText: string): p.TextEdit[] => {
+  return [
+    {
+      range,
+      newText,
+    },
+  ];
+};
+
 let removeTrailingComma = (text: string): string => {
   let str = text.trim();
   if (str.endsWith(",")) {
@@ -438,52 +449,8 @@ let applyUncurried: codeActionExtractor = ({
   return false;
 };
 
-// Untransformed is typically OCaml, and looks like these examples:
-//
-// `SomeVariantName
-//
-// SomeVariantWithPayload _
-//
-// ...and we'll need to transform this into proper ReScript. In the future, the
-// compiler itself should of course output real ReScript. But it currently does
-// not.
-//
-// Note that we're trying to not be too clever here, so we'll only try to
-// convert the very simplest cases - single variant/polyvariant, with single
-// payloads. No records, tuples etc yet. We can add those when the compiler
-// outputs them in proper ReScript.
-let transformMatchPattern = (matchPattern: string): string | null => {
-  let text = matchPattern.replace(/`/g, "#");
-
-  let payloadRegexp = / /g;
-  let matched = text.match(payloadRegexp);
-
-  // Constructors are preceded by a single space. Bail if there's more than 1.
-  if (matched != null && matched.length > 2) {
-    return null;
-  }
-
-  // Fix payloads if they can be fixed. If not, bail.
-  if (text.includes(" ")) {
-    let [variantText, payloadText] = text.split(" ");
-
-    let transformedPayloadText = transformMatchPattern(payloadText);
-    if (transformedPayloadText == null) {
-      return null;
-    }
-
-    text = `${variantText}(${payloadText})`;
-  }
-
-  return text;
-};
-
 // This action detects missing cases for exhaustive pattern matches, and offers
-// to insert dummy branches (using `assert false`) for those branches. Right now
-// it works on single variants/polyvariants with and without payloads. In the
-// future it could be made to work on anything the compiler tell us about, but
-// the compiler needs to emit proper ReScript in the error messages for that to
-// work.
+// to insert dummy branches (using `failwith("TODO")`) for those branches.
 let simpleAddMissingCases: codeActionExtractor = ({
   line,
   codeActions,
@@ -493,88 +460,33 @@ let simpleAddMissingCases: codeActionExtractor = ({
   array,
   index,
 }) => {
-  // Examples:
-  //
-  // You forgot to handle a possible case here, for example:
-  // (AnotherValue|Third|Fourth)
-  //
-  // You forgot to handle a possible case here, for example:
-  // (`AnotherValue|`Third|`Fourth)
-  //
-  // You forgot to handle a possible case here, for example:
-  // `AnotherValue
-  //
-  // You forgot to handle a possible case here, for example:
-  // AnotherValue
-  //
-  // You forgot to handle a possible case here, for example:
-  // (`One _|`Two _|
-  // `Three _)
-
   if (
     line.startsWith("You forgot to handle a possible case here, for example:")
   ) {
-    let cases: string[] = [];
-
     // This collects the rest of the fields if fields are printed on
     // multiple lines.
     let allCasesAsOneLine = array
       .slice(index + 1)
       .join("")
       .trim();
 
-    // We only handle the simplest possible cases until the compiler actually
-    // outputs ReScript. This means bailing on anything that's not a
-    // variant/polyvariant, with one payload (or no payloads at all).
-    let openParensCount = allCasesAsOneLine.split("(").length - 1;
-
-    if (openParensCount > 1 || allCasesAsOneLine.includes("{")) {
-      return false;
-    }
-
-    // Remove surrounding braces if they exist
-    if (allCasesAsOneLine[0] === "(") {
-      allCasesAsOneLine = allCasesAsOneLine.slice(
-        1,
-        allCasesAsOneLine.length - 1
-      );
-    }
-
-    cases.push(
-      ...(allCasesAsOneLine
-        .split("|")
-        .map(transformMatchPattern)
-        .filter(Boolean) as string[])
-    );
-
-    if (cases.length === 0) {
-      return false;
-    }
-
-    // The end char is the closing brace. In switches, the leading `|` always
-    // has the same left padding as the end brace.
-    let paddingContentSwitchCase = Array.from({
-      length: range.end.character,
-    }).join(" ");
-
-    let newText = cases
-      .map((variantName, index) => {
-        // The first case will automatically be padded because we're inserting
-        // it where the end brace is currently located.
-        let padding = index === 0 ? "" : paddingContentSwitchCase;
-        return `${padding}| ${variantName} => assert false`;
-      })
-      .join("\n");
+    let filePath = fileURLToPath(file);
 
-    // Let's put the end brace back where it was (we still have it to the direct right of us).
-    newText += `\n${paddingContentSwitchCase}`;
+    let newSwitchCode = utils.runAnalysisAfterSanityCheck(filePath, [
+      "codemod",
+      filePath,
+      range.start.line,
+      range.start.character,
+      "add-missing-cases",
+      allCasesAsOneLine,
+    ]);
 
     codeActions[file] = codeActions[file] || [];
     let codeAction: p.CodeAction = {
       title: `Insert missing cases`,
       edit: {
         changes: {
-          [file]: insertBeforeEndingChar(range, newText),
+          [file]: replaceText(range, newSwitchCode),
         },
       },
       diagnostics: [diagnostic],