Use native exclude in rules

buildifier/README.md

@@ -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

buildifier/buildifier.go

@@ -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

buildifier/config/config.go

@@ -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
@@ -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)")

buildifier/config/config_test.go

@@ -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) ("")
@@ -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",
@@ -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,

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")
@@ -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
@@ -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
 

buildifier/internal/factory.bzl

@@ -48,7 +48,7 @@ def buildifier_attr_factory(test_rule = False):
         ),
         "exclude_patterns": attr.string_list(
             allow_empty = True,
-            doc = "A list of glob patterns passed to the find command. E.g. './vendor/*' to exclude the Go vendor directory. In test rules, this attribute requires the use of the no_sandbox attribute.",
+            doc = "A list of Go-style path patterns excluded from recursive file discovery. E.g. './vendor/*' to exclude the Go vendor directory. In test rules, this attribute requires the use of the no_sandbox attribute.",
         ),
         "mode": attr.string(
             default = "fix" if not test_rule else "diff",
@@ -155,12 +155,10 @@ def buildifier_impl_factory(ctx, test_rule = False):
     if ctx.attr.add_tables:
         args.append("-add_tables=%s" % ctx.file.add_tables.path)
 
-    exclude_patterns_str = ""
     if ctx.attr.exclude_patterns:
         if test_rule and not ctx.attr.no_sandbox:
             fail("Cannot use 'exclude_patterns' in a test rule without 'no_sandbox'")
-        exclude_patterns = ["\\! -path %s" % shell.quote(pattern) for pattern in ctx.attr.exclude_patterns]
-        exclude_patterns_str = " ".join(exclude_patterns)
+        args.extend(["--exclude=%s" % pattern for pattern in ctx.attr.exclude_patterns])
 
     workspace = ""
     if test_rule and ctx.attr.no_sandbox:
@@ -172,7 +170,6 @@ def buildifier_impl_factory(ctx, test_rule = False):
     substitutions = {
         "@@ARGS@@": shell.array_literal(args),
         "@@BUILDIFIER_SHORT_PATH@@": shell.quote(ctx.executable.buildifier.short_path),
-        "@@EXCLUDE_PATTERNS@@": exclude_patterns_str,
         "@@WORKSPACE@@": workspace,
     }
 

buildifier/runner.bash.template

@@ -9,11 +9,8 @@ buildifier_short_path=$(readlink "$BUILDIFIER_SHORT_PATH")
 
 # Use TEST_WORKSPACE to determine if the script is being ran under a test
 if [[ ! -z "${TEST_WORKSPACE+x}" && -z "${BUILD_WORKSPACE_DIRECTORY+x}" ]]; then
-  FIND_FILE_TYPE="l"
   # If WORKSPACE was provided, then the script is being run under a test in no_sandbox mode
   if [[ ! -z "${WORKSPACE:+x}" ]]; then
-    FIND_FILE_TYPE="f"
-
     # resolve the WORKSPACE symlink
     # use `realpath` if available and `readlink` otherwise (typically macOS)
     if command -v realpath &> /dev/null; then
@@ -41,20 +38,5 @@ else
   fi
 fi
 
-# Run buildifier on all starlark files
-find . \
-  -type "${FIND_FILE_TYPE:-f}" \
-  @@EXCLUDE_PATTERNS@@ \
-  \( -name '*.bzl' \
-    -o -name '*.sky' \
-    -o -name '*.star' \
-    -o -name '*.bazel' \
-    -o -name BUILD \
-    -o -name '*.BUILD' \
-    -o -name BUILD.oss \
-    -o -name 'BUILD.*.oss' \
-    -o -name WORKSPACE \
-    -o -name WORKSPACE.oss \
-    -o -name WORKSPACE.bzlmod \
-    -o -name 'WORKSPACE.*.oss' \
-  \) -print | xargs "$buildifier_short_path" "${ARGS[@]}"
+# Run buildifier on all starlark files.
+"$buildifier_short_path" "${ARGS[@]}" -r .

buildifier/runner.bat.template

@@ -9,29 +9,12 @@ rem Unquote the arguments
 set stripped_args=%stripped_args:'=%
 
 rem Get the absolute path to the buildifier executable
-for /f "tokens=2" %%i in ('findstr /r "\<buildifier\.exe\>" MANIFEST') do (set buildifier_abs_path=%%i)
+for /f "tokens=1,* delims= " %%i in ('findstr /r "\<buildifier\.exe\>" MANIFEST') do set "buildifier_abs_path=%%j"
 
-powershell ^
-$Files = Get-ChildItem -LiteralPath '%BUILD_WORKSPACE_DIRECTORY:/=\%' -Recurse -File -ErrorAction SilentlyContinue ^|^
-    Where-Object {^
-        $_.Name -eq 'BUILD' `^
-        -or $_.Name -eq 'WORKSPACE' `^
-        -or $_.Name -eq 'WORKSPACE.oss' `^
-        -or $_.Name -eq 'WORKSPACE.bzlmod' `^
-        -or $_.Name -eq 'BUILD.oss' `^
-        -or $_.Name -clike '*.bazel' `^
-        -or $_.Name -clike '*.bzl' `^
-        -or $_.Name -clike '*.sky' `^
-        -or $_.Name -clike '*.star' `^
-        -or $_.Name -clike '*.BUILD' `^
-        -or $_.Name -clike 'BUILD.*.oss' `^
-        -or $_.Name -clike 'WORKSPACE.*.oss'^
-    };^
- ^<# Process files in batches of 100- to avoid exceeding CreateProcess' maximum length of 32,767 characters #^> ^
-$i = 0;^
-while ($i -lt $Files.Count)^
-{^
-    $Batch = $Files[$i..($i + 99)];^
-    ^& '%buildifier_abs_path%' %stripped_args% $Batch.FullName;^
-    $i += $Batch.Count;^
-};
+if defined BUILD_WORKSPACE_DIRECTORY (
+    cd /d "%BUILD_WORKSPACE_DIRECTORY%"
+    if errorlevel 1 exit /b 1
+)
+
+"%buildifier_abs_path%" %stripped_args% -r .
+exit /b %ERRORLEVEL%

buildifier/utils/utils.go

@@ -19,7 +19,9 @@ limitations under the License.
 package utils
 
 import (
+	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -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)
@@ -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
 		})

buildifier/utils/utils_test.go

@@ -17,6 +17,10 @@ limitations under the License.
 package utils
 
 import (
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -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)
+	}
+}