Add multiple scopes support to string-format rule

RULES_DESCRIPTIONS.md

@@ -758,7 +758,7 @@ This is geared towards user facing applications where string literals are often
 
 _Configuration_: Each argument is a slice containing 2-3 strings: a scope, a regex, and an optional error message.
 
-1. The first string defines a **scope**. This controls which string literals the regex will apply to, and is defined as a function argument. It must contain at least a function name (`core.WriteError`). Scopes may optionally contain a number specifying which argument in the function to check (`core.WriteError[1]`), as well as a struct field (`core.WriteError[1].Message`, only works for top level fields). Function arguments are counted starting at 0, so `[0]` would refer to the first argument, `[1]` would refer to the second, etc. If no argument number is provided, the first argument will be used (same as `[0]`).
+1. The first string defines a **scope**. This controls which string literals the regex will apply to, and is defined as a function argument. It must contain at least a function name (`core.WriteError`). Scopes may optionally contain a number specifying which argument in the function to check (`core.WriteError[1]`), as well as a struct field (`core.WriteError[1].Message`, only works for top level fields). Function arguments are counted starting at 0, so `[0]` would refer to the first argument, `[1]` would refer to the second, etc. If no argument number is provided, the first argument will be used (same as `[0]`). You can use multiple scopes to one regex. Split them by `,` (`core.WriteError,fmt.Errorf`). 
 
 2. The second string is a **regular expression** (beginning and ending with a `/` character), which will be used to check the string literals in the scope. The default semantics is "_strings matching the regular expression are OK_". If you need to inverse the semantics you can add a `!` just before the first `/`. Examples:
 
@@ -775,6 +775,7 @@ Example:
  ["core.WriteError[1].Message", "/^([^A-Z]|$)/", "must not start with a capital letter"],
  ["fmt.Errorf[0]", "/(^|[^\\.!?])$/", "must not end in punctuation"],
  ["panic", "/^[^\\n]*$/", "must not contain line breaks"],
