Skip to content

Commit

Permalink
Allow MatchAttrs to be slices (roll forward) (bazel-contrib#744)
Browse files Browse the repository at this point in the history
When bazel-contrib#730 was opened, it was based on top of bazel-contrib#720. So the tests were passing on bazel-contrib#730. However, bazel-contrib#720 was reverted on master before bazel-contrib#730 was merged. When merging bazel-contrib#730 onto master, its tests started to fail. This PR reverted bazel-contrib#743 and fixed the failing tests

Fixed bazel-contrib#742
  • Loading branch information
linzhp authored Mar 30, 2020
1 parent c7d8513 commit b6658b6
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 6 deletions.
62 changes: 62 additions & 0 deletions cmd/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2975,3 +2975,65 @@ go_repository(
},
})
}

func TestMatchProtoLibrary(t *testing.T) {
files := []testtools.FileSpec{
{
Path: "WORKSPACE",
},
{
Path: "proto/BUILD.bazel",
Content: `
load("@rules_proto//proto:defs.bzl", "proto_library")
# gazelle:prefix example.com/foo
proto_library(
name = "existing_proto",
srcs = ["foo.proto"],
)
`,
},
{
Path: "proto/foo.proto",
Content: `syntax = "proto3";`,
},
}
dir, cleanup := testtools.CreateFiles(t, files)
defer cleanup()

args := []string{"update"}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}

testtools.CheckFiles(t, dir, []testtools.FileSpec{
{
Path: "proto/BUILD.bazel",
Content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
# gazelle:prefix example.com/foo
proto_library(
name = "existing_proto",
srcs = ["foo.proto"],
visibility = ["//visibility:public"],
)
go_proto_library(
name = "foo_go_proto",
importpath = "example.com/foo",
proto = ":existing_proto",
visibility = ["//visibility:public"],
)
go_library(
name = "go_default_library",
embed = [":foo_go_proto"],
importpath = "example.com/foo",
visibility = ["//visibility:public"],
)`,
},
})
}
1 change: 1 addition & 0 deletions language/proto/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import "github.com/bazelbuild/bazel-gazelle/rule"

var protoKinds = map[string]rule.KindInfo{
"proto_library": {
MatchAttrs: []string{"srcs"},
NonEmptyAttrs: map[string]bool{"srcs": true},
MergeableAttrs: map[string]bool{
"srcs": true,
Expand Down
29 changes: 23 additions & 6 deletions merger/merger.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ package merger

import (
"fmt"
"sort"
"strings"

"github.com/bazelbuild/bazel-gazelle/rule"
Expand Down Expand Up @@ -222,19 +223,15 @@ func Match(rules []*rule.Rule, x *rule.Rule, info rule.KindInfo) (*rule.Rule, er

for _, key := range info.MatchAttrs {
var attrMatches []*rule.Rule
xvalue := x.AttrString(key)
if xvalue == "" {
continue
}
for _, y := range kindMatches {
if xvalue == y.AttrString(key) {
if attrMatch(x, y, key) {
attrMatches = append(attrMatches, y)
}
}
if len(attrMatches) == 1 {
return attrMatches[0], nil
} else if len(attrMatches) > 1 {
return nil, fmt.Errorf("could not merge %s(%s): multiple rules have the same attribute %s = %q", xkind, xname, key, xvalue)
return nil, fmt.Errorf("could not merge %s(%s): multiple rules have the same attribute %s", xkind, xname, key)
}
}

Expand All @@ -248,3 +245,23 @@ func Match(rules []*rule.Rule, x *rule.Rule, info rule.KindInfo) (*rule.Rule, er

return nil, nil
}

func attrMatch(x, y *rule.Rule, key string) bool {
xValue := x.AttrString(key)
if xValue != "" && xValue == y.AttrString(key) {
return true
}
xValues := x.AttrStrings(key)
yValues := y.AttrStrings(key)
if xValues == nil || yValues == nil || len(xValues) != len(yValues) {
return false
}
sort.Strings(xValues)
sort.Strings(yValues)
for i, v := range xValues {
if v != yValues[i] {
return false
}
}
return true
}
9 changes: 9 additions & 0 deletions merger/merger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,15 @@ go_binary(name = "y")
go_binary(name = "z")
`,
wantError: true,
}, {
desc: "srcs match",
gen: `proto_library(name = "proto1", srcs = ["foo.proto", "bar.proto"])`,
old: `proto_library(name = "proto2", srcs = ["bar.proto", "foo.proto"])`,
wantIndex: 0,
}, {
desc: "importpath match",
gen: `go_proto_library(name = "go_proto1", importpath="example.com/foo")`,
old: `go_proto_library(name = "go_proto2", importpath="example.com/foo")`,
},
} {
t.Run(tc.desc, func(t *testing.T) {
Expand Down

0 comments on commit b6658b6

Please sign in to comment.