Repository: mgechev/revive (go)

Issue:
add-constant suggests invalid fix of replacing struct tags with string constants for anonymous structs
**Describe the bug**

revive incorrectly suggests replacing duplicated struct tags with string constants, for tags found in anonymous struct literals.

Example file to reproduce:

```
package test

func x() {
	a := struct {
		X int `json:"x"`
	}{}

	b := struct {
		X int `json:"x"`
	}{}

	c := struct {
		X int `json:"x"`
	}{}

	_, _, _ = a, b, c
}
```

**To Reproduce**
Steps to reproduce the behavior:
1. I updated revive `go install github.com/mgechev/revive@latest`
2. I run it with the following flags & configuration file:

```shell
# flags

revive -config config.toml .
```

Output:

```
test.go:13:9: string literal `json:"x"` appears, at least, 3 times, create a named constant for it
```

```toml
[rule.add-constant]
```

**Expected behavior**
Struct tags in anonymous struct literals should be ignored by this rule, as they are for struct type definitions.



Pull request:
fix: add-constant struct tags in anonymous struct literals false positive
closes #951


Patch:
diff --git a/rule/add-constant.go b/rule/add-constant.go
index 0d8cf42..f39f224 100644
--- a/rule/add-constant.go
+++ b/rule/add-constant.go
@@ -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)
@@ -73,9 +74,14 @@ 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)
@@ -83,13 +89,23 @@ func (w lintAddConstantRule) Visit(node ast.Node) ast.Visitor {
 	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 {
@@ -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) {
@@ -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)
@@ -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
@@ -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
 	}
@@ -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
 	}
@@ -171,6 +187,11 @@ func (w lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
 	})
 }
 
+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()


Test patch:
diff --git a/testdata/add-constant.go b/testdata/add-constant.go
index 2186650..53eba17 100644
--- a/testdata/add-constant.go
+++ b/testdata/add-constant.go
@@ -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
@@ -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
+}