Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complete-prefix short-circuits too eagerly when deciding whether to return only record field names #1742

Open
ncik-roberts opened this issue Mar 11, 2024 · 0 comments
Labels

Comments

@ncik-roberts
Copy link
Contributor

ncik-roberts commented Mar 11, 2024

A fuller test for the current behavior is at: https://github.com/ncik-roberts/merlin/tree/complete-prefix-record-field-test

complete-prefix is used for autocompletion. One decision it makes is whether to return only record field names and nothing else. This issue describes a case where complete-prefix decides too eagerly that it should only return record field names, when actually a record field name could never work. The suggestion I make is that merlin avoid the overeager short-circuiting here.

Example

Let's take this program as an running example:

module Outer = struct
 let x = 3
 module Inner = struct
     let y = 4
    type t = { field : int }
    let default = { field = 3 }
  end
end

let create (_ : Outer.Inner.t) = ()

let () = create {
               (* ^ *)

Current behavior

Text at cursor complete-prefix results
field
O Outer
Outer. Outer.field
Outer.I Outer.Inner

The specific heuristic that Merlin uses is:

  • If the cursor is in a position inside a record where a field name could occur,
  • and the last component of the identifier under the cursor is a prefix of one of the record's field names,
  • then return only field names that match the prefix

The second bullet point is overeager. It makes merlin return Outer.field as the only completion candidate for the prefix Outer., even though Outer.field will never type-check:

let () = create { Outer.field }
(* Error: Unbound record field Outer.field *)

It would be better to not short-circuit here and provide Outer.Inner as a completion candidate. I propose this behavior:

Proposed behavior

Text at cursor complete-prefix results
field
O Outer
Outer. Outer.field, Outer.Inner
Outer.I Outer.Inner

It would be even better to not return Outer.field in the Outer. case, but I expect this to be somewhat harder to implement

Additional example

Similar concerns apply to record field projection:

let f (x : Outer.Inner.t) = x.
                           (* ^ *)

This has an identical "Current behavior"/"Proposed behavior" to the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants