ocaml / merlin

Context sensitive completion for OCaml in Vim and Emacs
https://ocaml.github.io/merlin/
MIT License
1.59k stars 232 forks source link

Optional argument include by `case-analysis` #1770

Closed liam923 closed 4 months ago

liam923 commented 6 months ago

With the file test.ml:

let foo ?bar x = x

let () = foo ()

running

$ ocamlmerlin single case-analysis -start 3:10 -end 3:15 -filename test.ml < test.ml | jq ".value[1]"

Expected:

"match foo () with | () -> _"

Actual:

"match foo ?bar:None () with | () -> _"

I believe the root of this issue is in Untypeast. Looking at Destruct.destruct_expression, it logs the expression being destructed after passing it to Untypeast.untype_expression. For this program, it is:

# 0.01 destruct - node_expression
expression (test.ml[3,20+9]..[3,20+15])
  Pexp_apply
  expression (test.ml[3,20+9]..[3,20+12])
    Pexp_ident "foo" (test.ml[3,20+9]..[3,20+12])
  [
    <arg>
    Optional "bar"
      expression (_none_[0,0+-1]..[0,0+-1]) ghost
        Pexp_construct "None" (_none_[0,0+-1]..[0,0+-1]) ghost
        None
    <arg>
    Nolabel
      expression (test.ml[3,20+13]..[3,20+15])
        Pexp_construct "()" (test.ml[3,20+13]..[3,20+15])
        None
  ]

We can see here that the ~bar argument has snuck into our expression. I think solving this might involve some Merlin-specifc logic in Untypeast that detects optional arguments that users didn't supply and strips them out.

Version

On branch master (commit fa9b391529d1b01f091bd6d565efa2cee545d58c)

voodoos commented 6 months ago

Thanks for the report @liam923 !

cc our destruct expert @xvw