Update bufcheck.RunnerProvider to use PluginConfig (#3328)

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
 	}

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)

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
 		}

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
 	}

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
 		}

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
 	}

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
 	}

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
 	}

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,

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.

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.

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,

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

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(

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