Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows inversing the semantics of string-format rule configurations #765

Merged
merged 3 commits into from
Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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