encoded-compare false positives fix (#199)

* encoded-compare: require string or []byte type for json-like or yaml-like named vars

* encoded-compare: strict match for yml (not yaml)

* cosmetic: reorder funcs

analyzer/analyzer_test.go

@@ -60,6 +60,14 @@ func TestTestifyLint_NotDefaultCases(t *testing.T) {
 			dir:   "checkers-priority",
 			flags: map[string]string{"enable-all": "true"},
 		},
+		{
+			dir:   "encoded-compare-issue196",
+			flags: map[string]string{"disable-all": "true", "enable": checkers.NewEncodedCompare().Name()},
+		},
+		{
+			dir:   "encoded-compare-issue198",
+			flags: map[string]string{"disable-all": "true", "enable": checkers.NewEncodedCompare().Name()},
+		},
 		{
 			dir:   "error-as-target",
 			flags: map[string]string{"disable-all": "true", "enable": checkers.NewErrorIsAs().Name()},

analyzer/testdata/src/encoded-compare-issue196/false_positive_test.go

@@ -0,0 +1,84 @@
+package encodedcompareissue196
+
+import (
+	"bytes"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepoFollowSymlink(t *testing.T) {
+	defer PrepareTestEnv(t)()
+	session := loginUser(t, "user2")
+
+	assertCase := func(t *testing.T, url, expectedSymlinkURL string, shouldExist bool) {
+		t.Helper()
+
+		req := NewRequest(t, "GET", url)
+		resp := session.MakeRequest(t, req, http.StatusOK)
+
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		symlinkURL, ok := htmlDoc.Find(".file-actions .button[data-kind='follow-symlink']").Attr("href")
+		if shouldExist {
+			assert.True(t, ok)
+			assert.Equal(t, expectedSymlinkURL, symlinkURL)
+		} else {
+			assert.False(t, ok)
+		}
+	}
+
+	t.Run("Normal", func(t *testing.T) {
+		defer PrintCurrentTest(t)()
+		assertCase(t, "/user2/readme-test/src/branch/symlink/README.md?display=source", "/user2/readme-test/src/branch/symlink/some/other/path/awefulcake.txt", true)
+	})
+}
+
+func PrepareTestEnv(t *testing.T) func()   { return func() {} }
+func PrintCurrentTest(t *testing.T) func() { return func() {} }
+
+func loginUser(t *testing.T, uid string) *session {
+	return new(session)
+}
+
+type session struct {
+}
+
+func (s *session) MakeRequest(t *testing.T, req *http.Request, expStatusCode int) *Response {
+	return new(Response)
+}
+
+func NewRequest(t *testing.T, method string, url string) *http.Request {
+	return new(http.Request)
+}
+
+type Response struct {
+	Body *bytes.Buffer
+}
+
+// HTMLDoc struct
+type HTMLDoc struct {
+}
+
+// NewHTMLParser parse html file
+func NewHTMLParser(t testing.TB, body *bytes.Buffer) *HTMLDoc {
+	t.Helper()
+	return new(HTMLDoc)
+}
+
+// Find gets the descendants of each element in the current set of
+// matched elements, filtered by a selector. It returns a new Selection
+// object containing these matched elements.
+func (doc *HTMLDoc) Find(selector string) *Selection {
+	return new(Selection)
+}
+
+type Selection struct {
+}
+
+// Attr gets the specified attribute's value for the first element in the
+// Selection. To get the value for each element individually, use a looping
+// construct such as Each or Map method.
+func (s *Selection) Attr(attrName string) (val string, exists bool) {
+	return "", false
+}

analyzer/testdata/src/encoded-compare-issue198/var_naming_and_type_test.go

@@ -0,0 +1,45 @@
+package encodedcompareissue198
+
+import (
+	"bytes"
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCommonResultFromFullResult(t *testing.T) {
+	jsonData := new(bytes.Buffer)
+	var cr *commonResult
+
+	crFromJSON := new(commonResult)
+	err := json.Unmarshal(jsonData.Bytes(), crFromJSON)
+	require.NoError(t, err)
+
+	assert.Equal(t, cr, crFromJSON)
+}
+
+func TestCommonResultFromFullAndCompactJSON(t *testing.T) {
+	compactJSONData := new(bytes.Buffer)
+	fullJSONData := new(bytes.Buffer)
+
+	crFromCompactJSON := new(commonResult)
+	crFromFullJSON := new(commonResult)
+
+	err := json.NewDecoder(compactJSONData).Decode(crFromCompactJSON)
+	require.NoError(t, err)
+
+	err = json.NewDecoder(fullJSONData).Decode(crFromCompactJSON)
+	require.NoError(t, err)
+
+	assert.Equal(t, crFromCompactJSON, crFromFullJSON)
+
+	var crFromYML *commonResult
+	assert.Equal(t, crFromYML, new(commonResult))
+}
+
+type commonResult struct {
+	Name string `json:"name"`
+	Size int64  `json:"size"`
+}

internal/checkers/encoded_compare.go

@@ -48,7 +48,7 @@ func (checker EncodedCompare) Check(pass *analysis.Pass, call *CallMeta) *analys
 	switch {
 	case aIsExplicitJSON, bIsExplicitJSON, isJSONStyleExpr(pass, a), isJSONStyleExpr(pass, b):
 		proposed = "JSONEq"
-	case isYAMLStyleExpr(a), isYAMLStyleExpr(b):
+	case isYAMLStyleExpr(pass, a), isYAMLStyleExpr(pass, b):
 		proposed = "YAMLEq"
 	}
 

internal/checkers/helpers_basic_type.go

@@ -166,6 +166,12 @@ func hasBytesType(pass *analysis.Pass, e ast.Expr) bool {
 	return ok && el.Kind() == types.Uint8
 }
 
+// hasStringType returns true if the expression is of `string` type.
+func hasStringType(pass *analysis.Pass, e ast.Expr) bool {
+	basicType, ok := pass.TypesInfo.TypeOf(e).(*types.Basic)
+	return ok && basicType.Kind() == types.String
+}
+
 // untype returns v from type(v) expression or v itself if there is no type conversion.
 func untype(e ast.Expr) ast.Expr {
 	ce, ok := e.(*ast.CallExpr)

internal/checkers/helpers_encoded.go

@@ -11,13 +11,15 @@ import (
 )
 
 var (
+	wordsRe = regexp.MustCompile(`[A-Z]+(?:[a-z]*|$)|[a-z]+`) // NOTE(a.telyshev): ChatGPT.
+
 	jsonIdentRe = regexp.MustCompile(`json|JSON|Json`)
-	yamlIdentRe = regexp.MustCompile(`yaml|YAML|Yaml|yml|YML|Yml`)
+	yamlWordRe  = regexp.MustCompile(`yaml|YAML|Yaml|^(yml|YML|Yml)$`)
 )
 
 func isJSONStyleExpr(pass *analysis.Pass, e ast.Expr) bool {
 	if isIdentNamedAfterPattern(jsonIdentRe, e) {
-		return true
+		return hasBytesType(pass, e) || hasStringType(pass, e)
 	}
 
 	if t, ok := pass.TypesInfo.Types[e]; ok && t.Value != nil {
@@ -35,6 +37,20 @@ func isJSONStyleExpr(pass *analysis.Pass, e ast.Expr) bool {
 	return false
 }
 
-func isYAMLStyleExpr(e ast.Expr) bool {
-	return isIdentNamedAfterPattern(yamlIdentRe, e)
+func isYAMLStyleExpr(pass *analysis.Pass, e ast.Expr) bool {
+	id, ok := e.(*ast.Ident)
+	return ok && (hasBytesType(pass, e) || hasStringType(pass, e)) && hasWordAfterPattern(id.Name, yamlWordRe)
+}
+
+func hasWordAfterPattern(s string, re *regexp.Regexp) bool {
+	for _, w := range splitIntoWords(s) {
+		if re.MatchString(w) {
+			return true
+		}
+	}
+	return false
+}
+
+func splitIntoWords(s string) []string {
+	return wordsRe.FindAllString(s, -1)
 }