Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
doriable committed Dec 17, 2024
1 parent 94a2388 commit 226382b
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 28 deletions.
2 changes: 1 addition & 1 deletion private/buf/cmd/buf/command/breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func run(
}
breakingOptions := []bufcheck.BreakingOption{
bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
bufcheck.WithRelatedCheckConfigs(allCheckConfigs...),
}
if flags.ExcludeImports {
breakingOptions = append(breakingOptions, bufcheck.BreakingWithExcludeImports())
Expand Down
2 changes: 1 addition & 1 deletion private/buf/cmd/buf/command/config/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func lsRun(
}
configuredRuleOptions := []bufcheck.ConfiguredRulesOption{
bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
bufcheck.WithRelatedCheckConfigs(allCheckConfigs...),
}
rules, err = client.ConfiguredRules(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion private/buf/cmd/buf/command/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func run(
}
lintOptions := []bufcheck.LintOption{
bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...),
bufcheck.WithAdditionalCheckConfigs(allCheckConfigs...),
bufcheck.WithRelatedCheckConfigs(allCheckConfigs...),
}
if err := client.Lint(
ctx,
Expand Down
13 changes: 7 additions & 6 deletions private/bufpkg/bufcheck/bufcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,15 @@ type LintBreakingAndConfiguredRulesOption interface {
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.
// WithRelatedCheckConfigs returns a new LintBreakingAndConfiguredRulesOption that allows
// the caller to provide additional related check configs. This allows the client to check
// for unused plugins across related check configs and provide users with a warning if the
// plugin is unused in all check configs.
//
// 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,
func WithRelatedCheckConfigs(relatedCheckConfigs ...bufconfig.CheckConfig) LintBreakingAndConfiguredRulesOption {
return &relatedCheckConfigsOption{
relatedCheckConfigs: relatedCheckConfigs,
}
}

Expand Down
36 changes: 18 additions & 18 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, lintOptions.additionalCheckConfigs)
config, err := configForLintConfig(lintConfig, allRules, allCategories, lintOptions.relatedCheckConfigs)
if err != nil {
return err
}
Expand Down Expand Up @@ -168,7 +168,7 @@ func (c *client) Breaking(
allRules,
allCategories,
breakingOptions.excludeImports,
breakingOptions.additionalCheckConfigs,
breakingOptions.relatedCheckConfigs,
)
if err != nil {
return err
Expand Down Expand Up @@ -230,7 +230,7 @@ func (c *client) ConfiguredRules(
if err != nil {
return nil, err
}
rulesConfig, err := rulesConfigForCheckConfig(checkConfig, allRules, allCategories, ruleType, configuredRulesOptions.additionalCheckConfigs)
rulesConfig, err := rulesConfigForCheckConfig(checkConfig, allRules, allCategories, ruleType, configuredRulesOptions.relatedCheckConfigs)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -477,27 +477,27 @@ func checkCommentLineForCheckIgnore(
}

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

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

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

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

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

func newConfiguredRulesOptions() *configuredRulesOptions {
Expand Down Expand Up @@ -558,18 +558,18 @@ func (p *pluginConfigsOption) applyToAllCategories(allCategoriesOptions *allCate
allCategoriesOptions.pluginConfigs = append(allCategoriesOptions.pluginConfigs, p.pluginConfigs...)
}

type additionalCheckConfigsOption struct {
additionalCheckConfigs []bufconfig.CheckConfig
type relatedCheckConfigsOption struct {
relatedCheckConfigs []bufconfig.CheckConfig
}

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

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

func (a *additionalCheckConfigsOption) applyToConfiguredRules(configuredRulesOptions *configuredRulesOptions) {
configuredRulesOptions.additionalCheckConfigs = append(configuredRulesOptions.additionalCheckConfigs, a.additionalCheckConfigs...)
func (r *relatedCheckConfigsOption) applyToConfiguredRules(configuredRulesOptions *configuredRulesOptions) {
configuredRulesOptions.relatedCheckConfigs = append(configuredRulesOptions.relatedCheckConfigs, r.relatedCheckConfigs...)
}
3 changes: 2 additions & 1 deletion private/bufpkg/bufcheck/rules_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ func newRulesConfig(
delete(unusedPluginNameToRuleIDs, pluginName)
}
}
// We check additional check configs. If rules
// We check additional check configs. If rules are set in the related check configs, then
// the plugin is not considered unused.
for _, checkConfig := range additionalCheckConfigs {
checkConfigUseRuleIDs, err := transformRuleOrCategoryIDsToRuleIDs(
checkConfig.UseIDsAndCategories(),
Expand Down

0 comments on commit 226382b

Please sign in to comment.