Gopls: Bad default workspace choice when go code is not at repository root

[zot]

Describe the bug In unregistered projects where the go code is in a repository subfolder (like src or go), lsp-mode offers the repository folder as the default choice (for the i choice).

If you choose this, gopls gets confused and produces errors like this:

2020/07/27 16:02:32 go/packages.Load: go [-e -json -compiled=true -test=true -export=false -deps=true -find=false -- ./]: exit status 1: go: cannot find main module, but found .git/config in /zen/aux/work/p2pmud2-projects/ipfs-p2p-websocket
    to create a module there, run:
    go mod init

    snapshot=1
    directory=/zen/aux/work/p2pmud2-projects/ipfs-p2p-websocket
    query=[./]
    packages=0

To Reproduce

1. Create a directory and run git init
2. Create a subdirectory called src
3. Cd into it and run go mod init
4. In EMACS, create a new go file in the src directory
5. Lsp-mode will ask for the project folder, with the (unworkable) repository folder as the default choice

Expected behavior lsp-mode should offer the nearest ancestor containing a go.mod file, if it can find one.

If it cannot find one, it should recommend the nearest other project-like directory but with a warning that it might not work.

Which Language Server did you use gopls

OS Linux

[yyoncho]

This has been discussed multiple times in the past and it is considered as out of scope. The default is only that - default, that user should inspect and if it is not what they mean they should press I and select the proper root. Both projectile/project.el has huge frameworks with different strategies for finding project root but in the end they don't provide 100% guarantee that they will pick the right project root. It doesn't make sense for lsp-mode to duplicate that effort and expect different results.

[zot]

Projectile cannot solve this problem for multi-language projects because lsp actually requires a directory within the project as its root and not the actual root of the project.

Multi-language projects are quite common today (probably the norm) so if lsp continues only to defer to projectile and not to check its result, it will offer bad default more and more commonly. Lsp for each language should use its own default and only use projectile's when projectile's root is a descendant of lsp's computed default.

My point here is that it is actually projectile which is out of scope and consulting it for authority on a language's root directory will increasingly be the wrong choice over time.

[yyoncho]

@zot

Projectile cannot solve this problem for multi-language projects because

This is not true - projectile can be configured to work in many different ways - check projectile-project-root-files-functions. So your language-specific strategy must be implemented in the project that is specifically designed for managing projects which in emacs world is projectile. There is nothing in the language server that can help in validating the root, furthermore, the reference lsp client implementation(VScode) even requires the users to explicitly select project roots upfront.

Again, in root detection, no matter if it is only for the default or not, there is nothing lsp specific so it doesn't make sense to put it in lsp-mode project.

[zot]

I really think this should be part of lsp-go.el but, looking through lsp-mode.el, it looks like there's not currently a good way for languages to contribute strategies for finding their project roots.

So I added this to my settings too accomplish it. Hopefully other people who want this behavior will benefit from my code:

(use-package lsp-mode :ensure t
  :config (progn
            ;; use flycheck, not flymake
            (setq lsp-prefer-flymake nil)
            (advice-add 'lsp--suggest-project-root
                        :around (lambda (oldfun &rest r)
                                  (custom/lsp-default-dir (apply oldfun r))))))
(defun custom/lsp-default-dir (suggestion)
  (if (eq major-mode 'go-mode) (custom/find-go-dir default-directory suggestion) suggestion))
(defun custom/find-go-dir (dir limit) 
  (if (or (string-empty-p dir) (equal dir limit)) limit
    (if (member "go.mod" (directory-files dir)) dir
      (custom/find-go-dir (file-name-directory dir) limit))))

[yyoncho]

it looks like there's not currently a good way for languages to contribute strategies for finding their project roots.

Nope...

(add-to-list 'projectile-project-root-files-functions 'custom/lsp-default-dir)

... and it works with no need advice private functions. You could even use projectile-root-top-down-recurring.

[zot]

Maybe I'm misunderstanding your reply but wouldn't that make projectile use the directory containing go.mod as the (wrong) project root?

[yyoncho]

Depends on what your definition for project is. See also https://github.com/bbatsov/projectile/issues/308

[zot]

Thanks for your help!

I added my function to projectile-project-root-files-functions and it works (after fixing a bug in my code :) ). I agree that it's way better to add a hook instead of advice. Here's my less buggy code, in case someone stumbles on this issue:

(defun custom/find-go-dir (dir)
  (if (equal dir "/") nil
    (if (member "go.mod" (directory-files dir)) dir
      (custom/find-go-dir (file-name-directory (string-trim-right dir "/"))))))
;...
(use-package projectile
  :config (progn
            ;;....
            (add-to-list 'projectile-project-root-files-functions 'custom/find-go-dir)
            ))

By the way, I also tried adding "go.mod" to projectile-project-root-files and that didn't work for me.

[yyoncho]

@zot I think that you should reorder the functions as suggested in bbatsov/projectile#308 or eventually forget that project in projectile to let it run the discovery functions.

[zot]

Clearly there are more projectile subtleties I need to learn but so far, with the testing I've done for this configuration, the current function order, with my custom function first seems to work just the way I want, i.e. I want Projectile to eagerly identify directories containing go.mod as project roots. This does make Projectile commands issued from within a Go buffer use the wrong root for me but at this point I don't really use Projectile for much.

I'd like user commands that operate on the buffer's current project to use the registered root (either Treemacs' or Projectile's if it exists) but I can gradually make my own commands to accomplish that. Within Treemacs, I made C-x C-f use projectile for finding files, passing in Treemacs' project root as the projectile root.