encoded-compare: suggest `JSONEq`/`YAMLEq` instead of `Equal` where appropriate

[scop]

Not sure how robustly this could be implemented, but it'd be nice to flag JSON string comparisons using .Equal and suggest to use .JSONEq instead.

[Antonboom]

@scop hi

thank you for request

could you provide some examples from real life?

in additional to obvious

assert.Equal(t, `{"foo": "bar"}`, string(body))

P.S. To YAMLeq relevant too

[scop]

Don't know how useful these might be, but:

One example in addition to the obvious where I've seen this include expected being w.Body.String() where w is a *httptest/ResponseRecorder that is being used to test a JSON emitting endpoint. But that's fairly specific and should be triggered only for responses that are actually JSON, so :shrug:

Perhaps another heuristic for the obvious case would be if expected JSON parses to a slice (not only to a map). Maybe combine with a quick check if the expected sample starts with [ or { before attempting a JSON parse. (I don't think this should be attempted for JSON scalars.) But then again these heuristics might not be good enough and e.g. JSON being a YAML subset could throw this off.

Also, expected as a const (or a static string variable).

[Antonboom]

Example: https://github.com/argoproj/argo-rollouts/blob/ec18d991020693fa2caa1fec4aa21f2f839a46de/utils/replicaset/canary_test.go#L1307

[ccoVeille]

https://github.com/search?q=language%3Ago+%22assert.Equal%28t%2C+%60%7B%22&type=code

				assert.Equal(t, `{"success":false,"error":"the given phone number is not valid"}`, string(body))

There are so many

I thought about the possible implementation.

I would say we should check for backtick followed by ", then try json.Marshal and if no error suggests.

It's a naive implementation idea, it needs to keep thinking about it.

Things to ignore to be found in this

https://github.com/search?q=language%3Ago+%22assert.Equal%28t%2C+%5C%22%7B%22&type=code

But these ones are good candidates

https://github.com/search?q=language%3Ago+%22assert.Equal%28t%2C+json.RawMessage%22&type=code

assert.Equal(t, json.RawMessage(`"b"`), reminders[0].Reminder.Data)

Also this

	assert.Equal(t, "{\"prev\":{\"balance\":999769400000,\"addr\":\"1LmyRajNDhosPBtbYXmiLFkQtb83g9HyUe\"},\"current\":{\"balance\":999769200000,\"addr\":\"1LmyRajNDhosPBtbYXmiLFkQtb83g9HyUe\"}}", string(j))

Some other to be found here https://github.com/search?q=language%3Ago+%22assert.Equal%28t%2C+%5C%22%7B%22&type=code

[Antonboom]

@ccoVeille thanks for examples, I am already implementing this

Please, pay attention to other issues in the nearest milestone 👌

[ccoVeille]

@ccoVeille thanks for examples, I am already implementing this

Please, pay attention to other issues in the nearest milestone 👌

I didn't plan to work on this one more than that, but I tried to find examples of the needs by looking at invalid pattern.

I was simply curious

[Antonboom]

@scop 👋

testifylint#encoded-compare

[viceice]

Can anyone tell my why this code is flagged?

	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) // <--- encoded-compare: use assert.YAMLEq (testifylint)
		} else {
			assert.False(t, ok)
		}
	}

[Antonboom]

@viceice hi

please, provide the full compiled (and flagged) example

[viceice]

https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/integration/repo_test.go#L905

there's the full code

[Antonboom]

I opened the separate issue, thanks 🫡

https://github.com/Antonboom/testifylint/issues/196