Incorrect tracking of identifier through inlcude module type
liam923 opened this issue · 3 comments
Here is a cram test of the issue:
Create a module with an mli file
$ cat > foo.ml << EOF
> type t = Foo
> module Bar = struct
> type t = Bar
> end
> EOF
$ cat > foo.mli << EOF
> module Bar : sig
> type t
> end
> type t
> EOF
$ $OCAMLC -c -bin-annot foo.mli
$ $OCAMLC -c -bin-annot foo.ml
Locate the Bar on line 4
$ cat > test1.ml << EOF
> module type Foo = sig
> include module type of Foo
> module Bar : sig
> include module type of Bar
> end
> end
> EOF
The expected location is 2:7 of foo.ml, but it instead goes to 1:9, which is the
constructor Foo
$ $MERLIN single locate -position 4:28 -look-for ml -filename test1.ml < test1.ml | jq .value
{
"file": "$TESTCASE_ROOT/foo.ml",
"pos": {
"line": 1,
"col": 9
}
}
Locate the Bar on line 3
$ cat > test2.ml << EOF
> include Foo
> module Bar = struct
> include Bar
> end
warning: here-document at line 1 delimited by end-of-file (wanted `EOF')
Correctly returns 2:7
$ $MERLIN single locate -position 3:12 -look-for ml -filename test2.ml < test2.ml | jq .value
{
"file": "$TESTCASE_ROOT/foo.ml",
"pos": {
"line": 2,
"col": 7
}
}
Locate the Foo.Bar on line 1
$ cat > test3.ml << EOF
> include module type of Foo.Bar
> EOF
Correctly returns 2:7
$ $MERLIN single locate -position 1:28 -look-for ml -filename test3.ml < test3.ml | jq .value
{
"file": "$TESTCASE_ROOT/foo.ml",
"pos": {
"line": 2,
"col": 7
}
}
Locating the Bar
originating from an include module type of Foo
goes to the wrong location. Looking at the logs for the failing case, we see:
# 0.01 env-lookup - env_lookup
found: 'Bar/279[1]' in namespace module with decl_uid Foo.1
at loc File "foo.mli", line 1, characters 0-29
# 0.01 locate - shape_of_path
initial: <Foo.1>
# 0.01 locate - shape_of_path
reduced: Resolved: Foo.1
Foo.1
is correct in the sense that Bar
is Foo.1
according to foo.mli
, but Foo.1
in foo.ml
is the constructor Foo
, which is what Merlin incorrectly returns.
Compare this to the logs in the successful cases:
# 0.01 env-lookup - env_lookup
found: 'Bar/279[1]' in namespace module with decl_uid Foo.1
at loc File "foo.mli", line 1, characters 0-29
# 0.01 locate - shape_of_path
initial: CU Foo . "Bar"[module]
# 0.01 locate - read_unit_shape
inspecting Foo
# 0.01 stat_cache - reuse cache
$TESTCASE_ROOT
# 0.01 File_cache(Exists_in_directory) - read
reading "$TESTCASE_ROOT" from disk
# 0.01 stat_cache - reuse cache
$TESTCASE_ROOT/foo.cmt
# 0.01 File_cache(Cmt_cache) - read
reusing "$TESTCASE_ROOT/foo.cmt"
# 0.01 locate - File_switching.move_to
file: $TESTCASE_ROOT/foo.cmt
digest: 4fd51ef55d4eb3719a2531fcdd6bff07
# 0.01 locate - File_switching.move_to
file: foo.ml
digest: 4fd51ef55d4eb3719a2531fcdd6bff07
# 0.01 locate - read_unit_shape
shapes loaded for Foo
# 0.01 locate - shape_of_path
reduced: Resolved: Foo.4
Thanks for the very thorough report ! I will have a look.
Foo.1 is correct in the sense that Bar is Foo.1 according to foo.mli, but Foo.1 in foo.ml is the constructor Foo, which is what Merlin incorrectly returns.
This is a case of "the uid doesn't indicate if it comes from the implementation or the interface" and we should be able to provide a robust fix soon thanks to ocaml/ocaml#13286
Given how locate works right now, I would expect the correct result of locating Bar
in the failing test to be 1:9 in foo.mli
since only the module type of Foo
was included and not the module implementation itself.
We might, however, be more clever at some point by looking at the new information linking declaration together in 5.3 ocaml/ocaml#13308