diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index afe251297c..586ed1c6a3 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storagemem" @@ -713,7 +712,7 @@ func equivalentCheckConfigInV2( ) (bufconfig.CheckConfig, error) { // No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1. // TODO: If we ever need v3, then we will have to deal with this. - client, err := bufcheck.NewClient(logger, tracer, pluginrpcutil.NewRunnerProvider(runner)) + client, err := bufcheck.NewClient(logger, tracer, bufcheck.NewRunnerProvider(runner)) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index a2e155825b..504ef6d2e7 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -40,7 +40,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/osext" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/storage/storagetesting" @@ -1350,7 +1349,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) { t.Run(version.String(), func(t *testing.T) { t.Parallel() // Do not need any custom lint/breaking plugins here. - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) require.NoError(t, err) diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 97e60339d7..d84dbe2f86 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" @@ -210,7 +209,7 @@ func run( tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index ba0e0db520..2ac56e61f6 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/syserror" @@ -186,7 +185,7 @@ func lsRun( } } tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index fb117a0bcf..0f871426d9 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/spf13/pflag" @@ -135,7 +134,7 @@ func run( tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index 64a777c60a..577a5c9e43 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/syserror" @@ -176,7 +175,7 @@ func lsRun( } // BufYAMLFiles <=v1 never had plugins. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) if err != nil { return err } diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index c0ee1420b8..ec6f1d4bdf 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -30,7 +30,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" @@ -126,7 +125,7 @@ func handle( } // The protoc plugins do not support custom lint/breaking change plugins for now. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) if err != nil { return err } diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 37bdd9d1f0..8f86b0ba47 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -29,7 +29,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" @@ -101,7 +100,7 @@ func handle( } // The protoc plugins do not support custom lint/breaking change plugins for now. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) if err != nil { return err } diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index 58231d569c..bbc3abee61 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -31,7 +31,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/stretchr/testify/assert" @@ -1348,7 +1347,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 7e418a61e1..7b17d4e573 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -21,6 +21,8 @@ import ( "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" @@ -162,17 +164,35 @@ func WithPluginsEnabled() ClientFunctionOption { return pluginsEnabledOption{} } -// RunnerProvider provides pluginrpc.Runners for program names and args. +// RunnerProvider provides pluginrpc.Runners for a given plugin config. type RunnerProvider interface { - NewRunner(programName string, programArgs ...string) pluginrpc.Runner + NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) } // RunnerProviderFunc is a function that implements RunnerProvider. -type RunnerProviderFunc func(programName string, programArgs ...string) pluginrpc.Runner +type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) // NewRunner implements RunnerProvider. -func (r RunnerProviderFunc) NewRunner(programName string, programArgs ...string) pluginrpc.Runner { - return r(programName, programArgs...) +func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { + return r(pluginConfig) +} + +// NewRunnerProvider returns a new RunnerProvider for the command.Runner. +func NewRunnerProvider(delegate command.Runner) RunnerProvider { + return RunnerProviderFunc( + func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { + if pluginConfig.Type() != bufconfig.PluginConfigTypeLocal { + return nil, syserror.New("only local plugins are supported") + } + path := pluginConfig.Path() + return pluginrpcutil.NewRunner( + delegate, + // We know that Path is of at least length 1. + path[0], + path[1:]..., + ), nil + }, + ) } // NewClient returns a new Client. diff --git a/private/bufpkg/bufcheck/client.go b/private/bufpkg/bufcheck/client.go index 361ac9aa43..f0fce83e44 100644 --- a/private/bufpkg/bufcheck/client.go +++ b/private/bufpkg/bufcheck/client.go @@ -331,21 +331,17 @@ func (c *client) getMultiClient( ) } for _, pluginConfig := range pluginConfigs { - if pluginConfig.Type() != bufconfig.PluginConfigTypeLocal { - return nil, syserror.New("we only handle local plugins for now with lint and breaking") - } options, err := check.NewOptions(pluginConfig.Options()) if err != nil { return nil, fmt.Errorf("could not parse options for plugin %q: %w", pluginConfig.Name(), err) } - pluginPath := pluginConfig.Path() + runner, err := c.runnerProvider.NewRunner(pluginConfig) + if err != nil { + return nil, fmt.Errorf("could not create runner for plugin %q: %w", pluginConfig.Name(), err) + } checkClient := check.NewClient( pluginrpc.NewClient( - c.runnerProvider.NewRunner( - // We know that Path is of at least length 1. - pluginPath[0], - pluginPath[1:]..., - ), + runner, pluginrpc.ClientWithStderr(c.stderr), // We have to set binary as some things cannot be encoded as JSON. // Example: google.protobuf.Timestamps with positive seconds and negative nanos. diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index e00d4f0609..07bb5ce86e 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/stretchr/testify/assert" @@ -1294,7 +1293,7 @@ func testLintWithOptions( lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID) require.NotNil(t, lintConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) err = client.Lint( ctx, diff --git a/private/bufpkg/bufcheck/multi_client.go b/private/bufpkg/bufcheck/multi_client.go index e755ad140e..52ed43403b 100644 --- a/private/bufpkg/bufcheck/multi_client.go +++ b/private/bufpkg/bufcheck/multi_client.go @@ -101,13 +101,13 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno } return fmt.Errorf("plugin %q failed: %w", delegate.PluginName, err) } - lock.Lock() annotations := slicesext.Map( delegateResponse.Annotations(), func(checkAnnotation check.Annotation) *annotation { return newAnnotation(checkAnnotation, delegate.PluginName) }, ) + lock.Lock() allAnnotations = append(allAnnotations, annotations...) lock.Unlock() return nil diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index c81c8da7e9..dadd64d134 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,7 +24,6 @@ import ( "buf.build/go/bufplugin/check/checkutil" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" @@ -185,7 +184,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - pluginrpcutil.NewRunnerProvider(command.NewRunner()), + NewRunnerProvider(command.NewRunner()), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -279,7 +278,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - pluginrpcutil.NewRunnerProvider(command.NewRunner()), + NewRunnerProvider(command.NewRunner()), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go index e533e8509a..f984f8b8a9 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -23,29 +23,3 @@ import ( func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner { return newRunner(delegate, programName, programArgs...) } - -// RunnerProvider provides pluginrpc.Runners for program names and args. -type RunnerProvider interface { - NewRunner(programName string, programArgs ...string) pluginrpc.Runner -} - -// RunnerProviderFunc is a function that implements RunnerProvider. -type RunnerProviderFunc func(programName string, programArgs ...string) pluginrpc.Runner - -// NewRunner implements RunnerProvider. -func (r RunnerProviderFunc) NewRunner(programName string, programArgs ...string) pluginrpc.Runner { - return r(programName, programArgs...) -} - -// NewRunnerProvider returns a new RunnerProvider for the command.Runner. -func NewRunnerProvider(delegate command.Runner) RunnerProvider { - return RunnerProviderFunc( - func(programName string, programArgs ...string) pluginrpc.Runner { - return NewRunner( - delegate, - programName, - programArgs..., - ) - }, - ) -}