Change bufcheck.NewClient to take a RunnerProvider instead of a comma…

…nd.Runner (#3294)

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
 	}

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)

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
 		}

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
 	}

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
 		}

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
 	}

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
 	}

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
 	}

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,

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.

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,15 +39,15 @@ import (
 type client struct {
 	logger                          *zap.Logger
 	tracer                          tracing.Tracer
-	runner                          command.Runner
+	runnerProvider                  RunnerProvider
 	stderr                          io.Writer
 	fileVersionToDefaultCheckClient map[bufconfig.FileVersion]check.Client
 }
 
 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.

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,

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(

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...,
+			)
+		},
+	)
 }

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{}
-}