Skip to content

Commit

Permalink
fix duplicated findings for string-format when there is more than one… (
Browse files Browse the repository at this point in the history
  • Loading branch information
chavacava authored Nov 4, 2024
1 parent fa9af82 commit 7c068a7
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 57 deletions.
106 changes: 49 additions & 57 deletions rule/string-format.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (w lintStringFormatRule) Visit(node ast.Node) ast.Visitor {
for _, rule := range w.rules {
for _, scope := range rule.scopes {
if scope.funcName == callName {
rule.Apply(call)
rule.apply(call, scope)
}
}
}
Expand Down Expand Up @@ -251,81 +251,73 @@ func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok

// #region Linting logic

// 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) {
for _, scope := range r.scopes {
if len(call.Args) <= scope.argument {
// 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, scope *stringFormatSubruleScope) {
if len(call.Args) <= scope.argument {
return
}

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 {
return
}

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)
for _, el := range composite.Elts {
kv, ok := el.(*ast.KeyValueExpr)
if !ok {
return
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
}
key, ok := kv.Key.(*ast.Ident)
if !ok || key.Name != scope.field {
continue
}
} else {
var ok bool
// Treat arg as a string literal
lit, ok = arg.(*ast.BasicLit)

// 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
}
}
// 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) {
if r.negated {
if !r.regexp.MatchString(s) {
} else {
var ok bool
// Treat arg as a string literal
lit, ok = arg.(*ast.BasicLit)
if !ok || lit.Kind != token.STRING {
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,
})
}
// Unquote the string literal before linting
unquoted := lit.Value[1 : len(lit.Value)-1]
if r.stringIsOK(unquoted) {
return
}

// Fail if the string does NOT match the user's regex
if r.regexp.MatchString(s) {
return
r.generateFailure(unquoted, lit)
}

func (r *stringFormatSubrule) stringIsOK(s string) bool {
matches := r.regexp.MatchString(s)
if r.negated {
return !matches
}

return matches
}

func (r *stringFormatSubrule) generateFailure(s string, node ast.Node) {
var failure string
if len(r.errorMessage) > 0 {
switch {
case len(r.errorMessage) > 0:
failure = r.errorMessage
} else {
case r.negated:
failure = fmt.Sprintf("string literal matches user defined regex /%s/", r.regexp.String())
case !r.negated:
failure = fmt.Sprintf("string literal doesn't match user defined regex /%s/", r.regexp.String())
}

r.parent.onFailure(lint.Failure{
Confidence: 1,
Failure: failure,
Expand Down
10 changes: 10 additions & 0 deletions test/string-format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,13 @@ func TestStringFormatArgumentParsing(t *testing.T) {
})
}
}

func TestStringFormatDuplicatedStrings(t *testing.T) {
testRule(t, "string-format-issue-1063", &rule.StringFormatRule{}, &lint.RuleConfig{
Arguments: lint.Arguments{[]any{
"fmt.Errorf[0],errors.New[0]",
"/^([^A-Z]|$)/",
"must not start with a capital letter",
}},
})
}
9 changes: 9 additions & 0 deletions testdata/string-format-issue-1063.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package fixtures

import (
"errors"
)

func ReturnError() error {
return errors.New("This is an error.") // MATCH /must not start with a capital letter/
}

0 comments on commit 7c068a7

Please sign in to comment.