Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions buildifier/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ in a directory recursively:
buildifier -r path/to/dir
```

Paths can be excluded from recursive discovery with repeatable `-exclude`
patterns. Patterns use Go's `path.Match` syntax, are matched against the full
traversed path, allow `*` and `?` to match path separators, and use
forward-slash separators on every platform:

```bash
buildifier -r -exclude='path/to/dir/vendor/*' path/to/dir
```

Buildifier supports the following file types: `BUILD`, `WORKSPACE`, `.bzl`, and
default, the latter is reserved for Starlark files buildifier doesn't know about
(e.g. configuration files for third-party projects that use Starlark). The
Expand Down
2 changes: 1 addition & 1 deletion buildifier/buildifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (b *buildifier) run(args []string) int {
files := args
if b.config.Recursive {
var err error
files, err = utils.ExpandDirectories(&args)
files, err = utils.ExpandDirectories(&args, b.config.Exclude...)
if err != nil {
fmt.Fprintf(os.Stderr, "buildifier: %v\n", err)
return 3
Expand Down
3 changes: 3 additions & 0 deletions buildifier/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ type Config struct {
WarningsList []string `json:"warningsList,omitempty"`
// Recursive instructs buildifier to find starlark files recursively
Recursive bool `json:"recursive,omitempty"`
// Exclude contains path patterns to skip during recursive file discovery
Exclude ArrayFlags `json:"exclude,omitempty"`
// Verbose instructs buildifier to output verbose diagnostics
Verbose bool `json:"verbose,omitempty"`
// DiffCommand is the command to run when the formatting mode is diff
Expand Down Expand Up @@ -172,6 +174,7 @@ func (c *Config) FlagSet(name string, errorHandling flag.ErrorHandling) *flag.Fl
flags.BoolVar(&c.Verbose, "v", c.Verbose, "print verbose information to standard error")
flags.BoolVar(&c.DiffMode, "d", c.DiffMode, "alias for -mode=diff")
flags.BoolVar(&c.Recursive, "r", c.Recursive, "find starlark files recursively")
flags.Var(&c.Exclude, "exclude", "path pattern to exclude when finding starlark files recursively")
flags.BoolVar(&c.MultiDiff, "multi_diff", c.MultiDiff, "the command specified by the -diff_command flag can diff multiple files in the style of tkdiff (default false)")
flags.StringVar(&c.Mode, "mode", c.Mode, "formatting mode: check, diff, or fix (default fix)")
flags.StringVar(&c.Format, "format", c.Format, "diagnostics format: text or json (default text)")
Expand Down
7 changes: 7 additions & 0 deletions buildifier/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func ExampleFlagSet() {
// config: path to .buildifier.json config file ("")
// d: alias for -mode=diff ("false")
// diff_command: command to run when the formatting mode is diff (default uses the BUILDIFIER_DIFF, BUILDIFIER_MULTIDIFF, and DISPLAY environment variables to create the diff command) ("")
// exclude: path pattern to exclude when finding starlark files recursively ("")
// format: diagnostics format: text or json (default text) ("")
// help: print usage information ("false")
// lint: lint mode: off, warn, or fix (default off) ("")
Expand All @@ -185,6 +186,8 @@ func ExampleFlagSet_parse() {
"--config=/path/to/.buildifier.json",
"-d",
"--diff_command=diff",
"--exclude=vendor/*",
"--exclude=third_party/*",
"--format=json",
"--help",
"--lint=fix",
Expand Down Expand Up @@ -214,6 +217,10 @@ func ExampleFlagSet_parse() {
// "lint": "fix",
// "warnings": "+print,-no-effect",
// "recursive": true,
// "exclude": [
// "vendor/*",
// "third_party/*"
// ],
// "verbose": true,
// "diffCommand": "diff",
// "multiDiff": true,
Expand Down
6 changes: 5 additions & 1 deletion buildifier/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ echo -e '{ "type": "build" }' > test_dir/.buildifier.test.json # demonstrate con
mkdir test_dir/workspace # name of a starlark file, but a directory
mkdir test_dir/.git # contents should be ignored
echo -e "a+b" > test_dir/.git/git.bzl
mkdir -p test_dir/excluded/nested
echo -e "$INPUT" > test_dir/excluded/nested/excluded.bzl
cat > test_dir/MODULE.bazel <<'EOF'
module(name='my-module',version='1.0',compatibility_level=1)
include("cpp.MODULE.bazel")
Expand Down Expand Up @@ -126,9 +128,10 @@ EOF
cp test_dir/foo.bar golden/foo.bar
cp test_dir/subdir/build golden/build
cp test_dir/.git/git.bzl golden/git.bzl
cp test_dir/excluded/nested/excluded.bzl golden/excluded.bzl

"$buildifier" < test_dir/BUILD > stdout
"$buildifier" -r test_dir
"$buildifier" -r --exclude='test_dir/excluded/*' test_dir
"$buildifier" test.bzl
"$buildifier" --path=foo.bzl test2.bzl
"$buildifier" --config=test_dir/.buildifier.test.json < test_dir/test.bzl > test_dir/test.bzl.BUILD.out
Expand Down Expand Up @@ -372,6 +375,7 @@ diff -u test.bzl golden/test.bzl.golden
diff -u test2.bzl golden/test.bzl.golden
diff -u stdout golden/test.bzl.golden
diff -u test_dir/.git/git.bzl golden/git.bzl
diff -u test_dir/excluded/nested/excluded.bzl golden/excluded.bzl
diff -u test_dir/MODULE.bazel golden/MODULE.bazel.golden
diff -u test_dir/.buildifier.example.json golden/.buildifier.example.json

Expand Down
58 changes: 52 additions & 6 deletions buildifier/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ limitations under the License.
package utils

import (
"fmt"
"os"
"path"
"path/filepath"
"strings"

Expand All @@ -46,9 +48,46 @@ func skip(info os.FileInfo) bool {
return info.IsDir() && info.Name() == ".git"
}

func normalizePathForMatch(value string) string {
value = filepath.ToSlash(value)
for strings.HasPrefix(value, "./") {
value = strings.TrimPrefix(value, "./")
}
return value
}

// matchPath uses path.Match syntax, but allows '*' and '?' to match path separators.
func matchPath(pattern, filename string) (bool, error) {
const separatorPlaceholder = "\x00"
pattern = strings.ReplaceAll(normalizePathForMatch(pattern), "/", separatorPlaceholder)
filename = strings.ReplaceAll(normalizePathForMatch(filename), "/", separatorPlaceholder)
return path.Match(pattern, filename)
}

func isExcluded(filename string, patterns []string) (bool, error) {
for _, pattern := range patterns {
matched, err := matchPath(pattern, filename)
if err != nil {
return false, err
}
if matched {
return true, nil
}
}
return false, nil
}

// ExpandDirectories takes a list of file/directory names and returns a list with file names
// by traversing each directory recursively and searching for relevant Starlark files.
func ExpandDirectories(args *[]string) ([]string, error) {
// by traversing each directory recursively and searching for relevant Starlark files. Paths
// matching any of the optional exclude patterns are skipped. Exclude patterns use path.Match
// syntax and are matched with slash separators on every platform.
func ExpandDirectories(args *[]string, excludePatterns ...string) ([]string, error) {
for _, pattern := range excludePatterns {
if _, err := matchPath(pattern, ""); err != nil {
return nil, fmt.Errorf("invalid exclude pattern %q: %w", pattern, err)
}
}

files := []string{}
for _, arg := range *args {
info, err := os.Stat(arg)
Expand All @@ -59,23 +98,30 @@ func ExpandDirectories(args *[]string) ([]string, error) {
files = append(files, arg)
continue
}
err = filepath.Walk(arg, func(path string, info os.FileInfo, err error) error {
err = filepath.Walk(arg, func(filename string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if skip(info) {
excluded, err := isExcluded(filename, excludePatterns)
if err != nil {
return err
}
if info.IsDir() && (skip(info) || excluded) {
return filepath.SkipDir
}
if excluded {
return nil
}
if !info.IsDir() && isStarlarkFile(info.Name()) {
// Don't traverse into directory symlinks such as bazel-foo.bzl
// for a project called foo.bzl.
if info.Mode()&os.ModeSymlink != 0 {
stat, err := os.Stat(path)
stat, err := os.Stat(filename)
if err != nil || stat.IsDir() {
return nil
}
}
files = append(files, path)
files = append(files, filename)
}
return nil
})
Expand Down
71 changes: 71 additions & 0 deletions buildifier/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ limitations under the License.
package utils

import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"
)

Expand Down Expand Up @@ -165,3 +169,70 @@ func TestIsStarlarkFile(t *testing.T) {
}
}
}

func TestExpandDirectoriesExcludesPaths(t *testing.T) {
root := t.TempDir()
for _, filename := range []string{
"BUILD",
"included/defs.bzl",
"excluded/direct.bzl",
"excluded/nested/BUILD.bazel",
"other/skip.sky",
"other/keep.star",
} {
path := filepath.Join(root, filepath.FromSlash(filename))
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, nil, 0o644); err != nil {
t.Fatal(err)
}
}

args := []string{root}
files, err := ExpandDirectories(
&args,
filepath.Join(root, "excluded", "*"),
filepath.Join(root, "*", "skip.sky"),
)
if err != nil {
t.Fatal(err)
}

want := []string{
filepath.Join(root, "BUILD"),
filepath.Join(root, "included", "defs.bzl"),
filepath.Join(root, "other", "keep.star"),
}
if !reflect.DeepEqual(files, want) {
t.Errorf("ExpandDirectories() = %q, want %q", files, want)
}
}

func TestMatchPathAllowsWildcardsAcrossSeparators(t *testing.T) {
for _, tc := range []struct {
pattern string
filename string
want bool
}{
{pattern: "./vendor/*", filename: "vendor/direct.bzl", want: true},
{pattern: "./vendor/*", filename: "vendor/nested/defs.bzl", want: true},
{pattern: "./vendor/*.bzl", filename: "vendor/nested/defs.bzl", want: true},
{pattern: "./vendor/*.bzl", filename: "third_party/defs.bzl", want: false},
} {
got, err := matchPath(tc.pattern, tc.filename)
if err != nil {
t.Errorf("matchPath(%q, %q) returned error: %v", tc.pattern, tc.filename, err)
} else if got != tc.want {
t.Errorf("matchPath(%q, %q) = %t, want %t", tc.pattern, tc.filename, got, tc.want)
}
}
}

func TestExpandDirectoriesRejectsInvalidExcludePattern(t *testing.T) {
args := []string{t.TempDir()}
_, err := ExpandDirectories(&args, "[")
if err == nil || !strings.Contains(err.Error(), "syntax error in pattern") {
t.Fatalf("ExpandDirectories() error = %v, want invalid pattern error", err)
}
}