Rename does not work properly with ppx_string

[ddickstein]

This bug was originally reported on ocaml/ocaml-lsp#1176, but it seems to be a problem with the response payload Merlin returns for ppx_string. Given a file with 1-indexed lines 242-246,

let f x =
  [%string
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \
     %{x}"]
;;

Here is the query & response with Merlin when the cursor is placed on the x argument and Merlin is asked to rename it to y:

# 0.11 occurrences - Occurrences paths
[
  {
    "start": { "line": 242, "col": 6 },
    "end": { "line": 242, "col": 7 },
    "under_cursor": true,
    "path": "x"
  }
]
# 0.11 New_merlin - run(result)
{
  "class": "return",
  "value": [
    { "start": { "line": 242, "col": 6 }, "end": { "line": 242, "col": 7 } },
    { "start": { "line": 244, "col": 4 }, "end": { "line": 245, "col": 10 } }
  ],
  ...
}

Applying these edits produces:

let f y =
  [%string
    y]
;;

(The :MerlinRename command in Vim does not actually apply this transformation faithfully, but ocaml-lsp does).

According to this comment, Merlin sometimes gives poor rename responses when there are location issues with the ppx. I see that ppx_string has
https://github.com/janestreet/ppx_string/blob/f2bf7353f9b1f4c92e180685018e88f25d3c3ec4/src/ppx_string.ml#L276-L277
Is this hiding the location information that Merlin needs?

[voodoos]

Yes, the AST output has a merlin.hide attribute since the generated locations are useless (for merlin) and should be ignored by occurrences (but apparently they are not).

For Merlin to work as expected, the locations in the generated AST should match with the original locations in the source.

I think two actions can be taken here:

• Merlin should not rely on hidden nodes to return occurrences to prevent incorrect renaming.
• ppx_string could improve the outputed locations to actually enable Merlin functionality.

[voodoos]

The merlin.locattribute might be useful in doing this, but the documentation does not mention that use case.

Doc: https://github.com/ocaml/merlin/blob/master/doc/dev/PROTOCOL.md#locations-in-ppx-rewriters