Skip to content

Commit

Permalink
Gazelle: sort srcs and deps string lists in buildifier order (#613)
Browse files Browse the repository at this point in the history
This is a workaround for bazelbuild/buildtools#122. When buildifier
sorts lists, it only processes list expressions as arguments for
certain attributes. It doesn't process lists inside select or lists
concatenated with select. When that bug is fixed, this code can be
removed.

Fixes #584
  • Loading branch information
jayconrod authored Jul 10, 2017
1 parent f4db0f4 commit f6dd5ae
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 0 deletions.
93 changes: 93 additions & 0 deletions go/tools/gazelle/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,99 @@ func TestNoGoPrefixArgOrRule(t *testing.T) {
}
}

// TestSelectLabelsSorted checks that string lists in srcs and deps are sorted
// using buildifier order, even if they are inside select expressions.
// This applies to both new and existing lists and should preserve comments.
// buildifier does not do this yet bazelbuild/buildtools#122, so we do this
// in addition to calling build.Rewrite.
func TestSelectLabelsSorted(t *testing.T) {
dir, err := createFiles([]fileSpec{
{path: "WORKSPACE"},
{
path: "BUILD",
content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_prefix")
go_prefix("example.com/foo")
go_library(
name = "go_default_library",
srcs = select({
"@io_bazel_rules_go//go/platform:linux_amd64": [
# top comment
"foo.go", # side comment
# bar comment
"bar.go",
],
"//conditions:default": [],
}),
)
`,
},
{
path: "foo.go",
content: `
// +build linux
package foo
import (
_ "example.com/foo/outer"
_ "example.com/foo/outer/inner"
_ "github.com/jr_hacker/tools"
)
`,
},
{
path: "bar.go",
content: `// +build linux
package foo
`,
},
{path: "outer/outer.go", content: "package outer"},
{path: "outer/inner/inner.go", content: "package inner"},
})
want := `load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_prefix")
go_prefix("example.com/foo")
go_library(
name = "go_default_library",
srcs = select({
"@io_bazel_rules_go//go/platform:linux_amd64": [
# top comment
# bar comment
"bar.go",
"foo.go", # side comment
],
"//conditions:default": [],
}),
visibility = ["//visibility:public"],
deps = select({
"@io_bazel_rules_go//go/platform:linux_amd64": [
"//outer:go_default_library",
"//outer/inner:go_default_library",
"@com_github_jr_hacker_tools//:go_default_library",
],
"//conditions:default": [],
}),
)
`
if err != nil {
t.Fatal(err)
}

if err := runGazelle(dir, nil); err != nil {
t.Fatal(err)
}
if got, err := ioutil.ReadFile(filepath.Join(dir, "BUILD")); err != nil {
t.Fatal(err)
} else if string(got) != want {
t.Fatalf("got %s ; want %s", string(got), want)
}
}

// TODO(jayconrod): more tests
// multiple directories can be visited
// error if directory not under -repo_root
Expand Down
2 changes: 2 additions & 0 deletions go/tools/gazelle/gazelle/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func processPackage(c *config.Config, r rules.LabelResolver, emit emitFunc, pkg

if oldFile == nil {
// No existing file, so no merge required.
rules.SortLabels(genFile)
bf.Rewrite(genFile, nil) // have buildifier 'format' our rules.
if err := emit(c, genFile); err != nil {
log.Print(err)
Expand All @@ -108,6 +109,7 @@ func processPackage(c *config.Config, r rules.LabelResolver, emit emitFunc, pkg
return
}

rules.SortLabels(mergedFile)
bf.Rewrite(mergedFile, nil) // have buildifier 'format' our rules.
if err := emit(c, mergedFile); err != nil {
log.Print(err)
Expand Down
1 change: 1 addition & 0 deletions go/tools/gazelle/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ go_library(
"resolve_external.go",
"resolve_structured.go",
"resolve_vendored.go",
"sort_labels.go",
],
visibility = ["//visibility:public"],
deps = [
Expand Down
130 changes: 130 additions & 0 deletions go/tools/gazelle/rules/sort_labels.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
package rules

import (
"sort"
"strings"

bf "github.com/bazelbuild/buildtools/build"
)

var (
goRuleKinds = map[string]bool{
"cgo_library": true,
"go_binary": true,
"go_library": true,
"go_test": true,
}
sortedAttrs = []string{"srcs", "deps"}
)

// SortLabels sorts lists of strings in "srcs" and "deps" attributes of
// Go rules using the same order as buildifier. Buildifier also sorts string
// lists, but not those involved with "select" expressions.
// TODO(jayconrod): remove this when bazelbuild/buildtools#122 is fixed.
func SortLabels(f *bf.File) {
for _, s := range f.Stmt {
c, ok := s.(*bf.CallExpr)
if !ok {
continue
}
r := bf.Rule{c}
if !goRuleKinds[r.Kind()] {
continue
}
for _, key := range []string{"srcs", "deps"} {
attr := r.AttrDefn(key)
if attr == nil {
continue
}
bf.Walk(attr.Y, sortExprLabels)
}
}
}

func sortExprLabels(e bf.Expr, _ []bf.Expr) {
list, ok := e.(*bf.ListExpr)
if !ok || len(list.List) == 0 {
return
}

keys := make([]stringSortKey, len(list.List))
for i, elem := range list.List {
s, ok := elem.(*bf.StringExpr)
if !ok {
return // don't sort lists unless all elements are strings
}
keys[i] = makeSortKey(i, s)
}

before := keys[0].x.Comment().Before
keys[0].x.Comment().Before = nil
sort.Sort(byStringExpr(keys))
keys[0].x.Comment().Before = append(before, keys[0].x.Comment().Before...)
for i, k := range keys {
list.List[i] = k.x
}
}

// Code below this point is adapted from
// github.com/bazelbuild/buildtools/build/rewrite.go

// A stringSortKey records information about a single string literal to be
// sorted. The strings are first grouped into four phases: most strings,
// strings beginning with ":", strings beginning with "//", and strings
// beginning with "@". The next significant part of the comparison is the list
// of elements in the value, where elements are split at `.' and `:'. Finally
// we compare by value and break ties by original index.
type stringSortKey struct {
phase int
split []string
value string
original int
x bf.Expr
}

func makeSortKey(index int, x *bf.StringExpr) stringSortKey {
key := stringSortKey{
value: x.Value,
original: index,
x: x,
}

switch {
case strings.HasPrefix(x.Value, ":"):
key.phase = 1
case strings.HasPrefix(x.Value, "//"):
key.phase = 2
case strings.HasPrefix(x.Value, "@"):
key.phase = 3
}

key.split = strings.Split(strings.Replace(x.Value, ":", ".", -1), ".")
return key
}

// byStringExpr implements sort.Interface for a list of stringSortKey.
type byStringExpr []stringSortKey

func (x byStringExpr) Len() int { return len(x) }
func (x byStringExpr) Swap(i, j int) { x[i], x[j] = x[j], x[i] }

func (x byStringExpr) Less(i, j int) bool {
xi := x[i]
xj := x[j]

if xi.phase != xj.phase {
return xi.phase < xj.phase
}
for k := 0; k < len(xi.split) && k < len(xj.split); k++ {
if xi.split[k] != xj.split[k] {
return xi.split[k] < xj.split[k]
}
}
if len(xi.split) != len(xj.split) {
return len(xi.split) < len(xj.split)
}
if xi.value != xj.value {
return xi.value < xj.value
}
return xi.original < xj.original
}

0 comments on commit f6dd5ae

Please sign in to comment.