bug#60407: [PATCH] Update go-ts-mode to use Imenu facility

[Evgeni Kolev]

Hi Randy, thank you for your feedback!

I'm providing an updated patch below. I tested with "type Quack int"
and other cases such as:
    type MyInt = int
    type Option func(s string)
    type List[T any] struct {
        head, tail *element[T]
    }
    type Number interface {
        int64 | float64
    }

After experimenting, I decided to add additional Imenu categories:
"Alias" and "Type".
So the final list of newly added categories is "Method", "Struct",
"Interface", "Alias" and "Type".

Structs and interfaces are technically a private case for "Type", but
are put in their own Imenu category.
Hence the "Type" category now holds all types except structs,
interfaces and aliases (aliases have their own tree sitter type
defined in Go's grammar.js).

For reference, eglot's Imenu uses category "Class" instead of "Type".
But I decided to not replicate this behavior - "Class" is not a widely
used Go concept.
However, I decided to replicate another eglot behaviour - prefixing
the method names with the type of the receiver (for example,
"(rect).area" for "func (r rect) area() float64...").

I've also addressed the other comments. I'm a bit unsure how the git
commit should be formatted - the part of the message which describes
changed functions/files.

Please let me know if the patch can be improved. The patch is below.

commit a96e70052a79881ac666ab699ffd63ed916eaf83
Author: Evgeni Kolev <evgenysw@gmail.com>
Date:   Thu Dec 29 17:49:40 2022 +0200

    Improve go-ts-mode Imenu

    The Imenu items are extended to support "Method", "Struct",
    "Interface", "Alias" and "Type".

    go-ts-mode is updated to use the Imenu facility added in commit
    b39dc7ab27a696a8607ab859aeff3c71509231f5.

    * lisp/progmodes/go-ts-mode.el (go-ts-mode--imenu-1) (go-ts-mode--imenu):
    Remove functions.
    (go-ts-mode--defun-name) (go-ts-mode--interface-node-p)
    (go-ts-mode--struct-node-p) (go-ts-mode--other-type-node-p)
    (go-ts-mode--alias-node-p): New functions.
    (go-ts-mode): Improve Imenu settings.

diff --git a/lisp/progmodes/go-ts-mode.el b/lisp/progmodes/go-ts-mode.el
index 1d6a8a30db5..d91b555e03e 100644
--- a/lisp/progmodes/go-ts-mode.el
+++ b/lisp/progmodes/go-ts-mode.el
@@ -173,44 +173,6 @@ go-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `go-ts-mode'.")

-(defun go-ts-mode--imenu ()
-  "Return Imenu alist for the current buffer."
-  (let* ((node (treesit-buffer-root-node))
-         (func-tree (treesit-induce-sparse-tree
-                     node "function_declaration" nil 1000))
-         (type-tree (treesit-induce-sparse-tree
-                     node "type_spec" nil 1000))
-         (func-index (go-ts-mode--imenu-1 func-tree))
-         (type-index (go-ts-mode--imenu-1 type-tree)))
-    (append
-     (when func-index `(("Function" . ,func-index)))
-     (when type-index `(("Type" . ,type-index))))))
-
-(defun go-ts-mode--imenu-1 (node)
-  "Helper for `go-ts-mode--imenu'.
-Find string representation for NODE and set marker, then recurse
-the subtrees."
-  (let* ((ts-node (car node))
-         (children (cdr node))
-         (subtrees (mapcan #'go-ts-mode--imenu-1
-                           children))
-         (name (when ts-node
-                 (treesit-node-text
-                  (pcase (treesit-node-type ts-node)
-                    ("function_declaration"
-                     (treesit-node-child-by-field-name ts-node "name"))
-                    ("type_spec"
-                     (treesit-node-child-by-field-name ts-node "name"))))))
-         (marker (when ts-node
-                   (set-marker (make-marker)
-                               (treesit-node-start ts-node)))))
-    (cond
-     ((or (null ts-node) (null name)) subtrees)
-     (subtrees
-      `((,name ,(cons name marker) ,@subtrees)))
-     (t
-      `((,name . ,marker))))))
-
 ;;;###autoload
 (add-to-list 'auto-mode-alist '("\\.go\\'" . go-ts-mode))

@@ -228,9 +190,21 @@ go-ts-mode
     (setq-local comment-end "")
     (setq-local comment-start-skip (rx "//" (* (syntax whitespace))))

