From 90b1ad2062ee070dea7164c79be26d8d2e29ab39 Mon Sep 17 00:00:00 2001 From: Milan Patel <32933108+theMPatel@users.noreply.github.com> Date: Sun, 8 May 2022 14:56:13 -0400 Subject: [PATCH] Add support for the new //go:build syntax --- language/go/fileinfo.go | 404 ++++++++++++++++++++++---------- language/go/fileinfo_go_test.go | 42 +++- language/go/fileinfo_test.go | 165 ++++++++++--- language/go/generate_test.go | 6 +- language/go/package.go | 22 +- 5 files changed, 453 insertions(+), 186 deletions(-) diff --git a/language/go/fileinfo.go b/language/go/fileinfo.go index 7f6671d67..31d8035c9 100644 --- a/language/go/fileinfo.go +++ b/language/go/fileinfo.go @@ -21,8 +21,10 @@ import ( "errors" "fmt" "go/ast" + "go/build/constraint" "go/parser" "go/token" + "io" "log" "os" "path" @@ -31,6 +33,7 @@ import ( "strings" "unicode" "unicode/utf8" + _ "unsafe" "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/internal/version" @@ -75,11 +78,11 @@ type fileInfo struct { // tags is a list of build tag lines. Each entry is the trimmed text of // a line after a "+build" prefix. - tags []tagLine + tags *buildTags // cppopts, copts, cxxopts and clinkopts contain flags that are part // of CPPFLAGS, CFLAGS, CXXFLAGS, and LDFLAGS directives in cgo comments. - cppopts, copts, cxxopts, clinkopts []taggedOpts + cppopts, copts, cxxopts, clinkopts []*cgoTagsAndOpts // hasServices indicates whether a .proto file has service definitions. hasServices bool @@ -93,78 +96,172 @@ type fileEmbed struct { pos token.Position } -// tagLine represents the space-separated disjunction of build tag groups -// in a line comment. -type tagLine []tagGroup +// buildTags represents the build tags specified in a file. +type buildTags struct { + // expr represents the parsed constraint expression + // that can be used to evaluate a file against a set + // of tags. + expr constraint.Expr + // rawTags represents the concrete tags that make up expr. + rawTags []string +} -// check returns true if at least one of the tag groups is satisfied. -func (l tagLine) check(c *config.Config, os, arch string) bool { - if len(l) == 0 { - return false +func newBuildTags(x constraint.Expr) (*buildTags, error) { + filtered, err := filterTags(x, func(tag string) bool { + return !isIgnoredTag(tag) + }) + if err != nil { + return nil, err } - for _, g := range l { - if g.check(c, os, arch) { - return true - } + + rawTags, err := collectTags(x) + if err != nil { + return nil, err } - return false + + return &buildTags{ + expr: filtered, + rawTags: rawTags, + }, nil } -// tagGroup represents a comma-separated conjuction of build tags. -type tagGroup []string +func (b *buildTags) tags() []string { + if b == nil { + return nil + } -// check returns true if all of the tags are true. Tags that start with -// "!" are negated (but "!!") is not allowed. Go release tags (e.g., "go1.8") -// are ignored. If the group contains an os or arch tag, but the os or arch -// parameters are empty, check returns false even if the tag is negated. -func (g tagGroup) check(c *config.Config, os, arch string) bool { - goConf := getGoConfig(c) - for _, t := range g { - if strings.HasPrefix(t, "!!") { // bad syntax, reject always - return false + return b.rawTags +} + +func (b *buildTags) eval(ok func(string) bool) bool { + if b == nil || b.expr == nil { + return true + } + + return b.expr.Eval(ok) +} + +func (b *buildTags) empty() bool { + if b == nil { + return true + } + + return len(b.rawTags) == 0 +} + +func filterTags(expr constraint.Expr, ok func(string) bool) (constraint.Expr, error) { + if expr == nil { + return nil, nil + } + + switch x := expr.(type) { + case *constraint.TagExpr: + if ok(x.Tag) { + return x, nil } - not := strings.HasPrefix(t, "!") - if not { - t = t[1:] + + case *constraint.NotExpr: + filtered, err := filterTags(x.X, ok) + if err != nil { + return nil, err } - if isIgnoredTag(t) { - // Release tags are treated as "unknown" and are considered true, - // whether or not they are negated. - continue + + if filtered != nil { + return &constraint.NotExpr{X: filtered}, nil } - var match bool - if _, ok := rule.KnownOSSet[t]; ok { - if os == "" { - return false - } - match = matchesOS(os, t) - } else if _, ok := rule.KnownArchSet[t]; ok { - if arch == "" { - return false - } - match = arch == t - } else { - match = goConf.genericTags[t] + + case *constraint.AndExpr: + a, err := filterTags(x.X, ok) + if err != nil { + return nil, err } - if not { - match = !match + + b, err := filterTags(x.Y, ok) + if err != nil { + return nil, err } - if !match { - return false + + if a != nil && b != nil { + return &constraint.AndExpr{ + X: a, + Y: b, + }, nil + + } else if a != nil { + return a, nil + + } else if b != nil { + return b, nil + } + + case *constraint.OrExpr: + a, err := filterTags(x.X, ok) + if err != nil { + return nil, err + } + + b, err := filterTags(x.Y, ok) + if err != nil { + return nil, err } + + if a != nil && b != nil { + return &constraint.OrExpr{ + X: a, + Y: b, + }, nil + + } else if a != nil { + return a, nil + + } else if b != nil { + return b, nil + } + default: + return nil, fmt.Errorf("unknown constraint type: %T", x) } - return true + + return nil, nil } -// taggedOpts a list of compile or link options which should only be applied +func collectTags(expr constraint.Expr) ([]string, error) { + var tags []string + _, err := filterTags(expr, func(tag string) bool { + tags = append(tags, tag) + return true + }) + if err != nil { + return nil, err + } + + return tags, err +} + +// cgoTagsAndOpts contains compile or link options which should only be applied // if the given set of build tags are satisfied. These options have already // been tokenized using the same algorithm that "go build" uses, then joined // with OptSeparator. -type taggedOpts struct { - tags tagLine +type cgoTagsAndOpts struct { + *buildTags opts string } +func (c *cgoTagsAndOpts) tags() []string { + if c == nil { + return nil + } + + return c.buildTags.tags() +} + +func (c *cgoTagsAndOpts) eval(ok func(string) bool) bool { + if c == nil { + return true + } + + return c.buildTags.eval(ok) +} + // optSeparator is a special character inserted between options that appeared // together in a #cgo directive. This allows options to be split, modified, // and escaped by other packages. @@ -376,6 +473,28 @@ func goFileInfo(path, rel string) fileInfo { return info } +// matchAuto interprets text as either a +build or //go:build expression (whichever works). +// Intended to match go/build.Context.matchAuto +func matchAuto(tokens []string) (*buildTags, error) { + if len(tokens) == 0 { + return nil, nil + } + + text := strings.Join(tokens, " ") + if strings.ContainsAny(text, "&|()") { + text = "//go:build " + text + } else { + text = "// +build " + text + } + + x, err := constraint.Parse(text) + if err != nil { + return nil, err + } + + return newBuildTags(x) +} + // saveCgo extracts CFLAGS, CPPFLAGS, CXXFLAGS, and LDFLAGS directives // from a comment above a "C" import. This is intended to match logic in // go/build.Context.saveCgo. @@ -393,27 +512,29 @@ func saveCgo(info *fileInfo, rel string, cg *ast.CommentGroup) error { } // Split at colon. - line = strings.TrimSpace(line[4:]) - i := strings.Index(line, ":") - if i < 0 { + line, argstr, ok := strings.Cut(strings.TrimSpace(line[4:]), ":") + if !ok { return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) } - line, optstr := strings.TrimSpace(line[:i]), strings.TrimSpace(line[i+1:]) - // Parse tags and verb. + // Parse GOOS/GOARCH stuff. f := strings.Fields(line) if len(f) < 1 { return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) } - verb := f[len(f)-1] - tags := parseTagsInGroups(f[:len(f)-1]) + + cond, verb := f[:len(f)-1], f[len(f)-1] + tags, err := matchAuto(cond) + if err != nil { + return err + } // Parse options. - opts, err := splitQuoted(optstr) + opts, err := splitQuoted(argstr) if err != nil { return fmt.Errorf("%s: invalid #cgo line: %s", info.path, orig) } - var ok bool + for i, opt := range opts { if opt, ok = expandSrcDir(opt, rel); !ok { return fmt.Errorf("%s: malformed #cgo argument: %s", info.path, orig) @@ -425,13 +546,16 @@ func saveCgo(info *fileInfo, rel string, cg *ast.CommentGroup) error { // Add tags to appropriate list. switch verb { case "CPPFLAGS": - info.cppopts = append(info.cppopts, taggedOpts{tags, joinedStr}) + info.cppopts = append(info.cppopts, &cgoTagsAndOpts{tags, joinedStr}) case "CFLAGS": - info.copts = append(info.copts, taggedOpts{tags, joinedStr}) + info.copts = append(info.copts, &cgoTagsAndOpts{tags, joinedStr}) case "CXXFLAGS": - info.cxxopts = append(info.cxxopts, taggedOpts{tags, joinedStr}) + info.cxxopts = append(info.cxxopts, &cgoTagsAndOpts{tags, joinedStr}) + case "FFLAGS": + // TODO: Add support + return fmt.Errorf("%s: unsupported #cgo verb: %s", verb, info.path) case "LDFLAGS": - info.clinkopts = append(info.clinkopts, taggedOpts{tags, joinedStr}) + info.clinkopts = append(info.clinkopts, &cgoTagsAndOpts{tags, joinedStr}) case "pkg-config": return fmt.Errorf("%s: pkg-config not supported: %s", info.path, orig) default: @@ -564,80 +688,91 @@ func safeCgoName(s string, spaces bool) bool { // rest of the file by a blank line. Each string in the returned slice // is the trimmed text of a line after a "+build" prefix. // Based on go/build.Context.shouldBuild. -func readTags(path string) ([]tagLine, error) { +func readTags(path string) (*buildTags, error) { f, err := os.Open(path) if err != nil { return nil, err } defer f.Close() - scanner := bufio.NewScanner(f) - // Pass 1: Identify leading run of // comments and blank lines, - // which must be followed by a blank line. - var lines []string - end := 0 + content, err := readComments(f) + if err != nil { + return nil, err + } + + content, goBuild, _, err := parseFileHeader(content) + if err != nil { + return nil, err + } + + if goBuild != nil { + x, err := constraint.Parse(string(goBuild)) + if err != nil { + return nil, err + } + + return newBuildTags(x) + } + + var fullConstraint constraint.Expr + // Search and parse +build tags + scanner := bufio.NewScanner(bytes.NewReader(content)) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) - if line == "" { - end = len(lines) + + if !constraint.IsPlusBuild(line) { continue } - if strings.HasPrefix(line, "//") { - lines = append(lines, line[len("//"):]) - continue + + x, err := constraint.Parse(line) + if err != nil { + return nil, err } - break - } - if err := scanner.Err(); err != nil { - return nil, err - } - lines = lines[:end] - // Pass 2: Process each line in the run. - var tagLines []tagLine - for _, line := range lines { - fields := strings.Fields(line) - if len(fields) > 0 && fields[0] == "+build" { - tagLines = append(tagLines, parseTagsInGroups(fields[1:])) + if fullConstraint != nil { + fullConstraint = &constraint.AndExpr{ + X: fullConstraint, + Y: x, + } + } else { + fullConstraint = x } } - return tagLines, nil -} -func parseTagsInGroups(groups []string) tagLine { - var l tagLine - for _, g := range groups { - l = append(l, tagGroup(strings.Split(g, ","))) + if scanner.Err() != nil { + return nil, scanner.Err() } - return l + + if fullConstraint == nil { + return nil, nil + } + + return newBuildTags(fullConstraint) } -func isOSArchSpecific(info fileInfo, cgoTags tagLine) (osSpecific, archSpecific bool) { +func isOSArchSpecific(info fileInfo, cgoTags *cgoTagsAndOpts) (osSpecific, archSpecific bool) { if info.goos != "" { osSpecific = true } if info.goarch != "" { archSpecific = true } - lines := info.tags - if len(cgoTags) > 0 { - lines = append(lines, cgoTags) - } - for _, line := range lines { - for _, group := range line { - for _, tag := range group { - tag = strings.TrimPrefix(tag, "!") - _, osOk := rule.KnownOSSet[tag] - if osOk { - osSpecific = true - } - _, archOk := rule.KnownArchSet[tag] - if archOk { - archSpecific = true - } + + checkTags := func(tags []string) { + for _, tag := range tags { + _, osOk := rule.KnownOSSet[tag] + if osOk { + osSpecific = true + } + _, archOk := rule.KnownArchSet[tag] + if archOk { + archSpecific = true } } } + checkTags(info.tags.tags()) + checkTags(cgoTags.tags()) + return osSpecific, archSpecific } @@ -665,22 +800,34 @@ func matchesOS(os, value string) bool { // if they are negated. // // The remaining arguments describe the file being tested. All of these may -// be empty or nil. osSuffix and archSuffix are filename suffixes. fileTags -// is a list tags from +build comments found near the top of the file. cgoTags +// be empty or nil. osSuffix and archSuffix are filename suffixes. tags +// is the parsed build tags found near the top of the file. cgoTags // is an extra set of tags in a #cgo directive. -func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, fileTags []tagLine, cgoTags tagLine) bool { +func checkConstraints(c *config.Config, os, arch, osSuffix, archSuffix string, tags *buildTags, cgoTags *cgoTagsAndOpts) bool { if osSuffix != "" && !matchesOS(os, osSuffix) || archSuffix != "" && archSuffix != arch { return false } - for _, l := range fileTags { - if !l.check(c, os, arch) { - return false + + goConf := getGoConfig(c) + checker := func(tag string) bool { + if _, ok := rule.KnownOSSet[tag]; ok { + if os == "" { + return false + } + return matchesOS(os, tag) + + } else if _, ok := rule.KnownArchSet[tag]; ok { + if arch == "" { + return false + } + return arch == tag + + } else { + return goConf.genericTags[tag] } } - if len(cgoTags) > 0 && !cgoTags.check(c, os, arch) { - return false - } - return true + + return tags.eval(checker) && cgoTags.eval(checker) } // rulesGoSupportsOS returns whether the os tag is recognized by the version of @@ -827,3 +974,14 @@ func parseGoEmbed(args string, pos token.Position) ([]fileEmbed, error) { } return list, nil } + +// In order to correctly capture the subtleties of build tag placement +// and to automatically stay up-to-date with the parsing semantics and +// syntax, we link to the stdlib version of header parsing. +//go:linkname parseFileHeader go/build.parseFileHeader +func parseFileHeader(_ []byte) ([]byte, []byte, bool, error) + +// readComments is like io.ReadAll, except that it only reads the leading +// block of comments in the file. +//go:linkname readComments go/build.readComments +func readComments(_ io.Reader) ([]byte, error) diff --git a/language/go/fileinfo_go_test.go b/language/go/fileinfo_go_test.go index 8c481d3a6..206012c75 100644 --- a/language/go/fileinfo_go_test.go +++ b/language/go/fileinfo_go_test.go @@ -16,6 +16,7 @@ limitations under the License. package golang import ( + "go/build/constraint" "io/ioutil" "os" "path/filepath" @@ -29,7 +30,8 @@ var ( fileInfoCmpOption = cmp.AllowUnexported( fileInfo{}, fileEmbed{}, - taggedOpts{}, + buildTags{}, + cgoTagsAndOpts{}, ) ) @@ -126,7 +128,10 @@ package foo `, fileInfo{ packageName: "foo", - tags: []tagLine{{{"linux"}, {"darwin"}}, {{"!ignore"}}}, + tags: &buildTags{ + expr: mustParseBuildTag(t, "(linux || darwin) && !ignore"), + rawTags: []string{"linux", "darwin", "ignore"}, + }, }, }, { @@ -142,7 +147,10 @@ package route `, fileInfo{ packageName: "route", - tags: []tagLine{{{"darwin"}, {"dragonfly"}, {"freebsd"}, {"netbsd"}, {"openbsd"}}}, + tags: &buildTags{ + expr: mustParseBuildTag(t, "darwin || dragonfly || freebsd || netbsd || openbsd"), + rawTags: []string{"darwin", "dragonfly", "freebsd", "netbsd", "openbsd"}, + }, }, }, { @@ -253,16 +261,16 @@ import "C" `, fileInfo{ isCgo: true, - cppopts: []taggedOpts{ + cppopts: []*cgoTagsAndOpts{ {opts: "-O1"}, }, - copts: []taggedOpts{ + copts: []*cgoTagsAndOpts{ {opts: "-O0"}, }, - cxxopts: []taggedOpts{ + cxxopts: []*cgoTagsAndOpts{ {opts: "-O2"}, }, - clinkopts: []taggedOpts{ + clinkopts: []*cgoTagsAndOpts{ {opts: strings.Join([]string{"-O3", "-O4"}, optSeparator)}, }, }, @@ -278,9 +286,12 @@ import "C" `, fileInfo{ isCgo: true, - copts: []taggedOpts{ + copts: []*cgoTagsAndOpts{ { - tags: tagLine{{"foo"}, {"bar", "!baz"}}, + buildTags: &buildTags{ + expr: mustParseBuildTag(t, "foo || (bar && !baz)"), + rawTags: []string{"foo", "bar", "baz"}, + }, opts: "-O0", }, }, @@ -296,7 +307,7 @@ import "C" `, fileInfo{ isCgo: true, - copts: []taggedOpts{ + copts: []*cgoTagsAndOpts{ {opts: "-O0"}, {opts: "-O1"}, }, @@ -313,7 +324,7 @@ import ("C") `, fileInfo{ isCgo: true, - copts: []taggedOpts{ + copts: []*cgoTagsAndOpts{ {opts: "-O0"}, }, }, @@ -460,3 +471,12 @@ func TestShellSafety(t *testing.T) { } } } + +func mustParseBuildTag(t *testing.T, in string) constraint.Expr { + x, err := constraint.Parse("//go:build " + in) + if err != nil { + t.Fatalf("%s: %s", in, err) + } + + return x +} diff --git a/language/go/fileinfo_test.go b/language/go/fileinfo_test.go index 7c73b2c20..e23975910 100644 --- a/language/go/fileinfo_test.go +++ b/language/go/fileinfo_test.go @@ -16,10 +16,10 @@ limitations under the License. package golang import ( + "go/build/constraint" "io/ioutil" "os" "path/filepath" - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -29,7 +29,7 @@ func TestOtherFileInfo(t *testing.T) { dir := "." for _, tc := range []struct { desc, name, source string - wantTags []tagLine + wantTag *buildTags }{ { "empty file", @@ -44,7 +44,10 @@ func TestOtherFileInfo(t *testing.T) { // +build baz,!ignore `, - []tagLine{{{"foo"}, {"bar"}}, {{"baz", "!ignore"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "(foo || bar) && (baz && !ignore)"), + rawTags: []string{"foo", "bar", "baz", "ignore"}, + }, }, } { t.Run(tc.desc, func(t *testing.T) { @@ -57,7 +60,7 @@ func TestOtherFileInfo(t *testing.T) { // Only check that we can extract tags. Everything else is covered // by other tests. - if diff := cmp.Diff(tc.wantTags, got.tags, fileInfoCmpOption); diff != "" { + if diff := cmp.Diff(tc.wantTag, got.tags, fileInfoCmpOption); diff != "" { t.Errorf("(-want, +got): %s", diff) } @@ -252,19 +255,21 @@ func TestFileNameInfo(t *testing.T) { }, }, } { - tc.want.name = tc.name - tc.want.path = filepath.Join("dir", tc.name) - - if got := fileNameInfo(tc.want.path); !reflect.DeepEqual(got, tc.want) { - t.Errorf("case %q: got %#v; want %#v", tc.desc, got, tc.want) - } + t.Run(tc.desc, func(t *testing.T) { + tc.want.name = tc.name + tc.want.path = filepath.Join("dir", tc.name) + got := fileNameInfo(tc.want.path) + if diff := cmp.Diff(tc.want, got, fileInfoCmpOption); diff != "" { + t.Errorf("(-want, +got): %s", diff) + } + }) } } func TestReadTags(t *testing.T) { for _, tc := range []struct { desc, source string - want []tagLine + want *buildTags }{ { "empty file", @@ -284,12 +289,18 @@ func TestReadTags(t *testing.T) { package main `, - []tagLine{{{"foo"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "foo"), + rawTags: []string{"foo"}, + }, }, { "single comment", "// +build foo\n\n", - []tagLine{{{"foo"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "foo"), + rawTags: []string{"foo"}, + }, }, { "multiple comments", @@ -297,7 +308,10 @@ package main // +build bar package main`, - []tagLine{{{"foo"}}, {{"bar"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "foo && bar"), + rawTags: []string{"foo", "bar"}, + }, }, { "multiple comments with blank", @@ -306,12 +320,39 @@ package main`, // +build bar package main`, - []tagLine{{{"foo"}}, {{"bar"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "foo && bar"), + rawTags: []string{"foo", "bar"}, + }, + }, + { + "Basic go:build", + `//go:build foo && bar + +package main`, + &buildTags{ + expr: mustParseBuildTag(t, "foo && bar"), + rawTags: []string{"foo", "bar"}, + }, + }, + { + "Both go:build and +build", + `//go:build foo && bar +// +build foo,bar + +package main`, + &buildTags{ + expr: mustParseBuildTag(t, "foo && bar"), + rawTags: []string{"foo", "bar"}, + }, }, { "comment with space", " // +build foo bar \n\n", - []tagLine{{{"foo"}, {"bar"}}}, + &buildTags{ + expr: mustParseBuildTag(t, "foo || bar"), + rawTags: []string{"foo", "bar"}, + }, }, { "slash star comment", @@ -319,24 +360,26 @@ package main`, nil, }, } { - f, err := ioutil.TempFile(".", "TestReadTags") - if err != nil { - t.Fatal(err) - } - path := f.Name() - defer os.Remove(path) - if err = f.Close(); err != nil { - t.Fatal(err) - } - if err = ioutil.WriteFile(path, []byte(tc.source), 0o600); err != nil { - t.Fatal(err) - } + t.Run(tc.desc, func(t *testing.T) { + f, err := ioutil.TempFile(".", "TestReadTags") + if err != nil { + t.Fatal(err) + } + path := f.Name() + defer os.Remove(path) + if err = f.Close(); err != nil { + t.Fatal(err) + } + if err = ioutil.WriteFile(path, []byte(tc.source), 0o600); err != nil { + t.Fatal(err) + } - if got, err := readTags(path); err != nil { - t.Fatal(err) - } else if diff := cmp.Diff(tc.want, got); diff != "" { - t.Errorf("(-want, +got): %s", diff) - } + if got, err := readTags(path); err != nil { + t.Fatal(err) + } else if diff := cmp.Diff(tc.want, got, fileInfoCmpOption); diff != "" { + t.Errorf("(-want, +got): %s", diff) + } + }) } } @@ -523,19 +566,65 @@ import "C" } path := filepath.Join(dir, filename) - if err := ioutil.WriteFile(path, []byte(content), 0o666); err != nil { + if err := ioutil.WriteFile(path, content, 0o666); err != nil { t.Fatal(err) } fi := goFileInfo(path, "") - var cgoTags tagLine + var cgoTags *cgoTagsAndOpts if len(fi.copts) > 0 { - cgoTags = fi.copts[0].tags + cgoTags = fi.copts[0] } got := checkConstraints(c, tc.os, tc.arch, fi.goos, fi.goarch, fi.tags, cgoTags) - if got != tc.want { - t.Errorf("got %v ; want %v", got, tc.want) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("(-want, +got): %s", diff) + } + }) + } +} + +func TestFilterBuildTags(t *testing.T) { + for _, tc := range []struct { + desc string + input constraint.Expr + want constraint.Expr + }{ + { + desc: "should be empty", + input: mustParseBuildTag(t, "go1.8 || go1.9"), + }, + { + desc: "filtered not is empty", + input: mustParseBuildTag(t, "!(go1.8 || go1.9)"), + }, + { + desc: "remaining not remains", + input: mustParseBuildTag(t, "!(foobar || go1.8 || go1.9)"), + want: mustParseBuildTag(t, "!foobar"), + }, + { + desc: "complex becomes empty", + input: mustParseBuildTag(t, "!(cgo && (go1.8 || go1.9) || race || msan)"), + }, + { + desc: "complex becomes simple", + input: mustParseBuildTag(t, "!(cgo && (go1.8 || go1.9 && (race && foobar)))"), + want: mustParseBuildTag(t, "!foobar"), + }, + { + desc: "complex becomes simple 2", + input: mustParseBuildTag(t, "!(cgo && (go1.8 || go1.9 && (race && foobar) || baz))"), + want: mustParseBuildTag(t, "!(foobar || baz)"), + }, + } { + t.Run(tc.desc, func(t *testing.T) { + bt, err := newBuildTags(tc.input) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.want, bt.expr); diff != "" { + t.Errorf("(-want, +got): %s", diff) } }) } diff --git a/language/go/generate_test.go b/language/go/generate_test.go index ea447eac9..4d087168c 100644 --- a/language/go/generate_test.go +++ b/language/go/generate_test.go @@ -30,6 +30,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/rule" "github.com/bazelbuild/bazel-gazelle/walk" bzl "github.com/bazelbuild/buildtools/build" + "github.com/google/go-cmp/cmp" ) func TestGenerateRules(t *testing.T) { @@ -97,9 +98,8 @@ func TestGenerateRules(t *testing.T) { } want := string(wantBytes) want = strings.ReplaceAll(want, "\r\n", "\n") - - if got != want { - t.Errorf("GenerateRules %q: got:\n%s\nwant:\n%s", rel, got, want) + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("(-want, +got): %s", diff) } }) }) diff --git a/language/go/package.go b/language/go/package.go index 38b1de7d3..20502e8ca 100644 --- a/language/go/package.go +++ b/language/go/package.go @@ -101,7 +101,7 @@ func (pkg *goPackage) addFile(c *config.Config, er *embedResolver, info fileInfo // Only add files in legacy mode. This is used to generate a filegroup // that contains all protos. In order modes, we get the .proto files // from information emitted by the proto language extension. - pkg.proto.addFile(c, info) + pkg.proto.addFile(info) } case info.isTest: if info.isCgo { @@ -277,29 +277,29 @@ func (t *goTarget) addFile(c *config.Config, er *embedResolver, info fileInfo) { } for _, cppopts := range info.cppopts { optAdd := add - if len(cppopts.tags) > 0 { - optAdd = getPlatformStringsAddFunction(c, info, cppopts.tags) + if !cppopts.empty() { + optAdd = getPlatformStringsAddFunction(c, info, cppopts) } optAdd(&t.cppopts, cppopts.opts) } for _, copts := range info.copts { optAdd := add - if len(copts.tags) > 0 { - optAdd = getPlatformStringsAddFunction(c, info, copts.tags) + if !copts.empty() { + optAdd = getPlatformStringsAddFunction(c, info, copts) } optAdd(&t.copts, copts.opts) } for _, cxxopts := range info.cxxopts { optAdd := add - if len(cxxopts.tags) > 0 { - optAdd = getPlatformStringsAddFunction(c, info, cxxopts.tags) + if !cxxopts.empty() { + optAdd = getPlatformStringsAddFunction(c, info, cxxopts) } optAdd(&t.cxxopts, cxxopts.opts) } for _, clinkopts := range info.clinkopts { optAdd := add - if len(clinkopts.tags) > 0 { - optAdd = getPlatformStringsAddFunction(c, info, clinkopts.tags) + if !clinkopts.empty() { + optAdd = getPlatformStringsAddFunction(c, info, clinkopts) } optAdd(&t.clinkopts, clinkopts.opts) } @@ -317,7 +317,7 @@ func protoTargetFromProtoPackage(name string, pkg proto.Package) protoTarget { return target } -func (t *protoTarget) addFile(c *config.Config, info fileInfo) { +func (t *protoTarget) addFile(info fileInfo) { t.sources.addGenericString(info.name) for _, imp := range info.imports { t.imports.addGenericString(imp) @@ -328,7 +328,7 @@ func (t *protoTarget) addFile(c *config.Config, info fileInfo) { // getPlatformStringsAddFunction returns a function used to add strings to // a *platformStringsBuilder under the same set of constraints. This is a // performance optimization to avoid evaluating constraints repeatedly. -func getPlatformStringsAddFunction(c *config.Config, info fileInfo, cgoTags tagLine) func(sb *platformStringsBuilder, ss ...string) { +func getPlatformStringsAddFunction(c *config.Config, info fileInfo, cgoTags *cgoTagsAndOpts) func(sb *platformStringsBuilder, ss ...string) { isOSSpecific, isArchSpecific := isOSArchSpecific(info, cgoTags) v := getGoConfig(c).rulesGoVersion