Skip to content

Commit

Permalink
Allows inversing the semantics of string-format rule configurations (
Browse files Browse the repository at this point in the history
…#765)

* allows negating the regexp

* updates rule documentation

* adds mgechev remarks
  • Loading branch information
chavacava authored Oct 24, 2022
1 parent 06881a9 commit 32a0cb8
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
9 changes: 6 additions & 3 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -565,11 +565,14 @@ 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]`).

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.
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:

3. The third string (optional) is a message containing the purpose for the regex, which will be used in lint errors.
* with `"/^[A-Z].*$/"` the rule will **accept** strings starting with capital letters
* with `"!/^[A-Z].*$/"` the rule will a **fail** on strings starting with capital letters

3. The third string (optional) is a **message** containing the purpose for the regex, which will be used in lint errors.

Example:

Expand Down
36 changes: 31 additions & 5 deletions rule/string-format.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type stringFormatSubrule struct {
parent *lintStringFormatRule
scope stringFormatSubruleScope
regexp *regexp.Regexp
negated bool
errorMessage string
}

Expand All @@ -89,17 +90,18 @@ var parseStringFormatScope = regexp.MustCompile(

func (w *lintStringFormatRule) parseArguments(arguments lint.Arguments) {
for i, argument := range arguments {
scope, regex, errorMessage := w.parseArgument(argument, i)
scope, regex, negated, errorMessage := w.parseArgument(argument, i)
w.rules = append(w.rules, stringFormatSubrule{
parent: w,
scope: scope,
regexp: regex,
negated: negated,
errorMessage: errorMessage,
})
}
}

func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (scope stringFormatSubruleScope, regex *regexp.Regexp, errorMessage string) {
func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (scope stringFormatSubruleScope, regex *regexp.Regexp, negated bool, errorMessage string) {
g, ok := argument.([]interface{}) // Cast to generic slice first
if !ok {
w.configError("argument is not a slice", ruleNum, 0)
Expand Down Expand Up @@ -146,7 +148,12 @@ func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (
}

// Strip / characters from the beginning and end of rule[1] before compiling
regex, err := regexp.Compile(rule[1][1 : len(rule[1])-1])
negated = rule[1][0] == '!'
offset := 1
if negated {
offset++
}
regex, err := regexp.Compile(rule[1][offset : len(rule[1])-1])
if err != nil {
w.parseError(fmt.Sprintf("unable to compile %s as regexp", rule[1]), ruleNum, 1)
}
Expand All @@ -155,7 +162,7 @@ func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) (
if len(rule) == 3 {
errorMessage = rule[2]
}
return scope, regex, errorMessage
return scope, regex, negated, errorMessage
}

// Report an invalid config, this is specifically the user's fault
Expand Down Expand Up @@ -261,7 +268,26 @@ func (r *stringFormatSubrule) Apply(call *ast.CallExpr) {
}

func (r *stringFormatSubrule) lintMessage(s string, node ast.Node) {
// Fail if the string doesn't match the user's regex
if r.negated {
if !r.regexp.MatchString(s) {
return
}
// Fail if the string does match the user's regex
var failure string
if len(r.errorMessage) > 0 {
failure = r.errorMessage
} else {
failure = fmt.Sprintf("string literal matches user defined regex /%s/", r.regexp.String())
}
r.parent.onFailure(lint.Failure{
Confidence: 1,
Failure: failure,
Node: node,
})
return
}

// Fail if the string does NOT match the user's regex
if r.regexp.MatchString(s) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion test/string-format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestStringFormat(t *testing.T) {
"/[^\\.]$/"}, // Must not end with a period
[]interface{}{
"s.Method3[2]",
"/^[^Tt][^Hh]/",
"!/^[Tt][Hh]/",
"must not start with 'th'"}}})
}

Expand Down

0 comments on commit 32a0cb8

Please sign in to comment.