diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index a0b554972f..cf711e5d01 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -27,6 +27,7 @@ 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" @@ -712,7 +713,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, runner) + client, err := bufcheck.NewClient(logger, tracer, pluginrpcutil.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 5af6d05c96..464ef032fc 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -39,6 +39,7 @@ 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" @@ -1349,7 +1350,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, command.NewRunner()) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.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 57b91a04c1..97e60339d7 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -28,6 +28,7 @@ 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" @@ -209,7 +210,7 @@ func run( tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, command.NewRunner(), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 9bc0a25ad3..a3ec285c1d 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -27,6 +27,7 @@ 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" @@ -185,7 +186,7 @@ func lsRun( } } tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, command.NewRunner(), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 6411b9fd69..fb117a0bcf 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -26,6 +26,7 @@ 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" @@ -134,7 +135,7 @@ func run( tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, command.NewRunner(), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 124e5936b0..62a084b887 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -26,6 +26,7 @@ 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" @@ -175,7 +176,7 @@ func lsRun( } // BufYAMLFiles <=v1 never had plugins. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, command.NewRunner(), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 1162d96bfc..dcd19521c9 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -30,6 +30,7 @@ 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/tracing" "github.com/bufbuild/protoplugin" @@ -117,7 +118,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, command.NewRunner(), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 f4ccc9d4b5..41bb9d099b 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -29,6 +29,7 @@ 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/tracing" "github.com/bufbuild/protoplugin" @@ -92,7 +93,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, command.NewRunner(), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient(container.Logger(), tracer, pluginrpcutil.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 050e5a7488..0df6950dd9 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -31,6 +31,7 @@ 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" @@ -1347,7 +1348,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, command.NewRunner()) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.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 13c228ad1d..7182da33e3 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -20,12 +20,12 @@ import ( "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/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/bufbuild/bufplugin-go/check" "go.uber.org/zap" + "pluginrpc.com/pluginrpc" ) // Rules are returned sorted by ID, but PrintRules does our sort by category. @@ -162,14 +162,27 @@ func WithPluginsEnabled() ClientFunctionOption { return pluginsEnabledOption{} } +// 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...) +} + // NewClient returns a new Client. func NewClient( logger *zap.Logger, tracer tracing.Tracer, - runner command.Runner, + runnerProvider RunnerProvider, options ...ClientOption, ) (Client, error) { - return newClient(logger, tracer, runner, options...) + return newClient(logger, tracer, runnerProvider, options...) } // ClientOption is an option for a new Client. diff --git a/private/bufpkg/bufcheck/client.go b/private/bufpkg/bufcheck/client.go index d6b77773c7..6cc3d2924d 100644 --- a/private/bufpkg/bufcheck/client.go +++ b/private/bufpkg/bufcheck/client.go @@ -24,9 +24,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck/bufcheckserver" "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/normalpath" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/protosourcepath" "github.com/bufbuild/buf/private/pkg/protoversion" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -41,7 +39,7 @@ import ( type client struct { logger *zap.Logger tracer tracing.Tracer - runner command.Runner + runnerProvider RunnerProvider stderr io.Writer fileVersionToDefaultCheckClient map[bufconfig.FileVersion]check.Client } @@ -49,7 +47,7 @@ type client struct { func newClient( logger *zap.Logger, tracer tracing.Tracer, - runner command.Runner, + runnerProvider RunnerProvider, options ...ClientOption, ) (*client, error) { clientOptions := newClientOptions() @@ -71,10 +69,10 @@ func newClient( } return &client{ - logger: logger, - tracer: tracer, - runner: runner, - stderr: clientOptions.stderr, + logger: logger, + tracer: tracer, + runnerProvider: runnerProvider, + stderr: clientOptions.stderr, fileVersionToDefaultCheckClient: map[bufconfig.FileVersion]check.Client{ bufconfig.FileVersionV1Beta1: v1beta1DefaultCheckClient, bufconfig.FileVersionV1: v1DefaultCheckClient, @@ -343,11 +341,10 @@ func (c *client) getMultiClient( pluginPath := pluginConfig.Path() checkClient := check.NewClient( pluginrpc.NewClient( - pluginrpcutil.NewRunner( - c.runner, + c.runnerProvider.NewRunner( // We know that Path is of at least length 1. pluginPath[0], - pluginrpcutil.RunnerWithArgs(pluginPath[1:]...), + pluginPath[1:]..., ), pluginrpc.ClientWithStderr(c.stderr), // We have to set binary as some things cannot be encoded as JSON. diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index e08fb0a1f7..4fb9c98129 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -28,6 +28,7 @@ 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" @@ -1285,7 +1286,7 @@ func testLintWithOptions( lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID) require.NotNil(t, lintConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, command.NewRunner()) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, pluginrpcutil.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) err = client.Lint( ctx, diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index f36ee5cfd3..a0e10eaad7 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -21,6 +21,7 @@ import ( "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" @@ -184,7 +185,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - command.NewRunner(), + pluginrpcutil.NewRunnerProvider(command.NewRunner()), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -278,7 +279,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - command.NewRunner(), + pluginrpcutil.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 d452f73108..e533e8509a 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -20,17 +20,32 @@ import ( ) // NewRunner returns a new pluginrpc.Runner for the command.Runner and program name. -func NewRunner(delegate command.Runner, programName string, options ...RunnerOption) pluginrpc.Runner { - return newRunner(delegate, programName, options...) +func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner { + return newRunner(delegate, programName, programArgs...) } -// RunnerOption is an option for a new Runner. -type RunnerOption func(*runnerOptions) +// 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...) +} -// RunnerWithArgs returns a new RunnerOption that specifies a sub-command to invoke -// on the program. -func RunnerWithArgs(args ...string) RunnerOption { - return func(runnerOptions *runnerOptions) { - runnerOptions.args = args - } +// 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..., + ) + }, + ) } diff --git a/private/pkg/pluginrpcutil/runner.go b/private/pkg/pluginrpcutil/runner.go index 8bc3218bfe..af29b6af70 100644 --- a/private/pkg/pluginrpcutil/runner.go +++ b/private/pkg/pluginrpcutil/runner.go @@ -27,29 +27,25 @@ import ( type runner struct { delegate command.Runner programName string - args []string + programArgs []string } func newRunner( delegate command.Runner, programName string, - options ...RunnerOption, + programArgs ...string, ) *runner { - runnerOptions := newRunnerOptions() - for _, option := range options { - option(runnerOptions) - } return &runner{ delegate: delegate, programName: programName, - args: runnerOptions.args, + programArgs: programArgs, } } func (r *runner) Run(ctx context.Context, env pluginrpc.Env) error { args := env.Args - if len(r.args) > 0 { - args = append(slices.Clone(r.args), env.Args...) + if len(r.programArgs) > 0 { + args = append(slices.Clone(r.programArgs), env.Args...) } if err := r.delegate.Run( ctx, @@ -67,11 +63,3 @@ func (r *runner) Run(ctx context.Context, env pluginrpc.Env) error { } return nil } - -type runnerOptions struct { - args []string -} - -func newRunnerOptions() *runnerOptions { - return &runnerOptions{} -}