some snapshot refactoring and tweaking

zth · zth · commit d0bdbf05c3f3 · 2025-04-29T11:42:27.000+02:00
diff --git a/analysis/bin/main.ml b/analysis/bin/main.ml
@@ -130,10 +130,13 @@ let main () =
   | [_; "completion"; path; line; col; currentFile] ->
     printHeaderInfo path line col;
     if !Cfg.useRevampedCompletion then
-      let source = Files.readFile currentFile in
-      Commands.completionRevamped ~source ~debug ~path
-        ~pos:(int_of_string line, int_of_string col)
-        ~currentFile
+      match
+        Commands.completionRevamped ~debug ~path
+          ~pos:(int_of_string line, int_of_string col)
+          ~currentFile
+      with
+      | None -> ()
+      | Some (_, completablesText) -> print_endline completablesText
     else
       Commands.completion ~debug:true ~path
         ~pos:(int_of_string line, int_of_string col)
diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml
@@ -11,18 +11,17 @@ let completion ~(debug : bool) ~path ~pos ~currentFile =
   in
   completions |> Protocol.array |> print_endline
 
-let completionRevamped ?(source = None) ~debug ~path ~pos ~currentFile =
-  let completions =
-    match
-      Completions.getCompletionsRevamped ~source ~debug ~path ~pos ~currentFile
-    with
-    | None -> []
-    | Some (completions, full, _) ->
-      completions
-      |> List.map (CompletionBackEnd.completionToItem ~full)
-      |> List.map Protocol.stringifyCompletionItem
-  in
-  completions |> Protocol.array |> print_endline
+let completionRevamped ~debug ~path ~pos ~currentFile =
+  match Completions.getCompletionsRevamped ~debug ~pos ~currentFile ~path with
+  | None -> None
+  | Some (completable, completions, full, _) ->
+    Some
+      ( completable,
+        completions
+        |> List.map (CompletionBackEnd.completionToItem ~full)
+        |> List.map Protocol.stringifyCompletionItem
+        |> Protocol.array )
+
 let completionResolve ~path ~modulePath =
   (* We ignore the internal module path as of now because there's currently
      no use case for it. But, if we wanted to move resolving documentation
@@ -380,15 +379,38 @@ let test ~path ~debug =
              ^ string_of_int col);
             definition ~path ~pos:(line, col) ~debug:true
           | "com" ->
-            print_endline
-              ("Complete " ^ path ^ " " ^ string_of_int line ^ ":"
-             ^ string_of_int col);
             let currentFile = createCurrentFile () in
-            if !Cfg.useRevampedCompletion then
+            if !Cfg.useRevampedCompletion then (
               let source = Files.readFile currentFile in
-              completionRevamped ~source ~debug ~path ~pos:(line, col)
-                ~currentFile
-            else completion ~debug:true ~path ~pos:(line, col) ~currentFile;
+              let completions =
+                completionRevamped ~debug ~path ~pos:(line, col) ~currentFile
+              in
+              (match (completions, source) with
+              | None, _ ->
+                print_endline "Completion Frontend did not return completable"
+              | Some (completable, completionsText), Some text -> (
+                match SharedTypes.CompletableRevamped.try_loc completable with
+                | Some loc ->
+                  let range =
+                    CodeFence.
+                      {
+                        start = loc.Location.loc_start.pos_cnum;
+                        finish = loc.Warnings.loc_end.pos_cnum;
+                      }
+                  in
+                  Printf.printf "Found Completable: %s\n\n"
+                    (SharedTypes.CompletableRevamped.toString completable);
+                  CodeFence.format_code_snippet_cropped text (Some range) 3
+                  |> print_endline;
+                  print_endline completionsText
+                | None -> ())
+              | _ -> print_endline "ERR: Unexpected completion result");
+              ())
+            else (
+              print_endline
+                ("Complete " ^ path ^ " " ^ string_of_int line ^ ":"
+               ^ string_of_int col);
+              completion ~debug:true ~path ~pos:(line, col) ~currentFile);
             Sys.remove currentFile
           | "cre" ->
             let modulePath = String.sub rest 3 (String.length rest - 3) in
diff --git a/analysis/src/Completions.ml b/analysis/src/Completions.ml
@@ -21,7 +21,7 @@ let getCompletions (debug : bool) ~path ~pos ~currentFile ~forHover =
         in
         Some (completables, full, scope)))
 
-let getCompletionsRevamped ?(source = None) ~debug ~path ~pos ~currentFile =
+let getCompletionsRevamped ~debug ~pos ~currentFile ~path =
   let textOpt = Files.readFile currentFile in
   match textOpt with
   | None | Some "" -> None
