compiler/protogen: properly filter option dependencies in go-protobuf plugin.

Because protoc sends option dependencies to its plugins, this results in the go plugin enforcing the import path flag requirements on them.  This is unnecessary though, since a true option dependency will have no generated code importing it anyway.  This is especially problematic in (but not unique to) Bazel, since the aspect doesn't trigger on the option_deps attributes.

Change-Id: I3b8cec00e462f84ef02ba1ab6bdb4dbe843703d5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/713342
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: mkruskal-google

cmd/protoc-gen-go/descriptor_test/descriptor_test.go

@@ -8,8 +8,13 @@ import (
 	"testing"
 
 	testopenpb "google.golang.org/protobuf/internal/testprotos/test"
+	testnopackagepb "google.golang.org/protobuf/internal/testprotos/test/test_nopackage"
+	testoptionpb "google.golang.org/protobuf/internal/testprotos/test/test_option"
 	testhybridpb "google.golang.org/protobuf/internal/testprotos/testeditions/testeditions_hybrid"
 	testopaquepb "google.golang.org/protobuf/internal/testprotos/testeditions/testeditions_opaque"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/descriptorpb"
+	"google.golang.org/protobuf/types/gofeaturespb"
 )
 
 func TestFileModeEnum(t *testing.T) {
@@ -41,3 +46,17 @@ func TestFileModeMessage(t *testing.T) {
 		t.Errorf("Hybrid proto did have deprecated method EnumDescriptor")
 	}
 }
+
+func TestImportOption(t *testing.T) {
+	m := &testoptionpb.OptionImportMessage{}
+	mdesc := m.ProtoReflect().Descriptor()
+
+	fileopts := mdesc.ParentFile().Options().(*descriptorpb.FileOptions)
+	if proto.GetExtension(fileopts.Features, gofeaturespb.E_Go).(*gofeaturespb.GoFeatures).GetApiLevel() != gofeaturespb.GoFeatures_API_OPAQUE {
+		t.Errorf("OptionImportMessage parent file options features does not have API_OPAQUE")
+	}
+
+	if proto.GetExtension(fileopts, testnopackagepb.E_NoPackageOption).(*testnopackagepb.NoPackageOption).GetName() != "no package option" {
+		t.Errorf("OptionImportMessage parent file options does not have no_package_option")
+	}
+}

compiler/protogen/protogen.go

@@ -267,6 +267,12 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
 		return nil, fmt.Errorf("cannot use module= with paths=source_relative")
 	}
 
+	// Instead of generating each gen.Request.ProtoFile,
+	// generate gen.Request.FileToGenerate and its transitive dependencies.
+	//
+	// This effectively filters out 'import option' dependencies.
+	files := gatherTransitiveDependencies(gen.Request)
+
 	// Figure out the import path and package name for each file.
 	//
 	// The rules here are complicated and have grown organically over time.
@@ -281,7 +287,7 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
 	//
 	// Alternatively, build systems which want to exert full control over
 	// import paths may specify M<filename>=<import_path> flags.
-	for _, fdesc := range gen.Request.ProtoFile {
+	for _, fdesc := range files {
 		filename := fdesc.GetName()
 		// The "M" command-line flags take precedence over
 		// the "go_package" option in the .proto source file.
@@ -351,7 +357,7 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
 
 	// The extracted types from the full import set
 	typeRegistry := newExtensionRegistry()
-	for _, fdesc := range gen.Request.ProtoFile {
+	for _, fdesc := range files {
 		filename := fdesc.GetName()
 		if gen.FilesByPath[filename] != nil {
 			return nil, fmt.Errorf("duplicate file name: %q", filename)
@@ -390,6 +396,50 @@ func (opts Options) New(req *pluginpb.CodeGeneratorRequest) (*Plugin, error) {
 	return gen, nil
 }
 
+type transitiveDependencies struct {
+	files      map[string]*descriptorpb.FileDescriptorProto
+	deps       map[string]bool
+	sortedDeps []*descriptorpb.FileDescriptorProto
+}
+
+func newTransitiveDependencies(req *pluginpb.CodeGeneratorRequest) *transitiveDependencies {
+	files := make(map[string]*descriptorpb.FileDescriptorProto)
+	for _, f := range req.GetProtoFile() {
+		files[f.GetName()] = f
+	}
+	return &transitiveDependencies{
+		files: files,
+		deps:  make(map[string]bool),
+	}
+}
+
+func (td *transitiveDependencies) add(name string) {
+	if td.deps[name] {
+		return
+	}
+	f := td.files[name]
+	if f == nil {
+		// This shouldn't happen, but will fail later if it does.
+		return
+	}
+	td.deps[name] = true
+	for _, dep := range f.GetDependency() {
+		td.add(dep)
+	}
+	td.sortedDeps = append(td.sortedDeps, f)
+}
+
+func gatherTransitiveDependencies(req *pluginpb.CodeGeneratorRequest) []*descriptorpb.FileDescriptorProto {
+	if len(req.GetFileToGenerate()) == 0 {
+		return req.GetProtoFile()
+	}
+	td := newTransitiveDependencies(req)
+	for _, f := range req.GetFileToGenerate() {
+		td.add(f)
+	}
+	return td.sortedDeps
+}
+
 // InternalStripForEditionsDiff returns whether or not to strip non-functional
 // codegen for editions diff testing.
 //

internal/cmd/generate-protos/main.go

@@ -345,7 +345,10 @@ func generateLocalProtos() {
 		},
 		annotate: map[string]bool{"cmd/protoc-gen-go/testdata/annotations/annotations.proto": true},
 	}, {
-		path:    "internal/testprotos",
+		path: "internal/testprotos",
+		pkgPaths: map[string]string{
+			"internal/testprotos/test/test_nopackage.proto": "google.golang.org/protobuf/internal/testprotos/test/test_nopackage",
+		},
 		exclude: map[string]bool{"internal/testprotos/irregular/irregular.proto": true},
 	}, {
 		path: "src/",

internal/testprotos/test/test_nopackage.proto

@@ -0,0 +1,20 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+edition = "2024";
+
+package goproto.proto.test;
+
+import "google/protobuf/descriptor.proto";
+
+// Intentionally leave out go_package options to make this hostile as a
+// transitive dependency.  It should still work well as an option dependency.
+
+message NoPackageOption {
+  string name = 1;
+}
+
+extend google.protobuf.FileOptions {
+  NoPackageOption no_package_option = 123456789;
+}

internal/testprotos/test/test_nopackage/test_nopackage.pb.go

@@ -0,0 +1,18 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+edition = "2024";
+
+package goproto.proto.test;
+
+import option "internal/testprotos/test/test_nopackage.proto";
+import option "google/protobuf/go_features.proto";
+
+option go_package = "google.golang.org/protobuf/internal/testprotos/test/test_option";
+option features.(pb.go).api_level = API_OPAQUE;
+option (no_package_option).name = "no package option";
+
+message OptionImportMessage {
+  string name = 1;
+}