+    ;; Navigation.
+    (setq-local treesit-defun-type-regexp
+                (regexp-opt '("method_declaration"
+                              "function_declaration"
+                              "type_declaration")))
+    (setq-local treesit-defun-name-function #'go-ts-mode--defun-name)
+
     ;; Imenu.
-    (setq-local imenu-create-index-function #'go-ts-mode--imenu)
-    (setq-local which-func-functions nil)
+    (setq-local treesit-simple-imenu-settings
+                `(("Function" "\\`function_declaration\\'" nil nil)
+                  ("Method" "\\`method_declaration\\'" nil nil)
+                  ("Struct" "\\`type_declaration\\'"
go-ts-mode--struct-node-p nil)
+                  ("Interface" "\\`type_declaration\\'"
go-ts-mode--interface-node-p nil)
+                  ("Type" "\\`type_declaration\\'"
go-ts-mode--other-type-node-p nil)
+                  ("Alias" "\\`type_declaration\\'"
go-ts-mode--alias-node-p nil)))

     ;; Indent.
     (setq-local indent-tabs-mode t
@@ -247,6 +221,53 @@ go-ts-mode

     (treesit-major-mode-setup)))

+(defun go-ts-mode--defun-name (node)
+  "Return the defun name of NODE.
+Return nil if there is no name or if NODE is not a defun node."
+  (pcase (treesit-node-type node)
+    ("function_declaration"
+     (treesit-node-text
+      (treesit-node-child-by-field-name
+       node "name")
+      t))
+    ("method_declaration"
+     (let* ((receiver-node (treesit-node-child-by-field-name node "receiver"))
+            (type-node (treesit-search-subtree receiver-node
"type_identifier"))
+            (name-node (treesit-node-child-by-field-name node "name")))
+       (concat
+        "(" (treesit-node-text type-node) ")."
+        (treesit-node-text name-node))))
+    ("type_declaration"
+     (treesit-node-text
+      (treesit-node-child-by-field-name
+       (treesit-node-child node 0 t) "name")
+      t))))
+
+(defun go-ts-mode--interface-node-p (node)
+  "Return t if NODE is an interface."
+  (and
+   (string-equal "type_declaration" (treesit-node-type node))
+   (treesit-search-subtree node "interface_type" nil nil 2)))
+
+(defun go-ts-mode--struct-node-p (node)
+  "Return t if NODE is a struct."
+  (and
+   (string-equal "type_declaration" (treesit-node-type node))
+   (treesit-search-subtree node "struct_type" nil nil 2)))
+
+(defun go-ts-mode--alias-node-p (node)
+  "Return t if NODE is a type alias."
+  (and
+   (string-equal "type_declaration" (treesit-node-type node))
+   (treesit-search-subtree node "type_alias" nil nil 1)))
+
+(defun go-ts-mode--other-type-node-p (node)
+  "Return t if NODE is a type, other than interface or struct."
+  (and
+   (string-equal "type_declaration" (treesit-node-type node))
+   (not (go-ts-mode--interface-node-p node))
+   (not (go-ts-mode--struct-node-p node))))
+
 ;; go.mod support.

 (defvar go-mod-ts-mode--syntax-table

On Mon, Jan 2, 2023 at 12:31 AM Randy Taylor <dev@rjt.dev> wrote:
>
> On Sunday, January 1st, 2023 at 12:08, Evgeni Kolev <evgenysw@gmail.com> 
> wrote:
> >
> > As I mentioned in the start of the mail thread - go-ts-mode's Imenu
> > puts Go interfaces and structs in the same "Type" bucket. This can be
> > improved in go-ts-mode.
> >
> > I'm providing a second patch below which splits the interfaces and
> > structs into their own Imenu categories.
> >
> > Please let me know if I should provide the second patch later, in a
> > separate thread, after the first patch is finished. I'm assuming it's
> > simpler to review the patches together. If it's not - I'll provide
> > them in a way to make the review easier, just let me know.
> >
>
> Hi Evgeni!
>
> Thanks for working on this, and apologies for the delay.
>
> The patches look good to me and work well, except for one issue (and a few 
> minor nits) mentioned below. All in one patch is probably best.
>
> - Type definitions are no longer captured. For example:
> type Quack int
> Does not show up anymore.
> - go-ts-mode--struct-node-p's docstring is missing a period at the end.
> - In go-ts-mode--interface-node-p and go-ts-mode--struct-node-p's docstrings, 
> I'm not sure it's worthwhile to mention "Go". Just interface and struct 
> should be fine.
> - Should the commit message mention changes to go-ts-mode (as in the actual 
> define-derived-mode mode part)?