Incorrect tracking of identifier through inlcude module type

[liam923]

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 Foogoes 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

[voodoos]

Thanks for the very thorough report ! I will have a look.

[voodoos]

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 https://github.com/ocaml/ocaml/pull/13286

[voodoos]

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 https://github.com/ocaml/ocaml/pull/13308