gopackagesdriver: fix interface to work with golangci-lint (bazelbuil…

…d#3523)

Unlike gopls which uses `file=` queries exclusively, golangci-lint uses
the more generic `go list` query syntax. This change allow for file
paths not prefixed with `file=` to be queried correctly.

Additionally, some bugs have been fixed up:
- Packages with different IDs and identical import paths work now.
- stdlib now includes CGO processed files.
- WKT for protobufs are no longer skipped when the proto wrapper is
  used.`

go/private/actions/stdlib.bzl

@@ -59,24 +59,44 @@ def _should_use_sdk_stdlib(go):
             not go.mode.gc_goopts and
             go.mode.link == LINKMODE_NORMAL)
 
-def _build_stdlib_list_json(go):
+def _build_stdlib_list_json(go, env):
     out = go.declare_file(go, "stdlib.pkg.json")
     args = go.builder_args(go, "stdliblist")
     args.add("-sdk", go.sdk.root_file.dirname)
     args.add("-out", out)
+
     go.actions.run(
         inputs = go.sdk_files,
         outputs = [out],
         mnemonic = "GoStdlibList",
         executable = go.toolchain._builder,
         arguments = [args],
-        env = go.env,
+        env = env,
     )
     return out
 
+def _cgo_env(go):
+    env = go.env
+
+    # NOTE(#2545): avoid unnecessary dynamic link
+    # go std library doesn't use C++, so should not have -lstdc++
+    ldflags = [
+        option
+        for option in extldflags_from_cc_toolchain(go)
+        if option not in ("-lstdc++", "-lc++")
+    ]
+    env.update({
+        "CGO_ENABLED": "1",
+        "CC": go.cgo_tools.c_compiler_path,
+        "CGO_CFLAGS": " ".join(go.cgo_tools.c_compile_options),
+        "CGO_LDFLAGS": " ".join(ldflags),
+    })
+
+    return env
+
 def _sdk_stdlib(go):
     return GoStdLib(
-        _list_json = _build_stdlib_list_json(go),
+        _list_json = _build_stdlib_list_json(go, _cgo_env(go)),
         libs = go.sdk.libs,
         root_file = go.sdk.root_file,
     )
@@ -92,24 +112,15 @@ def _build_stdlib(go):
     if not go.mode.pure:
         args.add("-package", "runtime/cgo")
     args.add_all(link_mode_args(go.mode))
+
     env = go.env
     if go.mode.pure:
         env.update({"CGO_ENABLED": "0"})
     else:
-        # NOTE(#2545): avoid unnecessary dynamic link
-        # go std library doesn't use C++, so should not have -lstdc++
-        ldflags = [
-            option
-            for option in extldflags_from_cc_toolchain(go)
-            if option not in ("-lstdc++", "-lc++")
-        ]
-        env.update({
-            "CGO_ENABLED": "1",
-            "CC": go.cgo_tools.c_compiler_path,
-            "CGO_CFLAGS": " ".join(go.cgo_tools.c_compile_options),
-            "CGO_LDFLAGS": " ".join(ldflags),
-        })
+        env = _cgo_env(go)
+
     args.add("-gcflags", quote_opts(go.mode.gc_goopts))
+
     inputs = (go.sdk.srcs +
               go.sdk.headers +
               go.sdk.tools +
@@ -125,7 +136,7 @@ def _build_stdlib(go):
         env = env,
     )
     return GoStdLib(
-        _list_json = _build_stdlib_list_json(go),
+        _list_json = _build_stdlib_list_json(go, env),
         libs = [pkg],
         root_file = pkg,
     )

go/tools/builders/stdliblist.go

@@ -116,20 +116,45 @@ func outputBasePath(cloneBase, p string) string {
 	return filepath.Join("__BAZEL_OUTPUT_BASE__", dir)
 }
 
-//  absoluteSourcesPaths replace cloneBase of the absolution
-//  paths with the label for all source files in a package
+// absoluteSourcesPaths replace cloneBase of the absolution
+// paths with the label for all source files in a package
 func absoluteSourcesPaths(cloneBase, pkgDir string, srcs []string) []string {
 	ret := make([]string, 0, len(srcs))
 	pkgDir = outputBasePath(cloneBase, pkgDir)
 	for _, src := range srcs {
-		ret = append(ret, filepath.Join(pkgDir, src))
+		absPath := src
+
+		// Generated files will already have an absolute path. These come from
+		// the compiler's cache.
+		if !filepath.IsAbs(src) {
+			absPath = filepath.Join(pkgDir, src)
+		}
+
+		ret = append(ret, absPath)
 	}
 	return ret
 }
 
+// filterGoFiles keeps only files either ending in .go or those without an
+// extension (which are from the cache). This is a work around for
+// https://golang.org/issue/28749: cmd/go puts assembly, C, and C++ files in
+// CompiledGoFiles.
+func filterGoFiles(srcs []string) []string {
+	ret := make([]string, 0, len(srcs))
+	for _, f := range srcs {
+		if ext := filepath.Ext(f); ext != ".go" && ext != "" {
+			continue
+		}
+
+		ret = append(ret, f)
+	}
+
+	return ret
+}
+
 func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage {
-	// Don't use generated files from the stdlib
 	goFiles := absoluteSourcesPaths(cloneBase, pkg.Dir, pkg.GoFiles)
+	compiledGoFiles := absoluteSourcesPaths(cloneBase, pkg.Dir, pkg.CompiledGoFiles)
 
 	newPkg := &flatPackage{
 		ID:              stdlibPackageID(pkg.ImportPath),
@@ -139,13 +164,31 @@ func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage {
 		Imports:         map[string]string{},
 		Standard:        pkg.Standard,
 		GoFiles:         goFiles,
-		CompiledGoFiles: goFiles,
+		CompiledGoFiles: filterGoFiles(compiledGoFiles),
+	}
+
+	// imports
+	//
+	// Imports contains the IDs of all imported packages.
+	// ImportsMap records (path, ID) only where they differ.
+	ids := make(map[string]bool)
+	for _, id := range pkg.Imports {
+		ids[id] = true
 	}
-	for _, imp := range pkg.Imports {
-		newPkg.Imports[imp] = stdlibPackageID(imp)
+
+	for path, id := range pkg.ImportMap {
+		newPkg.Imports[path] = stdlibPackageID(id)
+		delete(ids, id)
 	}
-	// We don't support CGo for now
-	delete(newPkg.Imports, "C")
+
+	for id := range ids {
+		if id == "C" {
+			continue
+		}
+
+		newPkg.Imports[id] = stdlibPackageID(id)
+	}
+
 	return newPkg
 }
 
@@ -200,12 +243,18 @@ func stdliblist(args []string) error {
 	}
 	os.Setenv("PATH", strings.Join(absPaths, string(os.PathListSeparator)))
 	os.Setenv("GOROOT", newGoRoot)
+
+	cgoEnabled := os.Getenv("CGO_ENABLED") == "1"
 	// Make sure we have an absolute path to the C compiler.
 	// TODO(#1357): also take absolute paths of includes and other paths in flags.
-	os.Setenv("CC", quotePathIfNeeded(abs(os.Getenv("CC"))))
+	ccEnv, ok := os.LookupEnv("CC")
+	if cgoEnabled && !ok {
+		return fmt.Errorf("CC must be set")
+	}
+	os.Setenv("CC", quotePathIfNeeded(abs(ccEnv)))
 
+	// We want to keep the cache around so that the processed files can be used by other tools.
 	cachePath := abs(*out + ".gocache")
-	defer os.RemoveAll(cachePath)
 	os.Setenv("GOCACHE", cachePath)
 	os.Setenv("GOMODCACHE", cachePath)
 	os.Setenv("GOPATH", cachePath)
@@ -214,6 +263,11 @@ func stdliblist(args []string) error {
 	if len(build.Default.BuildTags) > 0 {
 		listArgs = append(listArgs, "-tags", strings.Join(build.Default.BuildTags, ","))
 	}
+
+	if cgoEnabled {
+		listArgs = append(listArgs, "-compiled=true")
+	}
+
 	listArgs = append(listArgs, "-json", "builtin", "std", "runtime/cgo")
 
 	jsonFile, err := os.Create(*out)
@@ -226,6 +280,7 @@ func stdliblist(args []string) error {
 	if err := goenv.runCommandToFile(jsonData, os.Stderr, listArgs); err != nil {
 		return err
 	}
+
 	encoder := json.NewEncoder(jsonFile)
 	decoder := json.NewDecoder(jsonData)
 	for decoder.More() {

go/tools/gopackagesdriver/aspect.bzl

@@ -32,6 +32,7 @@ DEPS_ATTRS = [
 PROTO_COMPILER_ATTRS = [
     "compiler",
     "compilers",
+    "library",
 ]
 
 def bazel_supports_canonical_label_literals():
@@ -68,6 +69,10 @@ def _go_archive_to_pkg(archive):
             for src in archive.data.orig_srcs
             if not src.path.endswith(".go")
         ],
+        Imports = {
+            pkg.data.importpath: str(pkg.data.label)
+            for pkg in archive.direct
+        },
     )
 
 def make_pkg_json(ctx, name, pkg_info):
@@ -84,7 +89,13 @@ def _go_pkg_info_aspect_impl(target, ctx):
     transitive_compiled_go_files = []
 
     for attr in DEPS_ATTRS + PROTO_COMPILER_ATTRS:
-        for dep in getattr(ctx.rule.attr, attr, []) or []:
+        deps = getattr(ctx.rule.attr, attr, []) or []
+
+        # Some attrs are not iterable, ensure that deps is always iterable.
+        if type(deps) != "list":
+            deps = [deps]
+
+        for dep in deps:
             if GoPkgInfo in dep:
                 pkg_info = dep[GoPkgInfo]
                 transitive_json_files.append(pkg_info.pkg_json_files)

go/tools/gopackagesdriver/bazel.go

@@ -122,6 +122,7 @@ func (b *Bazel) Build(ctx context.Context, args ...string) ([]string, error) {
 		if err := decoder.Decode(&namedSet); err != nil {
 			return nil, fmt.Errorf("unable to decode %s: %w", jsonFile.Name(), err)
 		}
+
 		if namedSet.NamedSetOfFiles != nil {
 			for _, f := range namedSet.NamedSetOfFiles.Files {
 				fileUrl, err := url.Parse(f.URI)

go/tools/gopackagesdriver/bazel_json_builder.go

@@ -21,14 +21,15 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
 )
 
 type BazelJSONBuilder struct {
-	bazel    *Bazel
-	requests []string
+	bazel        *Bazel
+	includeTests bool
 }
 
 var RulesGoStdlibLabel = rulesGoRepositoryName + "//:stdlib"
@@ -42,6 +43,8 @@ func (b *BazelJSONBuilder) fileQuery(filename string) string {
 
 	if filepath.IsAbs(filename) {
 		label, _ = filepath.Rel(b.bazel.WorkspaceRoot(), filename)
+	} else if strings.HasPrefix(filename, "./") {
+		label = strings.TrimPrefix(filename, "./")
 	}
 
 	if matches := externalRe.FindStringSubmatch(filename); len(matches) == 5 {
@@ -79,31 +82,57 @@ func (b *BazelJSONBuilder) fileQuery(filename string) string {
 	return fmt.Sprintf(`kind("%s", same_pkg_direct_rdeps("%s"))`, strings.Join(kinds, "|"), label)
 }
 
+func (b *BazelJSONBuilder) getKind() string {
+	kinds := []string{"go_library"}
+	if b.includeTests {
+		kinds = append(kinds, "go_test")
+	}
+
+	return strings.Join(kinds, "|")
+}
+
+func (b *BazelJSONBuilder) localQuery(request string) string {
+	request = path.Clean(request)
+	if filepath.IsAbs(request) {
+		if relPath, err := filepath.Rel(workspaceRoot, request); err == nil {
+			request = relPath
+		}
+	}
+
+	if !strings.HasSuffix(request, "...") {
+		request = fmt.Sprintf("%s:*", request)
+	}
+
+	return fmt.Sprintf(`kind("%s", %s)`, b.getKind(), request)
+}
+
 func (b *BazelJSONBuilder) packageQuery(importPath string) string {
 	if strings.HasSuffix(importPath, "/...") {
 		importPath = fmt.Sprintf(`^%s(/.+)?$`, strings.TrimSuffix(importPath, "/..."))
 	}
-	return fmt.Sprintf(`kind("go_library", attr(importpath, "%s", deps(%s)))`, importPath, bazelQueryScope)
+
+	return fmt.Sprintf(
+		`kind("%s", attr(importpath, "%s", deps(%s)))`,
+		b.getKind(),
+		importPath,
+		bazelQueryScope)
 }
 
 func (b *BazelJSONBuilder) queryFromRequests(requests ...string) string {
 	ret := make([]string, 0, len(requests))
 	for _, request := range requests {
 		result := ""
-		if request == "." || request == "./..." {
-			if bazelQueryScope != "" {
-				result = fmt.Sprintf(`kind("go_library", %s)`, bazelQueryScope)
-			} else {
-				result = fmt.Sprintf(RulesGoStdlibLabel)
-			}
-		} else if request == "builtin" || request == "std" {
-			result = fmt.Sprintf(RulesGoStdlibLabel)
-		} else if strings.HasPrefix(request, "file=") {
+		if strings.HasSuffix(request, ".go") {
 			f := strings.TrimPrefix(request, "file=")
 			result = b.fileQuery(f)
+		} else if isLocalPattern(request) {
+			result = b.localQuery(request)
+		} else if request == "builtin" || request == "std" {
+			result = fmt.Sprintf(RulesGoStdlibLabel)
 		} else if bazelQueryScope != "" {
 			result = b.packageQuery(request)
 		}
+
 		if result != "" {
 			ret = append(ret, result)
 		}
@@ -114,10 +143,10 @@ func (b *BazelJSONBuilder) queryFromRequests(requests ...string) string {
 	return strings.Join(ret, " union ")
 }
 
-func NewBazelJSONBuilder(bazel *Bazel, requests ...string) (*BazelJSONBuilder, error) {
+func NewBazelJSONBuilder(bazel *Bazel, includeTests bool) (*BazelJSONBuilder, error) {
 	return &BazelJSONBuilder{
-		bazel:    bazel,
-		requests: requests,
+		bazel:        bazel,
+		includeTests: includeTests,
 	}, nil
 }
 
@@ -144,11 +173,12 @@ func (b *BazelJSONBuilder) query(ctx context.Context, query string) ([]string, e
 	if err != nil {
 		return nil, fmt.Errorf("unable to query: %w", err)
 	}
+
 	return labels, nil
 }
 
-func (b *BazelJSONBuilder) Build(ctx context.Context, mode LoadMode) ([]string, error) {
-	labels, err := b.query(ctx, b.queryFromRequests(b.requests...))
+func (b *BazelJSONBuilder) Labels(ctx context.Context, requests []string) ([]string, error) {
+	labels, err := b.query(ctx, b.queryFromRequests(requests...))
 	if err != nil {
 		return nil, fmt.Errorf("query failed: %w", err)
 	}
@@ -157,6 +187,10 @@ func (b *BazelJSONBuilder) Build(ctx context.Context, mode LoadMode) ([]string,
 		return nil, fmt.Errorf("found no labels matching the requests")
 	}
 
+	return labels, nil
+}
+
+func (b *BazelJSONBuilder) Build(ctx context.Context, labels []string, mode LoadMode) ([]string, error) {
 	aspects := append(additionalAspects, goDefaultAspect)
 
 	buildArgs := concatStringsArrays([]string{
@@ -179,7 +213,7 @@ func (b *BazelJSONBuilder) Build(ctx context.Context, mode LoadMode) ([]string,
 		writer := bufio.NewWriter(targetsFile)
 		defer writer.Flush()
 		for _, l := range labels {
-			writer.WriteString(l+"\n")
+			writer.WriteString(l + "\n")
 		}
 		if err := writer.Flush(); err != nil {
 			return nil, fmt.Errorf("unable to flush data to target pattern file: %w", err)