Silence config warning on empty rules when policies are defined (#3972)

Author: emcfarlane

private/bufpkg/bufcheck/client.go

@@ -124,6 +124,7 @@ func (c *client) Lint(
 		lintOptions.pluginConfigs,
 		nil, // policyConfig.
 		lintOptions.relatedCheckConfigs,
+		len(lintOptions.policyConfigs) > 0, // hasPolicyConfigs.
 	)
 	if err != nil {
 		return err
@@ -150,7 +151,8 @@ func (c *client) Lint(
 			policyLintConfig,
 			pluginConfigs,
 			policyConfig,
-			nil, // relatedCheckConfigs.
+			nil,  // relatedCheckConfigs.
+			true, // hasPolicyConfigs.
 		)
 		if err != nil {
 			return err
@@ -177,6 +179,7 @@ func (c *client) lint(
 	pluginConfigs []bufconfig.PluginConfig,
 	policyConfig bufconfig.PolicyConfig,
 	relatedCheckConfigs []bufconfig.CheckConfig,
+	hasPolicyConfigs bool,
 ) ([]*annotation, error) {
 	if lintConfig.Disabled() {
 		return nil, nil
@@ -195,7 +198,11 @@ func (c *client) lint(
 	if err != nil {
 		return nil, err
 	}
-	logRulesConfig(c.logger, config.rulesConfig)
+	configName := bufconfig.DefaultBufYAMLFileName
+	if policyConfig != nil {
+		configName = policyConfig.Name()
+	}
+	logRulesConfig(c.logger, configName, config.rulesConfig, hasPolicyConfigs)
 	files, err := descriptor.FileDescriptorsForProtoFileDescriptors(imageToProtoFileDescriptors(image))
 	if err != nil {
 		// An Image may be invalid if it does not contain all of the required dependencies.
@@ -254,6 +261,7 @@ func (c *client) Breaking(
 		nil, // policyConfig.
 		breakingOptions.excludeImports,
 		breakingOptions.relatedCheckConfigs,
+		len(breakingOptions.policyConfigs) > 0, // hasPolicyConfigs.
 	)
 	if err != nil {
 		return err
@@ -282,7 +290,8 @@ func (c *client) Breaking(
 			pluginConfigs,
 			policyConfig,
 			breakingOptions.excludeImports,
-			nil, // relatedCheckConfigs.
+			nil,  // relatedCheckConfigs.
+			true, // hasPolicyConfigs.
 		)
 		if err != nil {
 			return err
@@ -311,6 +320,7 @@ func (c *client) breaking(
 	policyConfig bufconfig.PolicyConfig,
 	excludeImports bool,
 	relatedCheckConfigs []bufconfig.CheckConfig,
+	hasPolicyConfigs bool,
 ) ([]*annotation, error) {
 	if breakingConfig.Disabled() {
 		return nil, nil
@@ -335,7 +345,11 @@ func (c *client) breaking(
 	if err != nil {
 		return nil, err
 	}
-	logRulesConfig(c.logger, config.rulesConfig)
+	configName := bufconfig.DefaultBufYAMLFileName
+	if policyConfig != nil {
+		configName = policyConfig.Name()
+	}
+	logRulesConfig(c.logger, configName, config.rulesConfig, hasPolicyConfigs)
 	fileDescriptors, err := descriptor.FileDescriptorsForProtoFileDescriptors(imageToProtoFileDescriptors(image))
 	if err != nil {
 		// An Image may be invalid if it does not contain all of the required dependencies.
@@ -402,6 +416,7 @@ func (c *client) ConfiguredRules(
 	if err != nil {
 		return nil, err
 	}
+	logRulesConfig(c.logger, "", rulesConfig, len(configuredRulesOptions.policyConfigs) > 0)
 	allRules := rulesForRuleIDs(rules, rulesConfig.RuleIDs)
 	policies, err := c.getPolicies(ctx, configuredRulesOptions.policyConfigs)
 	if err != nil {
@@ -436,7 +451,6 @@ func (c *client) ConfiguredRules(
 		}
 		allRules = append(allRules, rulesForRuleIDs(policyRules, policyRulesConfig.RuleIDs)...)
 	}
-	logRulesConfig(c.logger, rulesConfig)
 	return allRules, nil
 }
 

private/bufpkg/bufcheck/rules_config.go

@@ -47,13 +47,13 @@ func rulesConfigForCheckConfig(
 	)
 }
 
-func logRulesConfig(logger *slog.Logger, rulesConfig *rulesConfig) {
+func logRulesConfig(logger *slog.Logger, configName string, rulesConfig *rulesConfig, hasPolicyConfigs bool) {
 	logger.Debug("rulesConfig", slog.Any("ruleIDs", rulesConfig.RuleIDs))
-	if len(rulesConfig.RuleIDs) == 0 {
-		logger.Warn("No " + rulesConfig.RuleType.String() + " rules are configured.")
+	if len(rulesConfig.RuleIDs) == 0 && !hasPolicyConfigs {
+		logger.Warn("No " + rulesConfig.RuleType.String() + " rules are configured in " + configName + ".")
 	}
-	warnReferencedDeprecatedIDs(logger, rulesConfig)
-	warnUnusedPlugins(logger, rulesConfig)
+	warnReferencedDeprecatedIDs(logger, configName, rulesConfig)
+	warnUnusedPlugins(logger, configName, rulesConfig)
 }
 
 type rulesConfig struct {
@@ -344,28 +344,30 @@ func newRulesConfig(
 
 // *** JUST USED WITHIN THIS FILE ***
 
-func warnReferencedDeprecatedIDs(logger *slog.Logger, rulesConfig *rulesConfig) {
+func warnReferencedDeprecatedIDs(logger *slog.Logger, configName string, rulesConfig *rulesConfig) {
 	warnReferencedDeprecatedIDsForIDType(
 		logger,
+		configName,
 		rulesConfig.ReferencedDeprecatedRuleIDToReplacementIDs,
 		"Rule",
 		"rules",
 	)
 	warnReferencedDeprecatedIDsForIDType(
 		logger,
+		configName,
 		rulesConfig.ReferencedDeprecatedCategoryIDToReplacementIDs,
 		"Category",
 		"categories",
 	)
 }
 
-func warnUnusedPlugins(logger *slog.Logger, rulesConfig *rulesConfig) {
+func warnUnusedPlugins(logger *slog.Logger, configName string, rulesConfig *rulesConfig) {
 	if len(rulesConfig.UnusedPluginNameToRuleIDs) == 0 {
 		return
 	}
 	unusedPluginNames := xslices.MapKeysToSortedSlice(rulesConfig.UnusedPluginNameToRuleIDs)
 	var sb strings.Builder
-	_, _ = sb.WriteString("Your buf.yaml has plugins added which have no rules configured:\n\n")
+	_, _ = sb.WriteString("Your " + configName + " has plugins added which have no rules configured:\n\n")
 	for _, unusedPluginName := range unusedPluginNames {
 		_, _ = sb.WriteString("\t  - ")
 		_, _ = sb.WriteString(unusedPluginName)
@@ -390,6 +392,7 @@ func warnUnusedPlugins(logger *slog.Logger, rulesConfig *rulesConfig) {
 
 func warnReferencedDeprecatedIDsForIDType(
 	logger *slog.Logger,
+	configName string,
 	referencedDeprecatedIDToReplacementIDs map[string]map[string]struct{},
 	capitalizedIDType string,
 	pluralIDType string,
@@ -406,24 +409,26 @@ func warnReferencedDeprecatedIDsForIDType(
 		}
 		var specialCallout string
 		if deprecatedID == "DEFAULT" {
-			specialCallout = `
+			specialCallout = fmt.Sprintf(`
 
 	The concept of a default rule has been introduced. A default rule is a rule that will be run
-	if no rules are explicitly configured in your buf.yaml. Run buf config ls-lint-rules or
+	if no rules are explicitly configured in your %s. Run buf config ls-lint-rules or
 	buf config ls-breaking-rules to see which rules are defaults. With this introduction, having a category
 	also named DEFAULT is confusing, as while it happens that all the rules in the DEFAULT category
 	are also default rules, the name has become overloaded.
-`
+`, configName)
 		}
 		logger.Warn(
 			fmt.Sprintf(
-				"%s %s referenced in your buf.yaml is deprecated.%s%s\n\tAs with all buf changes, this change is backwards-compatible: %s will continue to work.\n\tWe recommend replacing %s in your buf.yaml, but no action is immediately necessary.",
+				"%s %s referenced in your %s is deprecated.%s%s\n\tAs with all buf changes, this change is backwards-compatible: %s will continue to work.\n\tWe recommend replacing %s in your %s, but no action is immediately necessary.",
 				capitalizedIDType,
 				deprecatedID,
+				configName,
 				replaceString,
 				specialCallout,
 				deprecatedID,
 				deprecatedID,
+				configName,
 			),
 		)
 	}