From e2fdd66d109280e9b55cb892b5f2710a4a472010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Fri, 12 Apr 2024 10:06:07 +0200 Subject: [PATCH 1/2] test: reproduce issue #1748 --- .../locate/ambiguity/homonyms.t/bar.ml | 3 ++ .../locate/ambiguity/homonyms.t/foo.ml | 1 + .../locate/ambiguity/homonyms.t/foo.mli | 1 + .../locate/ambiguity/homonyms.t/import.ml | 1 + .../locate/ambiguity/homonyms.t/run.t | 39 +++++++++++++++++++ 5 files changed, 45 insertions(+) create mode 100644 tests/test-dirs/locate/ambiguity/homonyms.t/bar.ml create mode 100644 tests/test-dirs/locate/ambiguity/homonyms.t/foo.ml create mode 100644 tests/test-dirs/locate/ambiguity/homonyms.t/foo.mli create mode 100644 tests/test-dirs/locate/ambiguity/homonyms.t/import.ml create mode 100644 tests/test-dirs/locate/ambiguity/homonyms.t/run.t diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/bar.ml b/tests/test-dirs/locate/ambiguity/homonyms.t/bar.ml new file mode 100644 index 0000000000..0e714f1495 --- /dev/null +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/bar.ml @@ -0,0 +1,3 @@ +module Import = struct + type hello = string +end diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/foo.ml b/tests/test-dirs/locate/ambiguity/homonyms.t/foo.ml new file mode 100644 index 0000000000..1a3c8af895 --- /dev/null +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/foo.ml @@ -0,0 +1 @@ +open! Import diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/foo.mli b/tests/test-dirs/locate/ambiguity/homonyms.t/foo.mli new file mode 100644 index 0000000000..1a3c8af895 --- /dev/null +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/foo.mli @@ -0,0 +1 @@ +open! Import diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/import.ml b/tests/test-dirs/locate/ambiguity/homonyms.t/import.ml new file mode 100644 index 0000000000..d0037b9428 --- /dev/null +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/import.ml @@ -0,0 +1 @@ +include Bar diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/run.t b/tests/test-dirs/locate/ambiguity/homonyms.t/run.t new file mode 100644 index 0000000000..496597e94d --- /dev/null +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/run.t @@ -0,0 +1,39 @@ +This test reproduces issue #1748 + + $ cat bar.ml + module Import = struct + type hello = string + end + + $ cat import.ml + include Bar + + $ cat foo.mli + open! Import + + $ cat foo.ml + open! Import + + $ $OCAMLC -c -bin-annot bar.ml import.ml foo.mli foo.ml + +Merlin correctly jump to the Import module. Not the one in Bar. + $ $MERLIN single locate -position 1:10 -look-for implementation \ + > -filename foo.ml < foo.ml | jq '.value' + { + "file": "$TESTCASE_ROOT/import.ml", + "pos": { + "line": 1, + "col": 0 + } + } + +FIXME: It is incorrect to jump to the `Bar.Import` module here: + $ $MERLIN single locate -position 1:10 -look-for implementation \ + > -filename foo.mli < foo.mli | jq '.value' + { + "file": "$TESTCASE_ROOT/bar.ml", + "pos": { + "line": 1, + "col": 0 + } + } From 1fafb75ba79962f4fb6f0b7195881f19ca6fd99f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= Date: Fri, 12 Apr 2024 10:42:25 +0200 Subject: [PATCH 2/2] Synthesize a node for module ident of [open_description] --- CHANGES.md | 5 ++++- src/ocaml/merlin_specific/browse_raw.ml | 16 +++++++++++++++- .../test-dirs/locate/ambiguity/homonyms.t/run.t | 6 +++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f522320c42..f00ce37290 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,8 +3,11 @@ merlin NEXT_VERSION + merlin binary - destruct: Removal of residual patterns (#1737, fixes #1560) - - Do not erase fields' names when destructing punned record fields (#1734, + - Do not erase fields' names when destructing punned record fields (#1734, fixes #1661) + - locate: fix an issue when locating the module ident in + an[open_description]. Merlin was using the wrong environment due to the + absence of a node representing the ident. (#1750, fixes #1748) merlin 4.14 =========== diff --git a/src/ocaml/merlin_specific/browse_raw.ml b/src/ocaml/merlin_specific/browse_raw.ml index 88caa4bed6..25e75d2f0a 100644 --- a/src/ocaml/merlin_specific/browse_raw.ml +++ b/src/ocaml/merlin_specific/browse_raw.ml @@ -688,7 +688,21 @@ let of_node = function | Module_binding_name _ -> id_fold | Module_declaration_name _ -> id_fold | Module_type_declaration_name _ -> id_fold - | Open_description _ -> id_fold + | Open_description od -> + (* We synthezise a [module_ident] node from the [open] infos so that Merlin + does not use the open resulting environment to answer queries about the + module ident. Without this synthetic node Merlin would select the entire + open node when the cursor is hover the ident. See issue #1748 *) + let path, lid = od.open_expr in + let m = { + mod_desc = Tmod_ident (path, lid); + mod_loc = lid.loc; + mod_type = Mty_ident path; + mod_env = Env.empty; + mod_attributes = []; + } + in + app (Module_expr m) | Open_declaration od -> app (Module_expr od.open_expr) | Include_declaration i -> diff --git a/tests/test-dirs/locate/ambiguity/homonyms.t/run.t b/tests/test-dirs/locate/ambiguity/homonyms.t/run.t index 496597e94d..b5fbd3ecc1 100644 --- a/tests/test-dirs/locate/ambiguity/homonyms.t/run.t +++ b/tests/test-dirs/locate/ambiguity/homonyms.t/run.t @@ -18,7 +18,7 @@ This test reproduces issue #1748 Merlin correctly jump to the Import module. Not the one in Bar. $ $MERLIN single locate -position 1:10 -look-for implementation \ - > -filename foo.ml < foo.ml | jq '.value' + > -filename foo.ml < foo.ml | jq '.value' { "file": "$TESTCASE_ROOT/import.ml", "pos": { @@ -27,11 +27,11 @@ Merlin correctly jump to the Import module. Not the one in Bar. } } -FIXME: It is incorrect to jump to the `Bar.Import` module here: +Same in the mli: $ $MERLIN single locate -position 1:10 -look-for implementation \ > -filename foo.mli < foo.mli | jq '.value' { - "file": "$TESTCASE_ROOT/bar.ml", + "file": "$TESTCASE_ROOT/import.ml", "pos": { "line": 1, "col": 0