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

Jump to incorrect module #1748

Open
liam923 opened this issue Apr 9, 2024 · 2 comments
Open

Jump to incorrect module #1748

liam923 opened this issue Apr 9, 2024 · 2 comments
Labels

Comments

@liam923
Copy link
Contributor

liam923 commented Apr 9, 2024

Hi, I've recently joined the Jane Street compiler team. We've had a user report a bug, which I've included reproduction steps for below. Essentially, there is an import.ml file, which includes another module named Import. This leads to a Import.Import module existing in scope. When another file contains open Import, Merlin traces the location of Import to the correct location when the file is a .ml file but incorrectly traces to Import.Import when the file is a .mli file.

Reproduction Steps

  1. Download and extract jump_to_wrong_import.tar.gz
  2. cd into the jump_to_wrong_import directory
  3. dune build
  4. ocamlmerlin single locate -position 1:10 -look-for implementation -filename lib/foo.mli < lib/foo.mli

This gives:

{"class":"return","value":{"file":"/path/to/jump_to_wrong_import/lib/bar.ml","pos":{"line":1,"col":0}},"notifications":[],"timing":{"clock":20,"cpu":4,"query":2,"pp":0,"reader":0,"ppx":0,"typer":2,"error":0},"heap_mbytes":1,"cache":{"reader_phase":"miss","ppx_phase":"miss","typer":"miss","cmt":{"hit":1,"miss":3},"cmi":{"hit":0,"miss":4}},"query_num":0}

Compare this to ocamlmerlin single locate -position 1:10 -look-for implementation -filename lib/foo.ml < lib/foo.ml, which gives the expected:

{"class":"return","value":{"file":"/path/to/jump_to_wrong_import/lib/import.ml","pos":{"line":1,"col":0}},"notifications":[],"timing":{"clock":18,"cpu":4,"query":2,"pp":0,"reader":0,"ppx":0,"typer":2,"error":0},"heap_mbytes":1,"cache":{"reader_phase":"miss","ppx_phase":"miss","typer":"miss","cmt":{"hit":1,"miss":2},"cmi":{"hit":0,"miss":4}},"query_num":0}

Version info:

Full opam env:

$ opam list -i --columns="name,version"
# Packages matching: installed
# Name                # Version
astring               0.8.5
base                  v0.16.3
base-bigarray         base
base-bytes            base
base-threads          base
base-unix             base
camlp-streams         5.0.1
chrome-trace          3.14.2
cmdliner              1.2.0
conf-bash             1
cppo                  1.6.9
csexp                 1.5.2
dune                  3.14.2
dune-build-info       3.14.2
dune-configurator     3.14.2
dune-rpc              3.14.2
dyn                   3.14.2
either                1.0.0
fiber                 3.7.0
fix                   20230505
fpath                 0.7.3
menhir                20201216
menhirLib             20201216
menhirSdk             20201216
merlin-lib            4.14-414
ocaml                 4.14.2
ocaml-base-compiler   4.14.2
ocaml-config          2
ocaml-lsp-server      1.17.0
ocaml-options-vanilla 1
ocaml-version         3.6.5
ocamlbuild            0.14.3
ocamlc-loc            3.14.2
ocamlfind             1.9.6
ocamlformat           0.26.1
ocamlformat-lib       0.26.1
ocamlformat-rpc-lib   0.26.1
ocp-indent            1.8.1
ordering              3.14.2
pp                    1.2.0
ppx_yojson_conv_lib   v0.16.0
re                    1.11.0
result                1.5
seq                   base
sexplib0              v0.16.0
spawn                 v0.15.1
stdio                 v0.16.0
stdune                3.14.2
topkg                 1.0.7
uucp                  15.1.0
uuseg                 15.1.0
uutf                  1.0.3
xdg                   3.14.2
yojson                2.1.2
voodoos added a commit to voodoos/merlin that referenced this issue Apr 12, 2024
@voodoos
Copy link
Collaborator

voodoos commented Apr 12, 2024

Thanks for the precise reproduction, I am adding it to the test suite in #1750

@voodoos
Copy link
Collaborator

voodoos commented Apr 12, 2024

I also pushed a potential fix for the issue in the reproduction PR.

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