Skip to content

Commit

Permalink
Add WithAdditionalCheckConfigs option to check additional check con…
Browse files Browse the repository at this point in the history
…figs for unused plugins warning
  • Loading branch information
doriable committed Dec 16, 2024
1 parent 0e644f7 commit 3637798
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 8 deletions.
8 changes: 8 additions & 0 deletions private/buf/cmd/buf/command/breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/bufbuild/buf/private/buf/buffetch"
"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
"github.com/bufbuild/buf/private/bufpkg/bufcheck"
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
"github.com/bufbuild/buf/private/bufpkg/bufimage"
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
Expand Down Expand Up @@ -216,6 +217,12 @@ func run(
defer func() {
retErr = errors.Join(retErr, wasmRuntime.Close(ctx))
}()
allCheckConfigs := slicesext.Map(
imageWithConfigs,
func(imageWithConfig bufctl.ImageWithConfig) bufconfig.CheckConfig {
return imageWithConfig.BreakingConfig()
},
)
var allFileAnnotations []bufanalysis.FileAnnotation
for i, imageWithConfig := range imageWithConfigs {
client, err := bufcheck.NewClient(
Expand All @@ -228,6 +235,7 @@ func run(
}
breakingOptions := []bufcheck.BreakingOption{
bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
}
if flags.ExcludeImports {
breakingOptions = append(breakingOptions, bufcheck.BreakingWithExcludeImports())
Expand Down
5 changes: 5 additions & 0 deletions private/buf/cmd/buf/command/config/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ func lsRun(
var rules []bufcheck.Rule
if flags.ConfiguredOnly {
moduleConfigs := bufYAMLFile.ModuleConfigs()
allCheckConfigs := append(
slicesext.Map(moduleConfigs, func(moduleConfig bufconfig.ModuleConfig) bufconfig.CheckConfig { return moduleConfig.LintConfig() }),
slicesext.Map(moduleConfigs, func(moduleConfig bufconfig.ModuleConfig) bufconfig.CheckConfig { return moduleConfig.BreakingConfig() })...,
)
var moduleConfig bufconfig.ModuleConfig
switch fileVersion := bufYAMLFile.FileVersion(); fileVersion {
case bufconfig.FileVersionV1Beta1, bufconfig.FileVersionV1:
Expand Down Expand Up @@ -242,6 +246,7 @@ func lsRun(
}
configuredRuleOptions := []bufcheck.ConfiguredRulesOption{
bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
}
rules, err = client.ConfiguredRules(
ctx,
Expand Down
9 changes: 9 additions & 0 deletions private/buf/cmd/buf/command/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
"github.com/bufbuild/buf/private/buf/bufctl"
"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
"github.com/bufbuild/buf/private/bufpkg/bufcheck"
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/wasm"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -142,6 +144,12 @@ func run(
retErr = errors.Join(retErr, wasmRuntime.Close(ctx))
}()
var allFileAnnotations []bufanalysis.FileAnnotation
allCheckConfigs := slicesext.Map(
imageWithConfigs,
func(imageWithConfig bufctl.ImageWithConfig) bufconfig.CheckConfig {
return imageWithConfig.LintConfig()
},
)
for _, imageWithConfig := range imageWithConfigs {
client, err := bufcheck.NewClient(
container.Logger(),
Expand All @@ -153,6 +161,7 @@ func run(
}
lintOptions := []bufcheck.LintOption{
bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
}
if err := client.Lint(
ctx,
Expand Down
18 changes: 18 additions & 0 deletions private/bufpkg/bufcheck/bufcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,24 @@ type ConfiguredRulesOption interface {
applyToConfiguredRules(*configuredRulesOptions)
}

// LintBreakingAndConfiguredRulesOption is an option for Lint, Breaking, and ConfiguredRules.
type LintBreakingAndConfiguredRulesOption interface {
LintOption
BreakingOption
ConfiguredRulesOption
}

// WithAdditionalCheckConfigs returns a new LintBreakingAndConfiguredRulesOption that allows
// the caller to provide additional check configs that are being run. This allows the client
// to check for unused plugins across additional check configs and provide users with a warning.
//
// The default is to only check the configs provided to the client for Lint, Breaking, or ConfiguredRules.
func WithAdditionalCheckConfigs(additionalCheckConfigs ...bufconfig.CheckConfig) LintBreakingAndConfiguredRulesOption {
return &additionalCheckConfigsOption{
additionalCheckConfigs: additionalCheckConfigs,
}
}

// AllRulesOption is an option for AllRules.
type AllRulesOption interface {
applyToAllRules(*allRulesOptions)
Expand Down
32 changes: 26 additions & 6 deletions private/bufpkg/bufcheck/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *client) Lint(
if err != nil {
return err
}
config, err := configForLintConfig(lintConfig, allRules, allCategories)
config, err := configForLintConfig(lintConfig, allRules, allCategories, lintOptions.additionalCheckConfigs)
if err != nil {
return err
}
Expand Down Expand Up @@ -168,6 +168,7 @@ func (c *client) Breaking(
allRules,
allCategories,
breakingOptions.excludeImports,
breakingOptions.additionalCheckConfigs,
)
if err != nil {
return err
Expand Down Expand Up @@ -229,7 +230,7 @@ func (c *client) ConfiguredRules(
if err != nil {
return nil, err
}
rulesConfig, err := rulesConfigForCheckConfig(checkConfig, allRules, allCategories, ruleType)
rulesConfig, err := rulesConfigForCheckConfig(checkConfig, allRules, allCategories, ruleType, configuredRulesOptions.additionalCheckConfigs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -476,24 +477,27 @@ func checkCommentLineForCheckIgnore(
}

type lintOptions struct {
pluginConfigs []bufconfig.PluginConfig
pluginConfigs []bufconfig.PluginConfig
additionalCheckConfigs []bufconfig.CheckConfig
}

func newLintOptions() *lintOptions {
return &lintOptions{}
}

type breakingOptions struct {
pluginConfigs []bufconfig.PluginConfig
excludeImports bool
pluginConfigs []bufconfig.PluginConfig
excludeImports bool
additionalCheckConfigs []bufconfig.CheckConfig
}

func newBreakingOptions() *breakingOptions {
return &breakingOptions{}
}

type configuredRulesOptions struct {
pluginConfigs []bufconfig.PluginConfig
pluginConfigs []bufconfig.PluginConfig
additionalCheckConfigs []bufconfig.CheckConfig
}

func newConfiguredRulesOptions() *configuredRulesOptions {
Expand Down Expand Up @@ -553,3 +557,19 @@ func (p *pluginConfigsOption) applyToAllRules(allRulesOptions *allRulesOptions)
func (p *pluginConfigsOption) applyToAllCategories(allCategoriesOptions *allCategoriesOptions) {
allCategoriesOptions.pluginConfigs = append(allCategoriesOptions.pluginConfigs, p.pluginConfigs...)
}

type additionalCheckConfigsOption struct {
additionalCheckConfigs []bufconfig.CheckConfig
}

func (a *additionalCheckConfigsOption) applyToLint(lintOptions *lintOptions) {
lintOptions.additionalCheckConfigs = append(lintOptions.additionalCheckConfigs, a.additionalCheckConfigs...)
}

func (a *additionalCheckConfigsOption) applyToBreaking(breakingOptions *breakingOptions) {
breakingOptions.additionalCheckConfigs = append(breakingOptions.additionalCheckConfigs, a.additionalCheckConfigs...)
}

func (a *additionalCheckConfigsOption) applyToConfiguredRules(configuredRulesOptions *configuredRulesOptions) {
configuredRulesOptions.additionalCheckConfigs = append(configuredRulesOptions.additionalCheckConfigs, a.additionalCheckConfigs...)
}
6 changes: 4 additions & 2 deletions private/bufpkg/bufcheck/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ func configForLintConfig(
lintConfig bufconfig.LintConfig,
allRules []Rule,
allCategories []Category,
additionalCheckConfigs []bufconfig.CheckConfig,
) (*config, error) {
rulesConfig, err := rulesConfigForCheckConfig(lintConfig, allRules, allCategories, check.RuleTypeLint)
rulesConfig, err := rulesConfigForCheckConfig(lintConfig, allRules, allCategories, check.RuleTypeLint, additionalCheckConfigs)
if err != nil {
return nil, err
}
Expand All @@ -48,8 +49,9 @@ func configForBreakingConfig(
allRules []Rule,
allCategories []Category,
excludeImports bool,
additionalCheckConfigs []bufconfig.CheckConfig,
) (*config, error) {
rulesConfig, err := rulesConfigForCheckConfig(breakingConfig, allRules, allCategories, check.RuleTypeBreaking)
rulesConfig, err := rulesConfigForCheckConfig(breakingConfig, allRules, allCategories, check.RuleTypeBreaking, additionalCheckConfigs)
if err != nil {
return nil, err
}
Expand Down
25 changes: 25 additions & 0 deletions private/bufpkg/bufcheck/rules_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func rulesConfigForCheckConfig(
allRules []Rule,
allCategories []Category,
ruleType check.RuleType,
additionalCheckConfigs []bufconfig.CheckConfig,
) (*rulesConfig, error) {
return newRulesConfig(
checkConfig.UseIDsAndCategories(),
Expand All @@ -42,6 +43,7 @@ func rulesConfigForCheckConfig(
allRules,
allCategories,
ruleType,
additionalCheckConfigs,
)
}

Expand Down Expand Up @@ -102,6 +104,10 @@ type rulesConfig struct {
// Rules, we don't warn for this plugin, as it may have had rules configured in breaking that
// we haven't accounted for.
//
// The caller can provider additional check configs to check if the plugin has rules configured.
// If the plugin has rules configured in any of the additional check configs provided, then
// we don't warn.
//
// The Rule IDs will be sorted.
// This will only contain RuleIDs of the given RuleType.
// There will be no empty key for plugin name (which means the Rule is builtin), that is
Expand All @@ -124,6 +130,7 @@ func newRulesConfig(
allRules []Rule,
allCategories []Category,
ruleType check.RuleType,
additionalCheckConfigs []bufconfig.CheckConfig,
) (*rulesConfig, error) {
allRulesForType := rulesForType(allRules, ruleType)
if len(allRulesForType) == 0 {
Expand Down Expand Up @@ -312,6 +319,24 @@ func newRulesConfig(
delete(unusedPluginNameToRuleIDs, pluginName)
}
}
// We check additional check configs. If rules
for _, checkConfig := range additionalCheckConfigs {
checkConfigUseRuleIDs, err := transformRuleOrCategoryIDsToRuleIDs(
checkConfig.UseIDsAndCategories(),
ruleIDToCategoryIDs,
categoryIDToRuleIDs,
)
if err != nil {
return nil, err
}
for _, checkConfigRuleID := range checkConfigUseRuleIDs {
// We do not validate the additional check config rules here, only check for those
// that are found.
if rule, ok := ruleIDToRule[checkConfigRuleID]; ok {
delete(unusedPluginNameToRuleIDs, rule.PluginName())
}
}
}

return &rulesConfig{
RuleType: ruleType,
Expand Down

0 comments on commit 3637798

Please sign in to comment.