Fix nested pattern completion with trailing comma (#906)

* only check for pattern completions if the associated expression does not have the cursor

* changelog

Author: zth

CHANGELOG.md

@@ -17,6 +17,10 @@
 - Prefer Core's `RegExp` when Core is open and completing for regexp functions. https://github.com/rescript-lang/rescript-vscode/pull/903
 - Add `%re("")` to the completions list when completing in a position where a regexp value is expected. https://github.com/rescript-lang/rescript-vscode/pull/903
 
+#### :bug: Bug Fix
+
+- Fix issue with completion in nested patterns that would make it not possible to complete for new record fields via trailing commas in certain situations. https://github.com/rescript-lang/rescript-vscode/pull/906
+
 ## 1.36.0
 
 #### :bug: Bug Fix

analysis/src/CompletionFrontEnd.ml

@@ -494,6 +494,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
         contextPath )
     with
     | Some (prefix, nestedPattern), Some ctxPath ->
+      if Debug.verbose () then
+        Printf.printf "[completePattern] found pattern that can be completed\n";
       setResult
         (Completable.Cpattern
            {
@@ -910,7 +912,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
       cases
       |> List.iter (fun (case : Parsetree.case) ->
              let oldScope = !scope in
-             completePattern ?contextPath:ctxPath case.pc_lhs;
+             if locHasCursor case.pc_rhs.pexp_loc = false then
+               completePattern ?contextPath:ctxPath case.pc_lhs;
              scopePattern ?contextPath:ctxPath case.pc_lhs;
              Ast_iterator.default_iterator.case iterator case;
              scope := oldScope);
@@ -1182,7 +1185,8 @@ let completionWithParser1 ~currentFile ~debug ~offset ~path ~posCursor
           (match defaultExpOpt with
           | None -> ()
           | Some defaultExp -> iterator.expr iterator defaultExp);
-          completePattern ?contextPath:argContextPath pat;
+          if locHasCursor e.pexp_loc = false then
+            completePattern ?contextPath:argContextPath pat;
           scopePattern ?contextPath:argContextPath pat;
           iterator.pat iterator pat;
           iterator.expr iterator e;

analysis/src/CompletionPatterns.ml

@@ -35,8 +35,14 @@ let rec traverseTupleItems tupleItems ~nextPatternPath ~resultFromFoundItemNum
 
 and traversePattern (pat : Parsetree.pattern) ~patternPath ~locHasCursor
     ~firstCharBeforeCursorNoWhite ~posBeforeCursor =
-  let someIfHasCursor v =
-    if locHasCursor pat.Parsetree.ppat_loc then Some v else None
+  let someIfHasCursor v debugId =
+    if locHasCursor pat.Parsetree.ppat_loc then (
+      if Debug.verbose () then
+        Printf.printf
+          "[traversePattern:someIfHasCursor] '%s' has cursor, returning \n"
+          debugId;
+      Some v)
+    else None
   in
   match pat.ppat_desc with
   | Ppat_constant _ | Ppat_interval _ -> None
@@ -57,21 +63,28 @@ and traversePattern (pat : Parsetree.pattern) ~patternPath ~locHasCursor
                   ~firstCharBeforeCursorNoWhite ~posBeforeCursor)
     in
     match orPatWithItem with
-    | None when isPatternHole p1 || isPatternHole p2 -> Some ("", patternPath)
+    | None when isPatternHole p1 || isPatternHole p2 ->
+      if Debug.verbose () then
+        Printf.printf
+          "[traversePattern] found or-pattern that was pattern hole\n";
+      Some ("", patternPath)
     | v -> v)
   | Ppat_any ->
     (* We treat any `_` as an empty completion. This is mainly because we're
        inserting `_` in snippets and automatically put the cursor there. So
        letting it trigger an empty completion improves the ergonomics by a
        lot. *)
-    someIfHasCursor ("", patternPath)
-  | Ppat_var {txt} -> someIfHasCursor (txt, patternPath)
+    someIfHasCursor ("", patternPath) "Ppat_any"
+  | Ppat_var {txt} -> someIfHasCursor (txt, patternPath) "Ppat_var"
   | Ppat_construct ({txt = Lident "()"}, None) ->
     (* switch s { | (<com>) }*)