@@ -30,32 +30,8 @@ let getCompletionsRevamped ?(source = None) ~debug ~path ~pos ~currentFile =
       CompletionFrontEndRevamped.completionWithParser ~debug ~path
         ~posCursor:pos ~currentFile ~text
     with
-    | None ->
-      source
-      |> Option.iter (fun _ ->
-             print_endline "Completion Frontend did not return completable");
-      None
+    | None -> None
     | Some (completable, scope) -> (
-      let _ =
-        match source with
-        | Some text -> (
-          match SharedTypes.CompletableRevamped.try_loc completable with
-          | Some loc ->
-            let range =
-              CodeFence.
-                {
-                  start = loc.Location.loc_start.pos_cnum;
-                  finish = loc.Warnings.loc_end.pos_cnum;
-                }
-            in
-            Printf.printf "Found Completable: %s\n\n"
-              (SharedTypes.CompletableRevamped.toString completable);
-            CodeFence.format_code_snippet_cropped text (Some range) 3
-            |> print_endline
-          | None -> ())
-        | None -> ()
-      in
-
       (* Only perform expensive ast operations if there are completables *)
       match Cmt.loadFullCmtFromPath ~path with
       | None -> None
@@ -66,4 +42,4 @@ let getCompletionsRevamped ?(source = None) ~debug ~path ~pos ~currentFile =
           |> CompletionBackEndRevamped.processCompletable ~debug ~full ~pos
                ~scope ~env
         in
-        Some (completables, full, scope)))
+        Some (completable, completables, full, scope)))
diff --git a/tests/analysis_new_tests/tests/snapshots.test.js b/tests/analysis_new_tests/tests/snapshots.test.js
@@ -16,21 +16,30 @@ const incrementalDir = path.join(
   import.meta.dirname,
   "./lib/bs/___incremental"
 );
+const snapshotDir = path.join(testFilesDir, "__snapshots__");
 
 // Recreate directories needed
