From 8ab2a5f35f0ccec8e867d6b43d14b475713b2747 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 29 Jan 2024 21:23:23 +0100 Subject: [PATCH 1/2] only check for pattern completions if the associated expression does not have the cursor --- analysis/src/CompletionFrontEnd.ml | 8 +++-- analysis/src/CompletionPatterns.ml | 33 ++++++++++++++----- analysis/tests/src/CompletionPattern.res | 11 +++++++ .../src/expected/CompletionPattern.res.txt | 30 +++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/analysis/src/CompletionFrontEnd.ml b/analysis/src/CompletionFrontEnd.ml index 4f83dde1a..053787576 100644 --- a/analysis/src/CompletionFrontEnd.ml +++ b/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; diff --git a/analysis/src/CompletionPatterns.ml b/analysis/src/CompletionPatterns.ml index 074cdd90c..296e457c9 100644 --- a/analysis/src/CompletionPatterns.ml +++ b/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 { | () }*) - 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}, diff --git a/analysis/tests/src/CompletionPattern.res b/analysis/tests/src/CompletionPattern.res index 82e5f3566..763903a03 100644 --- a/analysis/tests/src/CompletionPattern.res +++ b/analysis/tests/src/CompletionPattern.res @@ -219,3 +219,14 @@ let res: result = Ok(One) // switch res { | Error() } // ^com + +@react.component +let make = (~thing: result) => { + switch thing { + | Ok(Three(r, _)) => + let _x = r + // switch r { | {first, }} + // ^com + | _ => () + } +} diff --git a/analysis/tests/src/expected/CompletionPattern.res.txt b/analysis/tests/src/expected/CompletionPattern.res.txt index 850c64919..6cb2a79ba 100644 --- a/analysis/tests/src/expected/CompletionPattern.res.txt +++ b/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)\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 + }] + From 4b4bbb041afc691267072feb4e11c88a7893d258 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Mon, 29 Jan 2024 21:26:08 +0100 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4dbd9603..4afa6060d 100644 --- a/CHANGELOG.md +++ b/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