diff --git a/buildifier/README.md b/buildifier/README.md index b3a3a7c93..c77b6bfe7 100644 --- a/buildifier/README.md +++ b/buildifier/README.md @@ -32,6 +32,20 @@ in a directory recursively: buildifier -r path/to/dir ``` +Paths can be excluded from recursive discovery with repeatable `-exclude` +patterns. Patterns use the syntax of Bazel's `glob()`: `*` and `?` match within a +single path segment and `**` matches any number of segments. They are relative to +the workspace root, wherever buildifier runs, and use forward slashes on every +platform. A pattern matching a directory excludes everything below it: + +```bash +buildifier -r -exclude='third_party/**/vendor' -exclude='**/node_modules' . +``` + +By default, the paths listed in the workspace's `.bazelignore` and the patterns +passed to `ignore_directories()` in its `REPO.bazel` are excluded too. Use +`-respect_bazelignore=false` to discover them anyway. + 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 diff --git a/buildifier/buildifier.go b/buildifier/buildifier.go index 276c04d0a..116824ff4 100644 --- a/buildifier/buildifier.go +++ b/buildifier/buildifier.go @@ -191,8 +191,12 @@ func (b *buildifier) run(args []string) int { } else { files := args if b.config.Recursive { - var err error - files, err = utils.ExpandDirectories(&args) + excluder, err := utils.NewExcluder(b.config.Exclude, b.config.RespectBazelignore) + if err != nil { + fmt.Fprintf(os.Stderr, "buildifier: %v\n", err) + return 3 + } + files, err = utils.ExpandDirectories(&args, excluder) if err != nil { fmt.Fprintf(os.Stderr, "buildifier: %v\n", err) return 3 diff --git a/buildifier/config/config.go b/buildifier/config/config.go index 4efa1914d..99fa9516a 100644 --- a/buildifier/config/config.go +++ b/buildifier/config/config.go @@ -37,7 +37,8 @@ const buildifierJSONFilename = ".buildifier.json" // New constructs a Config with default values. func New() *Config { return &Config{ - InputType: "auto", + InputType: "auto", + RespectBazelignore: true, } } @@ -111,6 +112,13 @@ type Config struct { WarningsList []string `json:"warningsList,omitempty"` // Recursive instructs buildifier to find starlark files recursively Recursive bool `json:"recursive,omitempty"` + // Exclude contains glob() patterns, relative to the workspace root, to skip + // during recursive file discovery + Exclude ArrayFlags `json:"exclude,omitempty"` + // RespectBazelignore also skips the paths ignored by the workspace's + // .bazelignore and the ignore_directories() of its REPO.bazel during + // recursive file discovery + RespectBazelignore bool `json:"respectBazelignore"` // Verbose instructs buildifier to output verbose diagnostics Verbose bool `json:"verbose,omitempty"` // DiffCommand is the command to run when the formatting mode is diff @@ -177,6 +185,8 @@ 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", "glob() pattern, relative to the workspace root, to exclude when finding starlark files recursively") + flags.BoolVar(&c.RespectBazelignore, "respect_bazelignore", c.RespectBazelignore, "also exclude the paths ignored by .bazelignore and the ignore_directories() of REPO.bazel 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, json or sarif (default text)") diff --git a/buildifier/config/config_test.go b/buildifier/config/config_test.go index c73643338..94ca5eaa0 100644 --- a/buildifier/config/config_test.go +++ b/buildifier/config/config_test.go @@ -30,7 +30,8 @@ func ExampleNew() { fmt.Print(c.String()) // Output: // { - // "type": "auto" + // "type": "auto", + // "respectBazelignore": true // } } @@ -142,7 +143,8 @@ func ExampleExample() { // "unreachable", // "unsorted-dict-items", // "unused-variable" - // ] + // ], + // "respectBazelignore": true // } } @@ -160,6 +162,7 @@ func ExampleFlagSet() { // 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) ("") // disable_symlink_safety: per default Buildifier will not write to symlinks pointing outside of the Bazel workspace. Setting this to true will disable this behavior ("false") + // exclude: glob() pattern, relative to the workspace root, to exclude when finding starlark files recursively ("") // format: diagnostics format: text, json or sarif (default text) ("") // help: print usage information ("false") // lint: lint mode: off, warn, or fix (default off) ("") @@ -167,6 +170,7 @@ func ExampleFlagSet() { // multi_diff: the command specified by the -diff_command flag can diff multiple files in the style of tkdiff (default false) ("false") // path: assume BUILD file has this path relative to the workspace directory ("") // r: find starlark files recursively ("false") + // respect_bazelignore: also exclude the paths ignored by .bazelignore and the ignore_directories() of REPO.bazel when finding starlark files recursively ("true") // tables: path to JSON file with custom table definitions which will replace the built-in tables ("") // type: Input file type: build (for BUILD files), bzl (for .bzl files), workspace (for WORKSPACE files), module (for MODULE.bazel files), repo (for REPO.bazel files), vendor (for VENDOR.bazel files), default (for generic Starlark files) or auto (default, based on the filename) ("auto") // v: print verbose information to standard error ("false") @@ -186,6 +190,8 @@ func ExampleFlagSet_parse() { "-d", "--diff_command=diff", "--disable_symlink_safety=true", + "--exclude=vendor/*", + "--exclude=third_party/*", "--format=json", "--help", "--lint=fix", @@ -215,6 +221,11 @@ func ExampleFlagSet_parse() { // "lint": "fix", // "warnings": "+print,-no-effect", // "recursive": true, + // "exclude": [ + // "vendor/*", + // "third_party/*" + // ], + // "respectBazelignore": true, // "verbose": true, // "diffCommand": "diff", // "multiDiff": true, diff --git a/buildifier/integration_test.sh b/buildifier/integration_test.sh index e7ae2e764..dd64f2cd8 100755 --- a/buildifier/integration_test.sh +++ b/buildifier/integration_test.sh @@ -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") @@ -134,9 +136,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 @@ -376,7 +379,8 @@ cat > golden/.buildifier.example.json < 0 && matchSegments(pattern, segments[1:])) + } + if len(segments) == 0 { + return false + } + // Patterns are validated when added, so there is no error to handle. + if matched, _ := path.Match(pattern[0], segments[0]); !matched { + return false + } + return matchSegments(pattern[1:], segments[1:]) +} + +// escapeGlob escapes the glob metacharacters of a literal path. +func escapeGlob(literal string) string { + var b strings.Builder + for _, r := range literal { + switch r { + case '*', '?', '[', '\\': + b.WriteRune('\\') + } + b.WriteRune(r) + } + return b.String() +} + +// readBazelignore returns the paths listed in a .bazelignore file, if any. +func readBazelignore(filename string) ([]string, error) { + file, err := os.Open(filename) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, err + } + defer file.Close() + + var dirs []string + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + for strings.HasPrefix(line, "./") { + line = strings.TrimPrefix(line, "./") + } + if line = strings.TrimSuffix(line, "/"); line != "" { + dirs = append(dirs, line) + } + } + return dirs, scanner.Err() +} + +// readIgnoreDirectories returns the patterns passed to ignore_directories() in +// a REPO.bazel file, if any. +func readIgnoreDirectories(filename string) ([]string, error) { + data, err := os.ReadFile(filename) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, err + } + f, err := build.ParseRepo(filename, data) + if err != nil { + return nil, err + } + + var patterns []string + for _, stmt := range f.Stmt { + call, ok := stmt.(*build.CallExpr) + if !ok { + continue + } + if ident, ok := call.X.(*build.Ident); !ok || ident.Name != "ignore_directories" { + continue + } + for _, arg := range call.List { + if assign, ok := arg.(*build.AssignExpr); ok { + arg = assign.RHS + } + list, ok := arg.(*build.ListExpr) + if !ok { + continue + } + for _, item := range list.List { + if str, ok := item.(*build.StringExpr); ok { + patterns = append(patterns, str.Value) + } + } + } + } + return patterns, nil +} diff --git a/buildifier/utils/exclude_test.go b/buildifier/utils/exclude_test.go new file mode 100644 index 000000000..d32590501 --- /dev/null +++ b/buildifier/utils/exclude_test.go @@ -0,0 +1,188 @@ +/* +Copyright 2026 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "os" + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" +) + +func writeFiles(t *testing.T, root string, files map[string]string) { + t.Helper() + for filename, content := range files { + 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, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func TestExcluderMatchesGlobSyntax(t *testing.T) { + root := t.TempDir() + for _, tc := range []struct { + pattern string + path string + want bool + }{ + {"vendor", "vendor", true}, + {"vendor", "a/vendor", false}, + {"vendor/", "vendor", true}, + {"**/node_modules", "node_modules", true}, + {"**/node_modules", "a/b/node_modules", true}, + {"**/node_modules", "a/node_modules_x", false}, + {"vendor/*", "vendor/x", true}, + {"vendor/*", "vendor/x/y", false}, + {"*.bzl", "defs.bzl", true}, + {"*.bzl", "pkg/defs.bzl", false}, + {"a/**/b.bzl", "a/b.bzl", true}, + {"a/**/b.bzl", "a/x/y/b.bzl", true}, + {"a/**", "a", true}, + {"a/**", "a/x/y", true}, + {"**", "anything/at/all", true}, + {"?.bzl", "a.bzl", true}, + {"?.bzl", "ab.bzl", false}, + } { + e, err := newExcluder(root, []string{tc.pattern}, false) + if err != nil { + t.Fatalf("newExcluder(%q) returned error: %v", tc.pattern, err) + } + if got := e.excludedAbs(filepath.Join(root, filepath.FromSlash(tc.path))); got != tc.want { + t.Errorf("pattern %q, path %q: excluded = %t, want %t", tc.pattern, tc.path, got, tc.want) + } + } +} + +func TestExcluderNeverExcludesTheRootOrOutsidePaths(t *testing.T) { + root := t.TempDir() + e, err := newExcluder(root, []string{"**"}, false) + if err != nil { + t.Fatal(err) + } + if e.excludedAbs(root) { + t.Error("the root itself is excluded") + } + if e.excludedAbs(filepath.Dir(root)) { + t.Error("a path outside the root is excluded") + } +} + +func TestExcluderRejectsInvalidPatterns(t *testing.T) { + for _, pattern := range []string{"", "/abs", "a//b", "./a", "a/../b", "a**", "**b/c", "["} { + if _, err := newExcluder(t.TempDir(), []string{pattern}, false); err == nil { + t.Errorf("newExcluder(%q) succeeded, want an error", pattern) + } else if !strings.Contains(err.Error(), "invalid exclude pattern") { + t.Errorf("newExcluder(%q) error = %v, want an invalid exclude pattern error", pattern, err) + } + } +} + +func TestExcluderRespectsBazelignoreAndIgnoreDirectories(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + ".bazelignore": "# comment\n\nignored\n./dotted/\nlit*eral\n", + "REPO.bazel": "ignore_directories([\"**/node_modules\"])\n", + }) + + for _, respect := range []bool{true, false} { + e, err := newExcluder(root, nil, respect) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + path string + want bool + }{ + {"ignored", respect}, + {"dotted", respect}, + {"lit*eral", respect}, + // .bazelignore entries are literal paths, not globs. + {"litteral", false}, + {"pkg/node_modules", respect}, + {"pkg/ignored", false}, + } { + if got := e.excludedAbs(filepath.Join(root, filepath.FromSlash(tc.path))); got != tc.want { + t.Errorf("respect=%t, path %q: excluded = %t, want %t", respect, tc.path, got, tc.want) + } + } + } +} + +func TestExpandDirectoriesExcludes(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "MODULE.bazel": "", + "BUILD": "", + "included/defs.bzl": "", + "excluded/direct.bzl": "", + "excluded/nested/BUILD.bazel": "", + "other/skip.sky": "", + "other/keep.star": "", + "unreadable/BUILD": "", + }) + if runtime.GOOS != "windows" { + unreadable := filepath.Join(root, "unreadable") + if err := os.Chmod(unreadable, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(unreadable, 0o755) }) + } + t.Chdir(root) + + excluder, err := NewExcluder([]string{"excluded", "**/skip.sky", "unreadable"}, true) + if err != nil { + t.Fatal(err) + } + args := []string{"."} + files, err := ExpandDirectories(&args, excluder) + if err != nil { + t.Fatal(err) + } + want := []string{"BUILD", "MODULE.bazel", filepath.Join("included", "defs.bzl"), filepath.Join("other", "keep.star")} + if !reflect.DeepEqual(files, want) { + t.Errorf("ExpandDirectories() = %q, want %q", files, want) + } +} + +func TestExpandDirectoriesExcludesRelativeToTheWorkspaceRoot(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "MODULE.bazel": "", + "pkg/BUILD": "", + "pkg/excluded/BUILD.bazel": "", + }) + t.Chdir(filepath.Join(root, "pkg")) + + excluder, err := NewExcluder([]string{"pkg/excluded"}, true) + if err != nil { + t.Fatal(err) + } + args := []string{"."} + files, err := ExpandDirectories(&args, excluder) + if err != nil { + t.Fatal(err) + } + if want := []string{"BUILD"}; !reflect.DeepEqual(files, want) { + t.Errorf("ExpandDirectories() = %q, want %q", files, want) + } +} diff --git a/buildifier/utils/utils.go b/buildifier/utils/utils.go index a132d753e..3c15c03a7 100644 --- a/buildifier/utils/utils.go +++ b/buildifier/utils/utils.go @@ -47,8 +47,9 @@ func skip(info os.FileInfo) bool { } // 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 +// that the optional excluder excludes are skipped, without reading excluded directories. +func ExpandDirectories(args *[]string, excluder *Excluder) ([]string, error) { files := []string{} for _, arg := range *args { info, err := os.Stat(arg) @@ -59,7 +60,15 @@ 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 { + // Check exclusions first, so that excluded paths never fail the run, + // e.g. when they are unreadable. + if excluder.Excluded(filename) { + if err == nil && info.IsDir() { + return filepath.SkipDir + } + return nil + } if err != nil { return err } @@ -70,12 +79,12 @@ func ExpandDirectories(args *[]string) ([]string, error) { // 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 })