Golang proposal: x/tools/go/analysis/internal/checker: check should accept custom build flags

Proposal Details

checker always load packages with empty BuildFlags. however, analysis code in other build tags are needed

Comment From: ianlancetaylor

CC @alandonovan

Comment From: adonovan

This feature is already supported by the unitchecker analysis driver, as shown by this example based on the printf checker built in to go vet (which is based on unitchecker):

xtools$ cat p/p.go
package p

$ cat p/p_foo.go
//go:build mytag
// +build mytag

package p

import "fmt"

func _() { fmt.Printf("%d", "") }

$ go vet ./p

$ go vet -tags=mytag ./p
p/p_foo.go:8:12: fmt.Printf format %d has arg "" of wrong type string

This works even with the -vettool flag, which specifies an arbitrary unitchecker-based driver. For example:

xtools$ go build -o nilness ./go/analysis/passes/nilness/cmd/nilness/
xtools$ go vet  -vettool=./nilness ./p
...shows nilness diagnostics in p_foo.go...

So you may be able to use that instead. (Bonus: it's more efficient, because it does an "incremental build", just like go build).

You are correct that the {single,multi}checker driver (whose common implementation is internal/checker) does not currently support build tags, even though it declares a non-functional flag named -tags for historical reasons to do with vet compatibility.

We could revive this flag to support the desired behavior with a small patch such as this:

diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go
index ff14ff58f9..366d69db85 100644
--- a/go/analysis/internal/analysisflags/flags.go
+++ b/go/analysis/internal/analysisflags/flags.go
@@ -26,6 +26,7 @@ import (
 var (
        JSON    = false // -json
        Context = -1    // -c=N: if N>0, display offending line plus N lines of context
+       Tags    string
 )

 // Parse creates a flag for each of the analyzer's flags,
@@ -80,7 +81,7 @@ func Parse(analyzers []*analysis.Analyzer, multi bool) []*analysis.Analyzer {
        _ = flag.Bool("source", false, "no effect (deprecated)")
        _ = flag.Bool("v", false, "no effect (deprecated)")
        _ = flag.Bool("all", false, "no effect (deprecated)")
-       _ = flag.String("tags", "", "no effect (deprecated)")
+       flag.StringVar(&Tags, "tags", "", "comma-separated list of build tags")
        for old, new := range vetLegacyFlags {
                newFlag := flag.Lookup(new)
                if newFlag != nil && flag.Lookup(old) == nil {
diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go
index 3c89350089..4b06a2a265 100644
--- a/go/analysis/internal/checker/checker.go
+++ b/go/analysis/internal/checker/checker.go
@@ -175,6 +175,8 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) {
        conf := packages.Config{
                Mode:  mode,
                Tests: IncludeTests,
+               // TODO: assumes no GOPACKAGESDRIVER
+               BuildFlags: []string{"-tags=" + analysisflags.Tags},
        }
        initial, err := packages.Load(&conf, patterns...)
        if err == nil {

But it would need some extra care to address alternative go/packages drivers such as Blaze, Bazel, Pants, Buck, and so on; the packages.Config.BuildFlags field doesn't feel like it was designed with portability in mind.