Skip to content

Commit

Permalink
Update bufcheck.RunnerProvider to use PluginConfig (#3328)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emcfarlane
emcfarlane authored Sep 18, 2024
1 parent 4e2a48f commit 411778b
Show file tree
Hide file tree
Showing 15 changed files with 43 additions and 64 deletions.
3 changes: 1 addition & 2 deletions private/buf/bufmigrate/migrator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/buf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/breaking/breaking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/config/internal/internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/lint/lint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/buf/command/mod/internal/internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/protoc-gen-buf-breaking/breaking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/buf/cmd/protoc-gen-buf-lint/lint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions private/bufpkg/bufcheck/breaking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 25 additions & 5 deletions private/bufpkg/bufcheck/bufcheck.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 5 additions & 9 deletions private/bufpkg/bufcheck/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions private/bufpkg/bufcheck/lint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion private/bufpkg/bufcheck/multi_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions private/bufpkg/bufcheck/multi_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
26 changes: 0 additions & 26 deletions private/pkg/pluginrpcutil/pluginrpcutil.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...,
)
},
)
}

0 comments on commit 411778b

Please sign in to comment.