Ignore hidden branches when listing occurrences (#1677)

from voodoos/issue1671-occurrences-hidden-nodes

.github/workflows/main.yml

@@ -44,6 +44,10 @@ jobs:
     # The type of runner that the job will run on
     runs-on: ${{ matrix.os }}
 
+    # Some tests requiring specific ppxes are disabled by default
+    env:
+      MERLIN_TESTS: all
+
     # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
       # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
@@ -59,6 +63,7 @@ jobs:
         run: |
           opam depext conf-jq --yes # opam depext bug
           opam pin menhirLib 20201216 --no-action
+          opam install --yes ppx_string ppx_compare
           opam install . --deps-only --with-test --yes
 
       - name: Build and test in release mode (windows)

CHANGES.md

@@ -7,6 +7,7 @@ unreleased
     - Fix regression causing crash when using ppxes under Windows (#1673)
     - Fix confusion between aliased modules and module types (#1676,
       fixes #1667)
+    - Ignore hidden branches when listing occurrences (#1677, fixes #1671)
   + editor modes
     - emacs: fix/improve keybindings (#1668, fixes #1386):
       Unbind <kbd>C-c C-r</kbd> (to avoid shadowing `tuareg-eval-region`)

src/analysis/browse_tree.ml

@@ -115,7 +115,10 @@ let all_occurrences path =
       | [] -> acc
       | paths -> (t, paths) :: acc
     in
-    List.fold_left ~f:aux ~init:acc (Lazy.force t.t_children)
+    if Browse_raw.has_attr ~name:"merlin.hide" t.t_node then
+      acc
+    else
+      List.fold_left ~f:aux ~init:acc (Lazy.force t.t_children)
   in
   aux []
 
@@ -131,7 +134,10 @@ let all_constructor_occurrences ({t_env = env; _},d) t =
         {d' with Location.txt = t} :: acc
       | _ -> acc
     in
-    List.fold_left ~f:aux ~init:acc (Lazy.force t.t_children)
+    if Browse_raw.has_attr ~name:"merlin.hide" t.t_node then
+      acc
+    else
+      List.fold_left ~f:aux ~init:acc (Lazy.force t.t_children)
   in
   aux [] t
 

src/kernel/mbrowse.ml

@@ -78,34 +78,32 @@ let drop_leaf t =
   | [] | [ _ ] -> None
   | _leaf :: parents -> Some parents
 
-let has_attr attr_name attrs =
-  List.exists ~f:(fun a ->
-    let (str,_) = Ast_helper.Attr.as_tuple a in
-    str.Location.txt = attr_name
-  ) attrs
+let is_hidden node =
+  Browse_raw.has_attr ~name:"merlin.hide" node
+
+let is_focus node =
+  Browse_raw.has_attr ~name:"merlin.focus" node
 
 let select_leafs pos root =
   let branches = ref [] in
   let rec select_child branch env node has_selected =
     let loc = node_merlin_loc node in
-    let attrs = Browse_raw.node_attributes node in
     if Location_aux.compare_pos pos loc = 0 &&
-       not (has_attr "merlin.hide" attrs)
+       not (is_hidden node)
     then
       (traverse ((env, node) :: branch); true)
     else
       has_selected
   and traverse branch =
     let env, node = leaf_node branch in
-    let attrs = Browse_raw.node_attributes node in
-    if (has_attr "merlin.focus" attrs) then (
+    if (is_focus node) then (
       branches := [];
       let has_leaves = fold_node (select_child branch) env node false in
       if not has_leaves then
         branches := [branch];
       raise Exit
     )
-    else if not (has_attr "merlin.hide" attrs) then (
+    else if not (is_hidden node) then (
       let has_leaves = fold_node (select_child branch) env node false in
       if not has_leaves then
         branches := branch :: !branches

src/ocaml/merlin_specific/browse_raw.ml

@@ -193,6 +193,13 @@ let node_attributes = function
   | Record_field (`Pattern obj,_,_) -> obj.pat_attributes
   | _ -> []
 
+let has_attr ~name node =
+  let attrs = node_attributes node in
+  List.exists ~f:(fun a ->
+    let (str,_) = Ast_helper.Attr.as_tuple a in
+    str.Location.txt = name
+  ) attrs
+
 let node_merlin_loc loc0 node =
   let attributes = node_attributes node in
   let loc =

src/ocaml/merlin_specific/browse_raw.mli

@@ -108,6 +108,7 @@ val node_update_env : Env.t -> node -> Env.t
 val node_real_loc : Location.t -> node -> Location.t
 val node_merlin_loc : Location.t -> node -> Location.t
 val node_attributes : node -> attribute list
+val has_attr : name:string -> node -> bool
 
 val string_of_node : node -> string
 

tests/test-dirs/with-ppx/dune

@@ -3,5 +3,5 @@
  (locks ../server-tests/merlin_server))
 
 (cram
- (applies_to issue1660-deriving-compare)
+ (applies_to issue1660-deriving-compare issue1671-string)
  (enabled_if (= %{env:MERLIN_TESTS=default} all)))

tests/test-dirs/with-ppx/issue1660-deriving-compare.t

@@ -28,9 +28,10 @@
   > (library
   >  (name main)
   >  (preprocess (pps ppx_compare)))
+  > EOF
 
   $ dune build
 
   $ $MERLIN single type-enclosing -position 2:7 \
   > -filename main.ml < main.ml |  jq '.value[0].type'
-  "type t =o | Bar"
+  "type t = Foo | Bar"

tests/test-dirs/with-ppx/issue1671-string.t

@@ -0,0 +1,65 @@
+  $ cat >dune-project <<EOF
+  > (lang dune 2.0)
+  > EOF
+
+  $ cat >main.ml <<EOF
+  > let f x =
+  >   [%string
+  >     "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \
+  >      %{x}"]
+  > ;;
+  > print_endline @@ f "42";;
+  > EOF
+
+  $ cat >dune <<EOF
+  > (executable
+  >  (name main)
+  >  (preprocess (pps ppx_string)))
+  > EOF
+
+  $ dune build @check 
+
+  $ dune exec ./main.exe
+  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa      42
+
+FIX upstream: locs issued by the ppx does not enable Merlin to work as expected
+  $ $MERLIN single type-enclosing -position 3:7 \
+  > -filename main.ml <main.ml
+  {
+    "class": "return",
+    "value": [
+      {
+        "start": {
+          "line": 1,
+          "col": 6
+        },
+        "end": {
+          "line": 3,
+          "col": 97
+        },
+        "type": "string -> string",
+        "tail": "no"
+      }
+    ],
+    "notifications": []
+  }
+
+Merlin should ignore hidden nodes in occurrences results
+  $ $MERLIN single occurrences -identifier-at 1:6 \
+  > -filename main.ml <main.ml
+  {
+    "class": "return",
+    "value": [
+      {
+        "start": {
+          "line": 1,
+          "col": 6
+        },
+        "end": {
+          "line": 1,
+          "col": 7
+        }
+      }
+    ],
+    "notifications": []
+  }