+ ["fmt.Errorf[0],core.WriteError[1].Message", "!/^.*%w.*$/", "must not contain '%w'"],
  ]
 ```
 

rule/string-format.go

@@ -6,6 +6,7 @@ import (
  "go/token"
  "regexp"
  "strconv"
+ "strings"
 
  "github.com/mgechev/revive/lint"
 )
@@ -66,12 +67,14 @@ type lintStringFormatRule struct {
 
 type stringFormatSubrule struct {
  parent *lintStringFormatRule
- scope  stringFormatSubruleScope
+ scopes stringFormatSubruleScopes
  regexp *regexp.Regexp
  negated bool
  errorMessage string
 }
 
+type stringFormatSubruleScopes []*stringFormatSubruleScope
+
 type stringFormatSubruleScope struct {
  funcName string // Function name the rule is scoped to
  argument int // (optional) Which argument in calls to the function is checked against the rule (the first argument is checked by default)
@@ -90,18 +93,18 @@ var parseStringFormatScope = regexp.MustCompile(
 
 func (w *lintStringFormatRule) parseArguments(arguments lint.Arguments) {
  for i, argument := range arguments {
- scope, regex, negated, errorMessage := w.parseArgument(argument, i)
+ scopes, regex, negated, errorMessage := w.parseArgument(argument, i)
  w.rules = append(w.rules, stringFormatSubrule{
  parent: w,
- scope:  scope,
+ scopes: scopes,
  regexp: regex,
  negated: negated,
  errorMessage: errorMessage,
  })
  }
 }
 
-func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scope stringFormatSubruleScope, regex *regexp.Regexp, negated bool, errorMessage string) {
+func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scopes stringFormatSubruleScopes, regex *regexp.Regexp, negated bool, errorMessage string) {
  g, ok := argument.([]any) // Cast to generic slice first
  if !ok {
  w.configError("argument is not a slice", ruleNum, 0)
@@ -125,26 +128,39 @@ func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scope st
  w.configError("regex is too small (regexes should begin and end with '/')", ruleNum, 1)
  }
 
- // Parse rule scope
- scope = stringFormatSubruleScope{}
- matches := parseStringFormatScope.FindStringSubmatch(rule[0])
- if matches == nil {
- // The rule's scope didn't match the parsing regex at all, probably a configuration error
- w.parseError("unable to parse rule scope", ruleNum, 0)
- } else if len(matches) != 4 {
- // The rule's scope matched the parsing regex, but an unexpected number of submatches was returned, probably a bug
- w.parseError(fmt.Sprintf("unexpected number of submatches when parsing scope: %d, expected 4", len(matches)), ruleNum, 0)
- }
- scope.funcName = matches[1]
- if len(matches[2]) > 0 {
- var err error
- scope.argument, err = strconv.Atoi(matches[2])
- if err != nil {
- w.parseError("unable to parse argument number in rule scope", ruleNum, 0)
+ // Parse rule scopes
+ rawScopes := strings.Split(rule[0], ",")
+
+ scopes = make([]*stringFormatSubruleScope, 0, len(rawScopes))
+ for scopeNum, rawScope := range rawScopes {
+ rawScope = strings.TrimSpace(rawScope)
+
+ if len(rawScope) == 0 {
+ w.parseScopeError("empty scope in rule scopes:", ruleNum, 0, scopeNum)
  }
- }
- if len(matches[3]) > 0 {
- scope.field = matches[3]
+
+ scope := stringFormatSubruleScope{}
+ matches := parseStringFormatScope.FindStringSubmatch(rawScope)
+ if matches == nil {
+ // The rule's scope didn't match the parsing regex at all, probably a configuration error
+ w.parseScopeError("unable to parse rule scope", ruleNum, 0, scopeNum)
+ } else if len(matches) != 4 {
+ // The rule's scope matched the parsing regex, but an unexpected number of submatches was returned, probably a bug
+ w.parseScopeError(fmt.Sprintf("unexpected number of submatches when parsing scope: %d, expected 4", len(matches)), ruleNum, 0, scopeNum)
+ }
+ scope.funcName = matches[1]
+ if len(matches[2]) > 0 {
+ var err error
+ scope.argument, err = strconv.Atoi(matches[2])
+ if err != nil {
+ w.parseScopeError("unable to parse argument number in rule scope", ruleNum, 0, scopeNum)
+ }
+ }
+ if len(matches[3]) > 0 {
+ scope.field = matches[3]
+ }
+
+ scopes = append(scopes, &scope)
  }
 
  // Strip / characters from the beginning and end of rule[1] before compiling
@@ -162,7 +178,7 @@ func (w lintStringFormatRule) parseArgument(argument any, ruleNum int) (scope st
  if len(rule) == 3 {
  errorMessage = rule[2]
  }
- return scope, regex, negated, errorMessage
+ return scopes, regex, negated, errorMessage
 }
 
 // Report an invalid config, this is specifically the user's fault
@@ -175,6 +191,11 @@ func (lintStringFormatRule) parseError(msg string, ruleNum, option int) {
  panic(fmt.Sprintf("failed to parse configuration for string-format: %s [argument %d, option %d]", msg, ruleNum, option))
 }
 
+// Report a general scope config parsing failure, this may be the user's fault, but it isn't known for certain
+func (lintStringFormatRule) parseScopeError(msg string, ruleNum, option, scopeNum int) {
+ panic(fmt.Sprintf("failed to parse configuration for string-format: %s [argument %d, option %d, scope index %d]", msg, ruleNum, option, scopeNum))
+}
+
 // #endregion
 
 // #region Node traversal
@@ -193,8 +214,10 @@ func (w lintStringFormatRule) Visit(node ast.Node) ast.Visitor {
  }
 
  for _, rule := range w.rules {
- if rule.scope.funcName == callName {
- rule.Apply(call)
+ for _, scope := range rule.scopes {
+ if scope.funcName == callName {
+ rule.Apply(call)
+ }
  }
  }
 
@@ -230,45 +253,47 @@ func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok
 
 // Apply a single format rule to a call expression (should be done after verifying the that the call expression matches the rule's scope)
 func (r *stringFormatSubrule) Apply(call *ast.CallExpr) {
- if len(call.Args) <= r.scope.argument {
- return
- }
-
- arg := call.Args[r.scope.argument]
- var lit *ast.BasicLit
- if len(r.scope.field) > 0 {
- // Try finding the scope's Field, treating arg as a composite literal
- composite, ok := arg.(*ast.CompositeLit)
- if !ok {
+ for _, scope := range r.scopes {
+ if len(call.Args) <= scope.argument {
  return
  }
- for _, el := range composite.Elts {
- kv, ok := el.(*ast.KeyValueExpr)
+
+ arg := call.Args[scope.argument]
+ var lit *ast.BasicLit
+ if len(scope.field) > 0 {
+ // Try finding the scope's Field, treating arg as a composite literal
+ composite, ok := arg.(*ast.CompositeLit)
  if !ok {
- continue
+ return
  }
- key, ok := kv.Key.(*ast.Ident)
- if !ok || key.Name != r.scope.field {
- continue
+ for _, el := range composite.Elts {
+ kv, ok := el.(*ast.KeyValueExpr)
+ if !ok {
+ continue
+ }
+ key, ok := kv.Key.(*ast.Ident)
+ if !ok || key.Name != scope.field {
+ continue
+ }
+
+ // We're now dealing with the exact field in the rule's scope, so if anything fails, we can safely return instead of continuing the loop
+ lit, ok = kv.Value.(*ast.BasicLit)
+ if !ok || lit.Kind != token.STRING {
+ return
+ }
  }
-
- // We're now dealing with the exact field in the rule's scope, so if anything fails, we can safely return instead of continuing the loop
- lit, ok = kv.Value.(*ast.BasicLit)
+ } else {
+ var ok bool
+ // Treat arg as a string literal
+ lit, ok = arg.(*ast.BasicLit)
  if !ok || lit.Kind != token.STRING {
  return
  }
  }
- } else {
- var ok bool
- // Treat arg as a string literal
- lit, ok = arg.(*ast.BasicLit)
- if !ok || lit.Kind != token.STRING {
- return
- }
+ // Unquote the string literal before linting
+ unquoted := lit.Value[1 : len(lit.Value)-1]
+ r.lintMessage(unquoted, lit)
  }
- // Unquote the string literal before linting
- unquoted := lit.Value[1 : len(lit.Value)-1]
- r.lintMessage(unquoted, lit)
 }
 
 func (r *stringFormatSubrule) lintMessage(s string, node ast.Node) {

test/string-format_test.go

@@ -76,7 +76,7 @@ func TestStringFormatArgumentParsing(t *testing.T) {
  []any{
  "1.a",
  "//"}},
- expectedError: stringPtr("failed to parse configuration for string-format: unable to parse rule scope [argument 0, option 0]")},
+ expectedError: stringPtr("failed to parse configuration for string-format: unable to parse rule scope [argument 0, option 0, scope index 0]")},
  {
  name: "Bad Regex",
  config: lint.Arguments{
@@ -98,7 +98,65 @@ func TestStringFormatArgumentParsing(t *testing.T) {
  config: lint.Arguments{
  []any{
  "some_pkg._some_function_name[5].some_member",
- "//"}}}}
+ "//"}}},
+ {
+ name: "Underscores in Multiple Scopes",
+ config: lint.Arguments{
+ []any{
+ "fmt.Errorf[0],core.WriteError[1].Message",
+ "//"}}},
+ {
+ name: "', ' Delimiter",
+ config: lint.Arguments{
+ []any{
+ "abc, mt.Errorf",
+ "//"}}},
+ {
+ name: "' ,' Delimiter",
+ config: lint.Arguments{
+ []any{
+ "abc ,mt.Errorf",
+ "//"}}},
+ {
+ name: "', ' Delimiter",
+ config: lint.Arguments{
+ []any{
+ "abc, mt.Errorf",
+ "//"}}},
+ {
+ name: "', ' Delimiter",
+ config: lint.Arguments{
+ []any{
+ "abc, mt.Errorf",
+ "//"}}},
+ {
+ name: "Empty Middle Scope",
+ config: lint.Arguments{
+ []any{
+ "abc, ,mt.Errorf",
+ "//"}},
+ expectedError: stringPtr("failed to parse configuration for string-format: empty scope in rule scopes: [argument 0, option 0, scope index 1]")},
+ {
+ name: "Empty First Scope",
+ config: lint.Arguments{
+ []any{
+ ",mt.Errorf",
+ "//"}},
+ expectedError: stringPtr("failed to parse configuration for string-format: empty scope in rule scopes: [argument 0, option 0, scope index 0]")},
+ {
+ name: "Bad First Scope",
+ config: lint.Arguments{
+ []any{
+ "1.a,fmt.Errorf[0]",
+ "//"}},
+ expectedError: stringPtr("failed to parse configuration for string-format: unable to parse rule scope [argument 0, option 0, scope index 0]")},
+ {
+ name: "Bad Second Scope",
+ config: lint.Arguments{
+ []any{
+ "fmt.Errorf[0],1.a",
+ "//"}},
+ expectedError: stringPtr("failed to parse configuration for string-format: unable to parse rule scope [argument 0, option 0, scope index 1]")}}
 
  for _, a := range tests {
  t.Run(a.name, func(t *testing.T) {