-    someIfHasCursor ("", patternPath @ [Completable.NTupleItem {itemNum = 0}])
+    someIfHasCursor
+      ("", patternPath @ [Completable.NTupleItem {itemNum = 0}])
+      "Ppat_construct()"
   | Ppat_construct ({txt = Lident prefix}, None) ->
-    someIfHasCursor (prefix, patternPath)
-  | Ppat_variant (prefix, None) -> someIfHasCursor ("#" ^ prefix, patternPath)
+    someIfHasCursor (prefix, patternPath) "Ppat_construct(Lident)"
+  | Ppat_variant (prefix, None) ->
+    someIfHasCursor ("#" ^ prefix, patternPath) "Ppat_variant"
   | Ppat_array arrayPatterns ->
     let nextPatternPath = [Completable.NArray] @ patternPath in
     if List.length arrayPatterns = 0 && locHasCursor pat.ppat_loc then
@@ -94,6 +107,7 @@ and traversePattern (pat : Parsetree.pattern) ~patternPath ~locHasCursor
     (* Empty fields means we're in a record body `{}`. Complete for the fields. *)
     someIfHasCursor
       ("", [Completable.NRecordBody {seenFields = []}] @ patternPath)
+      "Ppat_record(empty)"
   | Ppat_record (fields, _) -> (
     let fieldWithCursor = ref None in
     let fieldWithPatHole = ref None in
@@ -125,10 +139,12 @@ and traversePattern (pat : Parsetree.pattern) ~patternPath ~locHasCursor
           ( "",
             [Completable.NFollowRecordField {fieldName = fname}] @ patternPath
           )
+          "patternhole"
       | Ppat_var {txt} ->
         (* A var means `{s}` or similar. Complete for fields. *)
         someIfHasCursor
           (txt, [Completable.NRecordBody {seenFields}] @ patternPath)
+          "Ppat_var #2"
       | _ ->
         f
         |> traversePattern
@@ -145,6 +161,7 @@ and traversePattern (pat : Parsetree.pattern) ~patternPath ~locHasCursor
       | Some ',' ->
         someIfHasCursor
           ("", [Completable.NRecordBody {seenFields}] @ patternPath)
+          "firstCharBeforeCursorNoWhite:,"
       | _ -> None))
   | Ppat_construct
       ( {txt},

analysis/tests/src/CompletionPattern.res

@@ -219,3 +219,14 @@ let res: result<someVariant, somePolyVariant> = Ok(One)
 
 // switch res { | Error() }
 //                      ^com
+
+@react.component
+let make = (~thing: result<someVariant, unit>) => {
+  switch thing {
+  | Ok(Three(r, _)) =>
+    let _x = r
+  // switch r { | {first, }}
+  //                     ^com
+  | _ => ()
+  }
+}

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

@@ -1177,3 +1177,33 @@ Path res
     "insertTextFormat": 2
   }]
 
+Complete src/CompletionPattern.res 227:25
+posCursor:[227:25] posNoWhite:[227:24] Found expr:[223:11->231:1]
+posCursor:[227:25] posNoWhite:[227:24] Found expr:[223:12->231:1]
+posCursor:[227:25] posNoWhite:[227:24] Found expr:[226:4->227:28]
+posCursor:[227:25] posNoWhite:[227:24] Found pattern:[227:18->227:27]
+Completable: Cpattern Value[r]->recordBody
+Package opens Pervasives.JsxModules.place holder
+Resolved opens 1 pervasives
+ContextPath Value[r]
+Path r
+[{
+    "label": "second",
+    "kind": 5,
+    "tags": [],
+    "detail": "second: (bool, option<someRecord>)\n\nsomeRecord",
+    "documentation": null
+  }, {
+    "label": "optThird",
+    "kind": 5,
+    "tags": [],
+    "detail": "optThird: option<[#first | #second(someRecord)]>\n\nsomeRecord",
+    "documentation": null
+  }, {
+    "label": "nest",
+    "kind": 5,
+    "tags": [],
+    "detail": "nest: nestedRecord\n\nsomeRecord",
+    "documentation": null
+  }]
+