-try {
-  await fs.access(incrementalDir);
-  await fs.rm(incrementalDir, { recursive: true });
-} catch (_) {}
-await fs.mkdir(incrementalDir, { recursive: true });
-
-try {
-  await fs.access(buildDir);
-  await fs.rm(buildDir, { recursive: true });
-} catch (_) {}
-await fs.mkdir(buildDir, { recursive: true });
-
-const resFiles = await glob("**/*.res", {
+async function ensureSnapshotDir() {
+  await fs.mkdir(snapshotDir, { recursive: true }); // Ensure snapshot dir exists
+}
+
+async function ensureIncrementalDir() {
+  try {
+    await fs.access(incrementalDir);
+    await fs.rm(incrementalDir, { recursive: true });
+  } catch (_) {}
+  await fs.mkdir(incrementalDir, { recursive: true });
+}
+
+async function ensureBuildDir() {
+  try {
+    await fs.access(buildDir);
+    await fs.rm(buildDir, { recursive: true });
+  } catch (_) {}
+  await fs.mkdir(buildDir, { recursive: true });
+}
+
+const resFilesPromise = glob("**/*.res", {
   cwd: testFilesDir,
   absolute: true,
 }).then((files) =>
@@ -40,6 +49,13 @@ const resFiles = await glob("**/*.res", {
   }))
 );
 
+const [resFiles] = await Promise.all([
+  resFilesPromise,
+  ensureSnapshotDir(),
+  ensureIncrementalDir(),
+  ensureBuildDir(),
+]);
+
 // Function to split test file contents into blocks
 const splitTestBlocks = (contents) => {
   const testBlocks = contents.split(/\/\/ == TEST:/);
@@ -154,9 +170,9 @@ resFiles.forEach((file) => {
       });
 
       // Construct snapshot path
-      const snapshotDir = path.join(testFilesDir, "__snapshots__");
-      await fs.mkdir(snapshotDir, { recursive: true }); // Ensure snapshot dir exists
-      const snapshotFileName = `${file.relativePath}_${block.description.replace(/\\s+/g, "_")}.snap`;
+      const snapshotFileName = `${
+        file.relativePath
+      }_${block.description.replace(/[^a-zA-Z0-9]+/g, "_")}.snap`;
       const snapshotPath = path.join(snapshotDir, snapshotFileName);
 
       // Use Vitest's expect().toMatchFileSnapshot()
diff --git a/tests/analysis_new_tests/tests/test_files/__snapshots__/RecordFieldCompletions.res_Record_field_completion_in_nested_record.snap b/tests/analysis_new_tests/tests/test_files/__snapshots__/RecordFieldCompletions.res_Record_field_completion_in_nested_record.snap
@@ -0,0 +1,22 @@
+Found Completable: Cexpression
+
+1 + // Record field completion in nested record
+2 + let x = TestTypeDefs.nestedTestRecord.
+            ----------------------------- 
+3 + //                                    ^com
+4 + 
+
+[{
+    "label": "test",
+    "kind": 5,
+    "tags": [],
+    "detail": "bool",
+    "documentation": {"kind": "markdown", "value": "```rescript\ntest: bool\n```\n\n```rescript\ntype nestedTestRecord = {\n  test: bool,\n  nested: {name: string, oneMoreLevel: {here: bool}},\n}\n```"}
+  }, {
+    "label": "nested",
+    "kind": 5,
+    "tags": [],
+    "detail": "\\\"nestedTestRecord.nested\"",
+    "documentation": {"kind": "markdown", "value": "```rescript\nnested: \\\"nestedTestRecord.nested\"\n```\n\n```rescript\ntype nestedTestRecord = {\n  test: bool,\n  nested: {name: string, oneMoreLevel: {here: bool}},\n}\n```"}
+  }]
+
diff --git a/tests/analysis_new_tests/tests/test_files/__snapshots__/RecordFieldCompletions.res_Record_field_completion_in_nested_record_another_level.snap b/tests/analysis_new_tests/tests/test_files/__snapshots__/RecordFieldCompletions.res_Record_field_completion_in_nested_record_another_level.snap
@@ -0,0 +1,22 @@
+Found Completable: Cexpression
+
+1 + // Record field completion in nested record, another level
+2 + let x = TestTypeDefs.nestedTestRecord.nested.
+            ------------------------------------ 
+3 + //                                           ^com
+4 + 
+
+[{
+    "label": "name",
+    "kind": 5,
+    "tags": [],
+    "detail": "string",
+    "documentation": {"kind": "markdown", "value": "```rescript\nname: string\n```\n\n```rescript\ntype \\\"nestedTestRecord.nested\" = {\n  name: string,\n  oneMoreLevel: {here: bool},\n}\n```"}
+  }, {
+    "label": "oneMoreLevel",
+    "kind": 5,
+    "tags": [],
+    "detail": "\\\"nestedTestRecord.nested.oneMoreLevel\"",
+    "documentation": {"kind": "markdown", "value": "```rescript\noneMoreLevel: \\\"nestedTestRecord.nested.oneMoreLevel\"\n```\n\n```rescript\ntype \\\"nestedTestRecord.nested\" = {\n  name: string,\n  oneMoreLevel: {here: bool},\n}\n```"}
+  }]
+
diff --git a/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_array.snap b/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_array.snap
@@ -0,0 +1,22 @@
+Found Completable: Cpattern
+
+1 + // Empty case, array
+2 + let someStringArr = ["hello"]
+3 + 
+4 + let x = switch someStringArr {
+                   -------------  
+5 +   | 
+6 + //  ^com
+7 + }
+
+[{
+    "label": "[]",
+    "kind": 12,
+    "tags": [],
+    "detail": "array<string>",
+    "documentation": null,
+    "sortText": "A",
+    "insertText": "[$0]",
+    "insertTextFormat": 2
+  }]
+
diff --git a/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_bool.snap b/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_bool.snap
@@ -0,0 +1,29 @@
+Found Completable: Cpattern
+
+1 + // Empty case, bool
+2 + let x = switch true {
+                   ----  
+3 +   | 
+4 + //  ^com
+5 + }
+
+[{
+    "label": "true",
+    "kind": 12,
+    "tags": [],
+    "detail": "bool",
+    "documentation": null,
+    "sortText": "A",
+    "insertText": "true",
+    "insertTextFormat": 2
+  }, {
+    "label": "false",
+    "kind": 12,
+    "tags": [],
+    "detail": "bool",
+    "documentation": null,
+    "sortText": "A",
+    "insertText": "false",
+    "insertTextFormat": 2
+  }]
+
diff --git a/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_record.snap b/tests/analysis_new_tests/tests/test_files/__snapshots__/SwitchCaseCompletions.res_Empty_case_record.snap
@@ -0,0 +1,20 @@
+Found Completable: Cpattern
+
+1 + // Empty case, record
+2 + let x = switch TestTypeDefs.nestedTestRecord {
+                   -----------------------------  
+3 +   | 
+4 + //  ^com
+5 + }
+
+[{
+    "label": "{}",
+    "kind": 12,
+    "tags": [],
+    "detail": "TestTypeDefs.nestedTestRecord",
+    "documentation": {"kind": "markdown", "value": "```rescript\ntype nestedTestRecord = {\n  test: bool,\n  nested: {name: string, oneMoreLevel: {here: bool}},\n}\n```"},
+    "sortText": "A",
+    "insertText": "{$0}",
+    "insertTextFormat": 2
+  }]
+
