Evaluate build tags as both true and false (#1938)

Gazelle already iterates over all supported os/arch combos to include go
files. This is similar but expanded to custom tags. When adding a new
build tag, we create a copy with the tag set to true and append to the
original. This means that build tag evaluation will grow exponentially.

Fixes #1262

<!-- Thanks for sending a PR! Before submitting:

1. If this is your first PR, please read CONTRIBUTING.md and sign the
CLA
   first. We cannot review code without a signed CLA.
2. Please file an issue *first*. All features and most bug fixes should
have
an associated issue with a design discussed and decided upon. Small bug
   fixes and documentation improvements don't need issues.
3. New features and bug fixes must have tests. Documentation may need to
be updated. If you're unsure what to update, send the PR, and we'll
discuss
   in review.
-->

**What type of PR is this?**

Feature

**What package or component does this PR mostly affect?**

language/go

**What does this PR do? Why is it needed?**

It changes the behavior of `build_tags` to defer evaluation to build
time. Instead of assuming the tag is _always_ true, it treats the tag as
both true and false when evaluating tags for a file.

**Which issues(s) does this PR fix?**

Fixes #1262

**Other notes for review**

Design.rst

@@ -107,8 +107,8 @@ Here are a few examples. See the `full list of directives`_.
 
 * ``# gazelle:prefix`` - sets the Go import path prefix for the current
   directory.
-* ``# gazelle:build_tags`` - sets the list of build tags which Gazelle considers
-  to be true on all platforms.
+* ``# gazelle:build_tags`` - sets the list of build tags which Gazelle will
+  defer to Bazel for evaluation.
 
 There are a few directives which are not applied to the ``Config`` object but
 are interpreted directly in packages where they are relevant.

README.rst

@@ -495,11 +495,12 @@ The following flags are accepted:
 +-------------------------------------------------------------------+----------------------------------------+
 | :flag:`-build_tags tag1,tag2`                                     |                                        |
 +-------------------------------------------------------------------+----------------------------------------+
-| List of Go build tags Gazelle will consider to be true. Gazelle applies                                    |
+| List of Go build tags Gazelle will defer to Bazel for evaluation. Gazelle applies                          |
 | constraints when generating Go rules. It assumes certain tags are true on                                  |
 | certain platforms (for example, ``amd64,linux``). It assumes all Go release                                |
 | tags are true (for example, ``go1.8``). It considers other tags to be false                                |
-| (for example, ``ignore``). This flag overrides that behavior.                                              |
+| (for example, ``ignore``). This flag allows custom tags to be evaluated by                                 |
+| Bazel at build time.                                                                                       |
 |                                                                                                            |
 | Bazel may still filter sources with these tags. Use                                                        |
 | ``bazel build --define gotags=foo,bar`` to set tags at build time.                                         |
@@ -761,11 +762,12 @@ The following directives are recognized:
 +---------------------------------------------------+----------------------------------------+
 | :direc:`# gazelle:build_tags foo,bar`             | none                                   |
 +---------------------------------------------------+----------------------------------------+
-| List of Go build tags Gazelle will consider to be true. Gazelle applies                    |
+| List of Go build tags Gazelle will defer to Bazel for evaluation. Gazelle applies          |
 | constraints when generating Go rules. It assumes certain tags are true on                  |
 | certain platforms (for example, ``amd64,linux``). It assumes all Go release                |
 | tags are true (for example, ``go1.8``). It considers other tags to be false                |
-| (for example, ``ignore``). This flag overrides that behavior.                              |
+| (for example, ``ignore``). This flag allows custom tags to be evaluated by                 |
+| Bazel at build time.                                                                       |
 |                                                                                            |
 | Bazel may still filter sources with these tags. Use                                        |
 | ``bazel build --define gotags=foo,bar`` to set tags at build time.                         |

language/go/config.go

@@ -41,6 +41,16 @@ import (
 
 var minimumRulesGoVersion = version.Version{0, 29, 0}
 
+type tagSet map[string]struct{}
+
+func (ts tagSet) clone() tagSet {
+	c := make(tagSet, len(ts))
+	for k, v := range ts {
+		c[k] = v
+	}
+	return c
+}
+
 // goConfig contains configuration values related to Go rules.
 type goConfig struct {
 	// The name under which the rules_go repository can be referenced from the
@@ -53,7 +63,7 @@ type goConfig struct {
 
 	// genericTags is a set of tags that Gazelle considers to be true. Set with
 	// -build_tags or # gazelle:build_tags. Some tags, like gc, are always on.
-	genericTags map[string]bool
+	genericTags []tagSet
 
 	// prefix is a prefix of an import path, used to generate importpath
 	// attributes. Set with -go_prefix or # gazelle:prefix.
@@ -178,8 +188,10 @@ func newGoConfig() *goConfig {
 		goProtoCompilers: defaultGoProtoCompilers,
 		goGrpcCompilers:  defaultGoGrpcCompilers,
 		goGenerateProto:  true,
+		genericTags: []tagSet{
+			{"gc": struct{}{}},
+		},
 	}
-	gc.preprocessTags()
 	return gc
 }
 
@@ -189,28 +201,18 @@ func getGoConfig(c *config.Config) *goConfig {
 
 func (gc *goConfig) clone() *goConfig {
 	gcCopy := *gc
-	gcCopy.genericTags = make(map[string]bool)
-	for k, v := range gc.genericTags {
-		gcCopy.genericTags[k] = v
+	gcCopy.genericTags = make([]tagSet, 0, len(gc.genericTags))
+	for _, ts := range gc.genericTags {
+		gcCopy.genericTags = append(gcCopy.genericTags, ts.clone())
 	}
 	gcCopy.goProtoCompilers = gc.goProtoCompilers[:len(gc.goProtoCompilers):len(gc.goProtoCompilers)]
 	gcCopy.goGrpcCompilers = gc.goGrpcCompilers[:len(gc.goGrpcCompilers):len(gc.goGrpcCompilers)]
 	gcCopy.submodules = gc.submodules[:len(gc.submodules):len(gc.submodules)]
 	return &gcCopy
 }
 
-// preprocessTags adds some tags which are on by default before they are
-// used to match files.
-func (gc *goConfig) preprocessTags() {
-	if gc.genericTags == nil {
-		gc.genericTags = make(map[string]bool)
-	}
-	gc.genericTags["gc"] = true
-}
-
 // setBuildTags sets genericTags by parsing as a comma separated list. An
 // error will be returned for tags that wouldn't be recognized by "go build".
-// preprocessTags should be called before this.
 func (gc *goConfig) setBuildTags(tags string) error {
 	if tags == "" {
 		return nil
@@ -219,7 +221,13 @@ func (gc *goConfig) setBuildTags(tags string) error {
 		if strings.HasPrefix(t, "!") {
 			return fmt.Errorf("build tags can't be negated: %s", t)
 		}
-		gc.genericTags[t] = true
+		var newSets []tagSet
+		for _, ts := range gc.genericTags {
+			c := ts.clone()
+			c[t] = struct{}{}
+			newSets = append(newSets, c)
+		}
+		gc.genericTags = append(gc.genericTags, newSets...)
 	}
 	return nil
 }
@@ -584,11 +592,6 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.`
 		for _, d := range f.Directives {
 			switch d.Key {
 			case "build_tags":
-				if err := gc.setBuildTags(d.Value); err != nil {
-					log.Print(err)
-					continue
-				}
-				gc.preprocessTags()
 				if err := gc.setBuildTags(d.Value); err != nil {
 					log.Print(err)
 				}

language/go/config_test.go

@@ -59,6 +59,21 @@ func testConfig(t *testing.T, args ...string) (*config.Config, []language.Langua
 	return c, langs, cexts
 }
 
+func newTagSet(tags ...string) tagSet {
+	ts := make(tagSet)
+	for _, t := range tags {
+		ts[t] = struct{}{}
+	}
+	return ts
+}
+
+var expectedBuildTags = []tagSet{
+	newTagSet("gc"),
+	newTagSet("gc", "foo"),
+	newTagSet("gc", "bar"),
+	newTagSet("gc", "foo", "bar"),
+}
+
 func TestCommandLine(t *testing.T) {
 	c, _, _ := testConfig(
 		t,
@@ -68,10 +83,8 @@ func TestCommandLine(t *testing.T) {
 		"-external=vendored",
 		"-repo_root=.")
 	gc := getGoConfig(c)
-	for _, tag := range []string{"foo", "bar", "gc"} {
-		if !gc.genericTags[tag] {
-			t.Errorf("expected tag %q to be set", tag)
-		}
+	if diff := cmp.Diff(expectedBuildTags, gc.genericTags); diff != "" {
+		t.Errorf("(-want, +got): %s", diff)
 	}
 	if gc.prefix != "example.com/repo" {
 		t.Errorf(`got prefix %q; want "example.com/repo"`, gc.prefix)
@@ -101,10 +114,8 @@ func TestDirectives(t *testing.T) {
 		cext.Configure(c, "test", f)
 	}
 	gc := getGoConfig(c)
-	for _, tag := range []string{"foo", "bar", "gc"} {
-		if !gc.genericTags[tag] {
-			t.Errorf("expected tag %q to be set", tag)
-		}
+	if diff := cmp.Diff(expectedBuildTags, gc.genericTags); diff != "" {
+		t.Errorf("(-want, +got): %s", diff)
 	}
 	if gc.prefix != "y" {
 		t.Errorf(`got prefix %q; want "y"`, gc.prefix)
@@ -268,22 +279,6 @@ load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")
 	}
 }
 
-func TestPreprocessTags(t *testing.T) {
-	gc := newGoConfig()
-	expectedTags := []string{"gc"}
-	for _, tag := range expectedTags {
-		if !gc.genericTags[tag] {
-			t.Errorf("tag %q not set", tag)
-		}
-	}
-	unexpectedTags := []string{"x", "cgo", "go1.8", "go1.7"}
-	for _, tag := range unexpectedTags {
-		if gc.genericTags[tag] {
-			t.Errorf("tag %q unexpectedly set", tag)
-		}
-	}
-}
-
 func TestPrefixFallback(t *testing.T) {
 	c, _, cexts := testConfig(t)
 	for _, tc := range []struct {

language/go/fileinfo.go

@@ -569,7 +569,7 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t
 	}
 
 	goConf := getGoConfig(c)
-	checker := func(tag string) bool {
+	checker := func(tag string, ts tagSet) bool {
 		if isIgnoredTag(tag) {
 			return true
 		}
@@ -585,13 +585,19 @@ func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, t
 				return false
 			}
 			return arch == tag
-
 		}
 
-		return goConf.genericTags[tag]
+		_, ok := ts[tag]
+		return ok
 	}
 
-	return tags.eval(checker) && cgoTags.eval(checker)
+	for _, ts := range goConf.genericTags {
+		c := func(tag string) bool { return checker(tag, ts) }
+		if tags.eval(c) && cgoTags.eval(c) {
+			return true
+		}
+	}
+	return false
 }
 
 // rulesGoSupportsOS returns whether the os tag is recognized by the version of

language/go/fileinfo_test.go

@@ -386,7 +386,7 @@ func TestCheckConstraints(t *testing.T) {
 	defer os.RemoveAll(dir)
 	for _, tc := range []struct {
 		desc                        string
-		genericTags                 map[string]bool
+		genericTags                 string
 		os, arch, filename, content string
 		want                        bool
 	}{
@@ -466,12 +466,12 @@ func TestCheckConstraints(t *testing.T) {
 			want:     false,
 		}, {
 			desc:        "tags all satisfied",
-			genericTags: map[string]bool{"a": true, "b": true},
+			genericTags: "a,b",
 			content:     "// +build a,b\n\npackage foo",
 			want:        true,
 		}, {
 			desc:        "tags some satisfied",
-			genericTags: map[string]bool{"a": true},
+			genericTags: "a",
 			content:     "// +build a,b\n\npackage foo",
 			want:        false,
 		}, {
@@ -480,36 +480,36 @@ func TestCheckConstraints(t *testing.T) {
 			want:    true,
 		}, {
 			desc:        "tag satisfied negated",
-			genericTags: map[string]bool{"a": true},
+			genericTags: "a",
 			content:     "// +build !a\n\npackage foo",
-			want:        false,
+			want:        true,
 		}, {
 			desc:    "tag double negative",
 			content: "// +build !!a\n\npackage foo",
 			want:    false,
 		}, {
 			desc:        "tag group and satisfied",
-			genericTags: map[string]bool{"foo": true, "bar": true},
+			genericTags: "foo,bar",
 			content:     "// +build foo,bar\n\npackage foo",
 			want:        true,
 		}, {
 			desc:        "tag group and unsatisfied",
-			genericTags: map[string]bool{"foo": true},
+			genericTags: "foo",
 			content:     "// +build foo,bar\n\npackage foo",
 			want:        false,
 		}, {
 			desc:        "tag line or satisfied",
-			genericTags: map[string]bool{"foo": true},
+			genericTags: "foo",
 			content:     "// +build foo bar\n\npackage foo",
 			want:        true,
 		}, {
 			desc:        "tag line or unsatisfied",
-			genericTags: map[string]bool{"foo": true},
+			genericTags: "foo",
 			content:     "// +build !foo bar\n\npackage foo",
-			want:        false,
+			want:        true,
 		}, {
 			desc:        "tag lines and satisfied",
-			genericTags: map[string]bool{"foo": true, "bar": true},
+			genericTags: "foo,bar",
 			content: `
 // +build foo
 // +build bar
@@ -518,7 +518,7 @@ package foo`,
 			want: true,
 		}, {
 			desc:        "tag lines and unsatisfied",
-			genericTags: map[string]bool{"foo": true},
+			genericTags: "foo",
 			content: `
 // +build foo
 // +build bar
@@ -528,7 +528,7 @@ package foo`,
 		}, {
 			desc:        "cgo tags satisfied",
 			os:          "linux",
-			genericTags: map[string]bool{"foo": true},
+			genericTags: "foo",
 			content: `
 // +build foo
 
@@ -581,9 +581,8 @@ import "C"
 		t.Run(tc.desc, func(t *testing.T) {
 			c, _, _ := testConfig(t)
 			gc := getGoConfig(c)
-			gc.genericTags = tc.genericTags
-			if gc.genericTags == nil {
-				gc.genericTags = map[string]bool{"gc": true}
+			if err := gc.setBuildTags(tc.genericTags); err != nil {
+				t.Errorf("error setting build tags %q", tc.genericTags)
 			}
 			filename := tc.filename
 			if filename == "" {