x/tools/gopls: improve cgo support

[stamblerre]

Original issue: #31561.
Currently, gopls does not crash with cgo, but language features are still missing for any symbols imported from "C".

[myitcv]

Just to confirm, any file that imports "C" is affected by this, correct?

package p

/*
#include <stdio.h>
#include <stdlib.h>

void myprint(char* s) {
	printf("%s\n", s);
}
*/
import "C"

import "fmt"

import "unsafe"

func Example() {
	fmt.Println()
	cs := C.CString("Hello from stdio\n")
	C.myprint(cs)
	C.free(unsafe.Pointer(cs))
}

For example, an attempt to go-to-def on fmt.Println results in:

GetAST: unable to check package for file:///home/myitcv/gostuff/src/github.com/myitcv/playground/p/p.go: loadParseTypecheck: no metadata found for /home/myitcv/gostuff/src/github.com/myitcv/playground/p/p.go

At least based on some rough tests, I can't get anything to work when a file imports "C".

[myitcv]

Actually, this appears to be limited to files that import "C". I've updated my previous comment to reflect this.

[myitcv]

Here is a test that demonstrates the issue within govim:

https://github.com/myitcv/govim/blob/cmd_govim_cgo_example/cmd/govim/testdata/cgo_support.txt

This attempts to go-to-def from SameFileN in each of the two files. In the file that does import "C" all is good; in the one that does we get:

GetAST: unable to check package for file://$WORK/p_importc.go: loadParseTypecheck: no metadata found for $WORK/p_importc.go

[ctxkenb]

I think I've found the cause of this. go list -json returns cached files in the list of CompileGoFiles, in this form (on Mac):

	"CompiledGoFiles": [
		"a.go",
		"b.go",
		"/Users/me/Library/Caches/go-build/d0/d0806a53076964ea7f690a3852e2ada78bc4bf692c37febfacbb6f22bb0c74e5-d",
		"/Users/me/Library/Caches/go-build/f0/f02a2cefdc3e9521cf5705b4c5d454f0b606b37fc6f3186eb6f05235e53979a3-d",
	]

These cache files appear to be the cgo source. Because the name doesn't match the file name of the original source, gopls is unable to attribute the metadata.

I'm not sure what the 'right' solution is, but i found a quick hack that enumerates GoFiles and CgoFiles instead of using CompiledGoFiles: (golang.org/x/tools)

diff --git a/go/packages/packages.go b/go/packages/packages.go
index 4ca2b117..75b91f9d 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -249,6 +249,8 @@ type Package struct {
        // GoFiles lists the absolute file paths of the package's Go source files.
        GoFiles []string
 
+       CgoFiles []string
+
        // CompiledGoFiles lists the absolute file paths of the package's source
        // files that were presented to the compiler.
        // This may differ from GoFiles if files are processed before compilation.
@@ -333,6 +335,7 @@ type flatPackage struct {
        PkgPath         string            `json:",omitempty"`
        Errors          []Error           `json:",omitempty"`
        GoFiles         []string          `json:",omitempty"`
+       CgoFiles        []string          `json:",omitempty"`
        CompiledGoFiles []string          `json:",omitempty"`
        OtherFiles      []string          `json:",omitempty"`
        ExportFile      string            `json:",omitempty"`
@@ -381,6 +384,7 @@ func (p *Package) UnmarshalJSON(b []byte) error {
                PkgPath:         flat.PkgPath,
                Errors:          flat.Errors,
                GoFiles:         flat.GoFiles,
+               CgoFiles:        flat.CgoFiles,
                CompiledGoFiles: flat.CompiledGoFiles,
                OtherFiles:      flat.OtherFiles,
                ExportFile:      flat.ExportFile,
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index 6b1320bc..ba7abd6e 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -241,7 +241,7 @@ func (v *view) link(ctx context.Context, g *importGraph) error {
                typesSizes: g.pkg.TypesSizes,
                errors:     g.pkg.Errors,
        }
-       for _, filename := range g.pkg.CompiledGoFiles {
+       for _, filename := range append(g.pkg.GoFiles, g.pkg.CgoFiles...) {
                m.files = append(m.files, span.FileURI(filename))
 
                // Call the unlocked version of getFile since we are holding the view's mutex.

[muirdm]

The generated files that correspond to real files have a //line ... directive mentioning the real file right? Maybe we can parse those to map them back to the real file, then proxy requests/responses between the real file and generated file, adjusting the file offsets as necessary?

[ctxkenb]

load.go got changed in master, here's an updated diff for the hacky fix...

diff --git a/go/packages/packages.go b/go/packages/packages.go
index b29c9136..f025c2f5 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -248,6 +248,8 @@ type Package struct {
        // GoFiles lists the absolute file paths of the package's Go source files.
        GoFiles []string
 
+       CgoFiles []string
+
        // CompiledGoFiles lists the absolute file paths of the package's source
        // files that were presented to the compiler.
        // This may differ from GoFiles if files are processed before compilation.
@@ -332,6 +334,7 @@ type flatPackage struct {
        PkgPath         string            `json:",omitempty"`
        Errors          []Error           `json:",omitempty"`
        GoFiles         []string          `json:",omitempty"`
+       CgoFiles        []string          `json:",omitempty"`
        CompiledGoFiles []string          `json:",omitempty"`
        OtherFiles      []string          `json:",omitempty"`
        ExportFile      string            `json:",omitempty"`
@@ -380,6 +383,7 @@ func (p *Package) UnmarshalJSON(b []byte) error {
                PkgPath:         flat.PkgPath,
                Errors:          flat.Errors,
                GoFiles:         flat.GoFiles,
+               CgoFiles:        flat.CgoFiles,
                CompiledGoFiles: flat.CompiledGoFiles,
                OtherFiles:      flat.OtherFiles,
                ExportFile:      flat.ExportFile,
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index cdbe9703..fe42c4b6 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -152,7 +152,7 @@ func (s *snapshot) updateMetadata(ctx context.Context, uri span.URI, pkgs []*pac
 
        var results []*metadata
        for _, pkg := range pkgs {
-               log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", pkg.CompiledGoFiles))
+               log.Print(ctx, "go/packages.Load", tag.Of("package", pkg.PkgPath), tag.Of("files", append(pkg.GoFiles, pkg.CgoFiles...)))
 
                // Set the metadata for this package.
                if err := s.updateImports(ctx, packagePath(pkg.PkgPath), pkg, cfg); err != nil {
@@ -183,7 +183,7 @@ func (s *snapshot) updateImports(ctx context.Context, pkgPath packagePath, pkg *
                errors:     pkg.Errors,
                config:     cfg,
        }
-       for _, filename := range pkg.CompiledGoFiles {
+       for _, filename := range append(pkg.GoFiles, pkg.CgoFiles...) {
                uri := span.FileURI(filename)
                m.files = append(m.files, uri)