Warn when calling reflect.DeepEqual with errors

[zombiezen]

A common antipattern I see in production code is using reflect.DeepEqual on errors in tests, which results in brittle tests. Even just adding more diagnostic information to the error can break it. The more proper thing to do is either:

1. Check whether a substring is present in the error message (not great, but more correct).
2. Use something like Dave Cheney's errors package to make the system-under-test provide more useful errors (great! because it's better API design anyway).

It would be helpful to have a lint check that discourages this by warning when either parameter to reflect.DeepEqual is of error type.

One additional point, quoting @dsnet:

If they are structs that contain a field of an error type, no warning is given. While I would consider using deep equal in that case a code smell, there are legitimate uses of it when it is used to compare sentinel errors.

(This was originally reported as golang/go#16748.)

[tariq1890]

@zombiezen is this still up for grabs? I can work on it if this is still needed :)

[zombiezen]

@andybons for triage

[andybons]

This appears to be out of scope.

I don't see a mention of this rule in either document.

Please note this is not the forum to discuss edits to CodeReviewComments. If you wish to propose changes to it please send a mail to golang-nuts.

[tariq1890]

Thanks for your response :).

[mvdan]

Thank you for submitting this issue! As per golang/go#38968, we are freezing and deprecating golint. There's no drop-in replacement to golint per se, but you should find that Staticcheck works well in encouraging good Go code, much like golint did in the past, since it also includes style checks. There's always gofmt and go vet too, of course.

If you would like to contribute further, I'd encourage you to engage Staticcheck's issue tracker or look at vet's open issues, as they are both actively maintained. If you have an idea that doesn't fit into either of those tools, you could look at other Go linters, or write your own - these days it's fairly straightforward with go/analysis.

To help avoid confusion, I'm closing all issues before we freeze the repository. If you have any feedback, you can leave a comment on the proposal thread where it was decided to deprecate golint - though note that the proposal has been accepted for nearly a year. Thanks!