Skip to content

Commit

Permalink
fix: add-constant struct tags in anonymous struct literals false posi…
Browse files Browse the repository at this point in the history
…tive
  • Loading branch information
denisvmedia committed Dec 27, 2023
1 parent 8d5724f commit 0c55270
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 10 deletions.
40 changes: 31 additions & 9 deletions rule/add-constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lin
failures = append(failures, failure)
}

w := lintAddConstantRule{
w := &lintAddConstantRule{
onFailure: onFailure,
strLits: make(map[string]int),
strLitLimit: r.strLitLimit,
whiteLst: r.whiteList,
ignoreFunctions: r.ignoreFunctions,
structTags: make(map[*ast.BasicLit]struct{}),
}

ast.Walk(w, file.AST)
Expand All @@ -73,23 +74,38 @@ type lintAddConstantRule struct {
strLitLimit int
whiteLst whiteList
ignoreFunctions []*regexp.Regexp
structTags map[*ast.BasicLit]struct{}
}

func (w lintAddConstantRule) Visit(node ast.Node) ast.Visitor {
func (w *lintAddConstantRule) Visit(node ast.Node) ast.Visitor {
if node == nil {
return nil
}

switch n := node.(type) {
case *ast.CallExpr:
w.checkFunc(n)
return nil
case *ast.GenDecl:
return nil // skip declarations
case *ast.BasicLit:
w.checkLit(n)
if !w.isStructTag(n) {
w.checkLit(n)
}
case *ast.StructType:
if n.Fields != nil {
for _, field := range n.Fields.List {
if field.Tag != nil {
w.structTags[field.Tag] = struct{}{}
}
}
}
}

return w
}

func (w lintAddConstantRule) checkFunc(expr *ast.CallExpr) {
func (w *lintAddConstantRule) checkFunc(expr *ast.CallExpr) {
fName := w.getFuncName(expr)

for _, arg := range expr.Args {
Expand All @@ -105,7 +121,7 @@ func (w lintAddConstantRule) checkFunc(expr *ast.CallExpr) {
}
}

func (lintAddConstantRule) getFuncName(expr *ast.CallExpr) string {
func (*lintAddConstantRule) getFuncName(expr *ast.CallExpr) string {
switch f := expr.Fun.(type) {
case *ast.SelectorExpr:
switch prefix := f.X.(type) {
Expand All @@ -119,7 +135,7 @@ func (lintAddConstantRule) getFuncName(expr *ast.CallExpr) string {
return ""
}

func (w lintAddConstantRule) checkLit(n *ast.BasicLit) {
func (w *lintAddConstantRule) checkLit(n *ast.BasicLit) {
switch kind := n.Kind.String(); kind {
case kindFLOAT, kindINT:
w.checkNumLit(kind, n)
Expand All @@ -128,7 +144,7 @@ func (w lintAddConstantRule) checkLit(n *ast.BasicLit) {
}
}

func (w lintAddConstantRule) isIgnoredFunc(fName string) bool {
func (w *lintAddConstantRule) isIgnoredFunc(fName string) bool {
for _, pattern := range w.ignoreFunctions {
if pattern.MatchString(fName) {
return true
Expand All @@ -138,7 +154,7 @@ func (w lintAddConstantRule) isIgnoredFunc(fName string) bool {
return false
}

func (w lintAddConstantRule) checkStrLit(n *ast.BasicLit) {
func (w *lintAddConstantRule) checkStrLit(n *ast.BasicLit) {
if w.whiteLst[kindSTRING][n.Value] {
return
}
Expand All @@ -158,7 +174,7 @@ func (w lintAddConstantRule) checkStrLit(n *ast.BasicLit) {
}
}

func (w lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
func (w *lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
if w.whiteLst[kind][n.Value] {
return
}
Expand All @@ -171,6 +187,12 @@ func (w lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
})
}

// isStructTag checks if the given BasicLit is part of a struct tag.
func (w *lintAddConstantRule) isStructTag(n *ast.BasicLit) bool {
_, ok := w.structTags[n]
return ok
}

func (r *AddConstantRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()
Expand Down
38 changes: 37 additions & 1 deletion testdata/add-constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"os"
)

func foo(a, b, c, d int) {
func foo(a float32, b string, c any, d int) {
a = 1.0 // ignore
b = "ignore"
c = 2 // ignore
Expand Down Expand Up @@ -51,3 +51,39 @@ func ignoredFunc(num int) int {
func notIgnoredFunc(num int) int {
return num
}

func tagsInStructLiteralsShouldBeOK() {
a := struct {
X int `json:"x"`
}{}

b := struct {
X int `json:"x"`
}{}

c := struct {
X int `json:"x"`
}{}

d := struct {
X int `json:"x"`
Y int `json:"y"`
}{}

e := struct {
X int `json:"x"`
Y int `json:"y"`
}{}

var f struct {
X int `json:"x"`
Y int `json:"y"`
}

var g struct {
X int `json:"x"`
Y int `json:"y"`
}

_, _, _, _, _, _, _ = a, b, c, d, e, f, g
}

0 comments on commit 0c55270

Please sign in to comment.