Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/cmd/guru: definition doesn't look up the function references in the same package if modules are enabled #31720

Closed
jdevelop opened this issue Apr 28, 2019 · 5 comments
Closed

x/tools/cmd/guru: definition doesn't look up the function references in the same package if modules are enabled #31720

jdevelop opened this issue Apr 28, 2019 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@jdevelop
Copy link

What version of Go are you using (go version)?

go version go1.12.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/user/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build666000922=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Given the files a.go and b.go defined in the same package, guru definition isn't able to find the method that is defined in the same package but in different file if go.mod is present and the project is outside of GOPATH.

a.go
package a

func TestA() {
	// do nothing
}
b.go
package a

func TestAA() {
	TestA()
}

Then with if the files are in the $GOPATH/src/test/guru_mod_files/a/ the command

guru definition /home/user/go/src/test/guru_mod_files/a/b.go:#30

returns

/home/user/go/src/test/guru_mod_files/a/a.go:3:6: defined here as func TestA()

however if the package is moved out of $GOPATH, then it doesn't work as expected:

guru definition /home/user/tmp/guru_mod_files/a/b.go:#30
guru: no object for identifier

What did you expect to see?

Proper function definition from the other file in the same package.

What did you see instead?

guru: no object for identifier
@gopherbot gopherbot added this to the Unreleased milestone Apr 28, 2019
@jdevelop
Copy link
Author

Some quick fix to consider, at the first glance it works ( tested on several modules I tried so far ). And looks pretty logical, given that CreateFromFilenames is meant to take several files.

diff --git a/cmd/guru/guru.go b/cmd/guru/guru.go
index 8dea3b53..f4c2fb26 100644
--- a/cmd/guru/guru.go
+++ b/cmd/guru/guru.go
@@ -20,6 +20,8 @@ import (
 	"go/types"
 	"io"
 	"log"
+	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -164,7 +166,24 @@ func importQueryPackage(pos string, conf *loader.Config) (string, error) {
 		// Can't find GOPATH dir.
 		// Treat the query file as its own package.
 		importPath = "command-line-arguments"
-		conf.CreateFromFilenames(importPath, filename)
+		dirPath := path.Dir(filename)
+		dir, err := os.Open(dirPath)
+		if err != nil {
+			return "", err
+		}
+		defer dir.Close()
+		fis, err := dir.Readdir(-1)
+		if err != nil {
+			return "", err
+		}
+		files := make([]string, 0, 1)
+		for _, fileinfo := range fis {
+			if fileinfo.IsDir() {
+				continue
+			}
+			files = append(files, path.Join(dirPath, fileinfo.Name()))
+		}
+		conf.CreateFromFilenames(importPath, files...)
 	} else {
 		// Check that it's possible to load the queried package.
 		// (e.g. guru tests contain different 'package' decls in same dir.)

@gopherbot
Copy link

Change https://golang.org/cl/174021 mentions this issue: x/tools/cmd/guru definition: fix look up of references in the same package if modules are enabled

@bcmills
Copy link
Contributor

bcmills commented May 22, 2019

CC @ianthehat

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 22, 2019
@ianthehat
Copy link

As mentioned on the linked cl, we have chosen not to convert guru to work with modules because we feel the effort required is too large and are focusing on gopls as a better solution instead.
If anyone wants to contribute a true port of guru to modules please to re-open this bug and send us the changes.

@jostyee jostyee mentioned this issue Aug 19, 2019
Open
@bcmills bcmills mentioned this issue Aug 22, 2019
Closed
@bogdando
Copy link

bogdando commented May 14, 2020 •
edited

not to convert guru to work with modules

https://github.com/bogdando/gogetguru a script that workarounds that issue. This time it works much better, I hope.

@golang golang locked and limited conversation to collaborators May 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@jdevelop @ianthehat @bogdando @bcmills @gopherbot