Support recursive glob (.**) in bufimageutil type filters

A trailing ".**" on an include type name now matches the named element and
every symbol nested beneath it: a package and all its sub-packages and types,
or a message and all its nested types. Exclusion was already recursive for
named elements and is now recursive for packages too, so the suffix is accepted
but has no extra effect there.

Author: jhump

private/bufpkg/bufimage/bufimageutil/bufimageutil.go

@@ -107,6 +107,11 @@ func WithAllowIncludeOfImportedType() ImageFilterOption {
 // For example, "google.protobuf.Any" or "buf.validate". Types may be nested,
 // and can be any package, message, enum, extension, service or method name.
 //
+// A name ending in ".**" is a recursive glob that matches the named element and
+// every symbol nested beneath it. For example, "acme.foo.**" matches the package
+// "acme.foo", all of its sub-packages, and all of their types; "acme.Foo.**"
+// matches the message "acme.Foo" and all of its nested types.
+//
 // If the type does not exist in the image, an error wrapping
 // [ErrImageFilterTypeNotFound] will be returned.
 func WithIncludeTypes(typeNames ...string) ImageFilterOption {
@@ -127,6 +132,11 @@ func WithIncludeTypes(typeNames ...string) ImageFilterOption {
 // For example, "google.protobuf.Any" or "buf.validate". Types may be nested,
 // and can be any package, message, enum, extension, service or method name.
 //
+// Exclusion is always recursive: excluding a name removes the named element and
+// every symbol nested beneath it. For example, excluding a package removes the
+// package, all of its sub-packages, and all of their types. A trailing ".**" is
+// accepted for symmetry with [WithIncludeTypes] but has no additional effect.
+//
 // If the  type does not exist in the image, an error wrapping
 // [ErrImageFilterTypeNotFound] will be returned.
 func WithExcludeTypes(typeNames ...string) ImageFilterOption {
@@ -345,6 +355,7 @@ func (t *transitiveClosure) hasOption(
 
 func (t *transitiveClosure) includeType(
 	typeName protoreflect.FullName,
+	recursive bool,
 	imageIndex *imageIndex,
 	options *imageFilterOptions,
 ) error {
@@ -372,6 +383,12 @@ func (t *transitiveClosure) includeType(
 		if err := t.addElement(descriptorInfo.element, "", false, imageIndex, options); err != nil {
 			return fmt.Errorf("inclusion of type %q: %w", typeName, err)
 		}
+		if recursive {
+			// Auto-include any symbol nested beneath the named element.
+			if err := t.includeChildElements(descriptorInfo.element, imageIndex, options); err != nil {
+				return fmt.Errorf("inclusion of type %q: %w", typeName, err)
+			}
+		}
 		return nil
 	}
 	// It could be a package name
@@ -381,9 +398,14 @@ func (t *transitiveClosure) includeType(
 		return fmt.Errorf("inclusion of type %q: %w", typeName, ErrImageFilterTypeNotFound)
 	}
 	if !options.allowImportedTypes {
-		// if package includes only imported files, then reject
+		// If the package contains only imported files, then reject. For a
+		// recursive glob, sub-packages count toward this check too.
+		packageFiles := pkg.files
+		if recursive {
+			packageFiles = appendPackageFiles(nil, pkg)
+		}
 		onlyImported := true
-		for _, file := range pkg.files {
+		for _, file := range packageFiles {
 			if !file.IsImport() {
 				onlyImported = false
 				break
@@ -393,18 +415,91 @@ func (t *transitiveClosure) includeType(
 			return fmt.Errorf("inclusion of type %q: %w", typeName, ErrImageFilterTypeIsImport)
 		}
 	}
+	return t.includePackage(pkg, recursive, true, imageIndex, options)
+}
+
+// includePackage adds the files of the given package to the closure, recursing
+// into sub-packages when recursive is set.
+//
+// The top-level (named) package is an error if it has been excluded: including
+// and excluding the same package expand to the same set and cancel out, exactly
+// as for a non-recursive package include. Sub-packages reached only via the
+// recursive glob are instead skipped when excluded, so that e.g. including
+// "a.b.**" while excluding "a.b.c" succeeds.
+func (t *transitiveClosure) includePackage(
+	pkg *packageInfo,
+	recursive bool,
+	isTopLevel bool,
+	imageIndex *imageIndex,
+	options *imageFilterOptions,
+) error {
 	for _, file := range pkg.files {
 		fileDescriptor := file.FileDescriptorProto()
 		if mode := t.elements[fileDescriptor]; mode == inclusionModeExcluded {
-			return fmt.Errorf("inclusion of excluded package %q", typeName)
+			if !isTopLevel {
+				continue
+			}
+			return fmt.Errorf("inclusion of excluded package %q", pkg.fullName)
 		}
 		if err := t.addElement(fileDescriptor, "", false, imageIndex, options); err != nil {
-			return fmt.Errorf("inclusion of type %q: %w", typeName, err)
+			return fmt.Errorf("inclusion of type %q: %w", pkg.fullName, err)
+		}
+	}
+	if !recursive {
+		return nil
+	}
+	for _, subPackage := range pkg.subPackages {
+		if err := t.includePackage(subPackage, recursive, false, imageIndex, options); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// includeChildElements recursively adds the symbols nested beneath the given
+// element to the closure. Only messages have such children: their nested
+// messages and enums. Other element kinds have nothing to expand.
+func (t *transitiveClosure) includeChildElements(
+	descriptor namedDescriptor,
+	imageIndex *imageIndex,
+	options *imageFilterOptions,
+) error {
+	message, ok := descriptor.(*descriptorpb.DescriptorProto)
+	if !ok {
+		return nil
+	}
+	for _, nestedMessage := range message.GetNestedType() {
+		if mode := t.elements[nestedMessage]; mode == inclusionModeExcluded {
+			continue
+		}
+		if err := t.addElement(nestedMessage, "", false, imageIndex, options); err != nil {
+			return err
+		}
+		if err := t.includeChildElements(nestedMessage, imageIndex, options); err != nil {
+			return err
+		}
+	}
+	for _, nestedEnum := range message.GetEnumType() {
+		if mode := t.elements[nestedEnum]; mode == inclusionModeExcluded {
+			continue
+		}
+		if err := t.addElement(nestedEnum, "", false, imageIndex, options); err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
+// appendPackageFiles appends the files of the given package and all of its
+// sub-packages, recursively, to files.
+func appendPackageFiles(files []bufimage.ImageFile, pkg *packageInfo) []bufimage.ImageFile {
+	files = append(files, pkg.files...)
+	for _, subPackage := range pkg.subPackages {
+		files = appendPackageFiles(files, subPackage)
+	}
+	return files
+}
+
 func (t *transitiveClosure) addImport(fromPath, toPath string) {
 	if _, ok := t.imports[toPath]; !ok {
 		t.imports[toPath] = nil // mark as seen
@@ -616,6 +711,8 @@ func (t *transitiveClosure) excludeType(
 ) error {
 	descriptorInfo, ok := imageIndex.ByName[typeName]
 	if ok {
+		// excludeElement recurses into all nested elements: a symbol cannot be
+		// removed while keeping the types nested within it.
 		return t.excludeElement(descriptorInfo.element, imageIndex, options)
 	}
 	// It could be a package name
@@ -624,8 +721,13 @@ func (t *transitiveClosure) excludeType(
 		// but it's not...
 		return fmt.Errorf("exclusion of type %q: %w", typeName, ErrImageFilterTypeNotFound)
 	}
-	// Exclude the package and all of its files.
-	for _, file := range pkg.files {
+	// Exclude the package, all of its files, and all of its sub-packages.
+	// TODO: Exclusion is always recursive. Ideally a name without a ".**" suffix
+	//   would exclude only the named package's own files (not its sub-packages),
+	//   matching the include semantics. Making non-glob exclusion non-recursive
+	//   is an incompatible behavior change that would require auditing and
+	//   updating all callers, so it is deferred.
+	for _, file := range appendPackageFiles(nil, pkg) {
 		fileDescriptor := file.FileDescriptorProto()
 		if err := t.excludeElement(fileDescriptor, imageIndex, options); err != nil {
 			return err

private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go

@@ -142,6 +142,17 @@ func TestNesting(t *testing.T) {
 		t.Parallel()
 		runDiffTest(t, "testdata/nesting", "message.txtar", WithIncludeTypes("pkg.Foo"))
 	})
+	t.Run("recursive_message", func(t *testing.T) {
+		t.Parallel()
+		// "pkg.Foo.**" additionally pulls in the otherwise-unreferenced nested
+		// types pkg.Foo.NestedButNotUsed and pkg.Foo.NestedFoo.NestedNestedFoo.
+		runDiffTest(t, "testdata/nesting", "message.recursive.txtar", WithIncludeTypes("pkg.Foo.**"))
+	})
+	t.Run("recursive_message_with_exclude", func(t *testing.T) {
+		t.Parallel()
+		// An explicitly excluded nested type is skipped by the recursive glob.
+		runDiffTest(t, "testdata/nesting", "message.recursive-exclude.txtar", WithIncludeTypes("pkg.Foo.**"), WithExcludeTypes("pkg.Foo.NestedButNotUsed"))
+	})
 	t.Run("recursenested", func(t *testing.T) {
 		t.Parallel()
 		runDiffTest(t, "testdata/nesting", "recursenested.txtar", WithIncludeTypes("pkg.Foo.NestedFoo.NestedNestedFoo"))
@@ -321,6 +332,29 @@ func TestPackages(t *testing.T) {
 	runDiffTest(t, "testdata/packages", "foo.txtar", WithIncludeTypes("foo"))
 	runDiffTest(t, "testdata/packages", "foo.bar.txtar", WithIncludeTypes("foo.bar"))
 	runDiffTest(t, "testdata/packages", "foo.bar.baz.txtar", WithIncludeTypes("foo.bar.baz"))
+	// "foo.**" includes package foo and all of its sub-packages (foo.bar and
+	// foo.bar.baz), in contrast to "foo" which includes only package foo.
+	runDiffTest(t, "testdata/packages", "foo.recursive.txtar", WithIncludeTypes("foo.**"))
+	// Exclusion is always recursive even without an include: excluding "foo.bar"
+	// also drops its sub-package foo.bar.baz, while leaving package foo (and
+	// unrelated packages) intact.
+	runDiffTest(t, "testdata/packages", "foo.bar.exclude.txtar", WithExcludeTypes("foo.bar"))
+	// Excluding "foo.bar" is always recursive, so it also drops foo.bar.baz.
+	// The excluded sub-package is skipped by the "foo.**" include rather than
+	// being treated as an error.
+	runDiffTest(t, "testdata/packages", "foo.recursive-exclude.txtar", WithIncludeTypes("foo.**"), WithExcludeTypes("foo.bar"))
+
+	// Including a recursive glob while excluding the same top-level package
+	// cancel out, so this is an error just as including and excluding the same
+	// package directly would be.
+	t.Run("recursive_include_excludes_self", func(t *testing.T) {
+		t.Parallel()
+		ctx := t.Context()
+		_, image, err := getImage(ctx, slogtestext.NewLogger(t), "testdata/packages", bufimage.WithExcludeSourceCodeInfo())
+		require.NoError(t, err)
+		_, err = FilterImage(image, WithIncludeTypes("foo.bar.**"), WithExcludeTypes("foo.bar"))
+		require.ErrorContains(t, err, "inclusion of excluded package \"foo.bar\"")
+	})
 }
 
 func TestAny(t *testing.T) {

private/bufpkg/bufimage/bufimageutil/image_filter.go

@@ -35,18 +35,20 @@ func filterImage(image bufimage.Image, options *imageFilterOptions) (bufimage.Im
 		return nil, err
 	}
 	closure := newTransitiveClosure()
-	// All excludes are added first, then includes walk included all non excluded types.
-	// TODO: consider supporting a glob syntax of some kind, to do more advanced pattern
-	//   matching, such as ability to get a package AND all of its sub-packages.
+	// All excludes are added first, then includes walk all non-excluded types.
+	// For includes, a trailing ".**" is a recursive glob: it matches the named
+	// element and every symbol nested beneath it (e.g. a package and all its
+	// sub-packages, or a message and all its nested types). Exclusion is always
+	// recursive, so the suffix is simply stripped there.
 	for excludeType := range options.excludeTypes {
-		excludeType := protoreflect.FullName(excludeType)
-		if err := closure.excludeType(excludeType, imageIndex, options); err != nil {
+		excludeType, _ := strings.CutSuffix(excludeType, ".**")
+		if err := closure.excludeType(protoreflect.FullName(excludeType), imageIndex, options); err != nil {
 			return nil, err
 		}
 	}
 	for includeType := range options.includeTypes {
-		includeType := protoreflect.FullName(includeType)
-		if err := closure.includeType(includeType, imageIndex, options); err != nil {
+		includeType, recursive := strings.CutSuffix(includeType, ".**")
+		if err := closure.includeType(protoreflect.FullName(includeType), recursive, imageIndex, options); err != nil {
 			return nil, err
 		}
 	}

private/bufpkg/bufimage/bufimageutil/testdata/nesting/message.recursive-exclude.txtar

@@ -0,0 +1,13 @@
+-- a.proto --
+syntax = "proto3";
+package pkg;
+message Foo {
+  string x = 1;
+  NestedFoo nested_foo = 2;
+  message NestedFoo {
+    string nested_x = 1;
+    message NestedNestedFoo {
+      string nested_nested_x = 1;
+    }
+  }
+}

private/bufpkg/bufimage/bufimageutil/testdata/nesting/message.recursive.txtar

@@ -0,0 +1,16 @@
+-- a.proto --
+syntax = "proto3";
+package pkg;
+message Foo {
+  string x = 1;
+  NestedFoo nested_foo = 2;
+  message NestedButNotUsed {
+    string nested_but_not_used = 1;
+  }
+  message NestedFoo {
+    string nested_x = 1;
+    message NestedNestedFoo {
+      string nested_nested_x = 1;
+    }
+  }
+}