Skip to content

Add error-compare checker: detect err.Error() in assertions#294

Open
mmorel-35 wants to merge 4 commits into
Antonboom:masterfrom
mmorel-35:error-compare
Open

Add error-compare checker: detect err.Error() in assertions#294
mmorel-35 wants to merge 4 commits into
Antonboom:masterfrom
mmorel-35:error-compare

Conversation

@mmorel-35

@mmorel-35 mmorel-35 commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Implements the error-compare checker (issue #47). Using err.Error() in testify assertions compares error string representations — fragile, ignores the error chain, and bypasses dedicated testify error APIs.

Detected patterns

Bad Suggested fix Autofix
assert.Contains(t, err.Error(), "substr") assert.ErrorContains(t, err, "substr")
assert.Contains(t, err.Error(), sentinel.Error()) assert.ErrorIs(t, err, sentinel)
assert.Equal(t, err.Error(), "msg") assert.EqualError(t, err, "msg")
assert.Equal(t, "msg", err.Error()) assert.EqualError(t, err, "msg") ✅ (args swapped)
assert.Equal(t, err, sentinel) (both implement error) assert.ErrorIs(t, err, sentinel)
assert.NotEqual(t, err, sentinel) (both implement error) assert.NotErrorIs(t, err, sentinel)

All f-suffixed variants are handled automatically.

Implementation

  • helpers_error.go: Added isErrErrorCall() (detects x.Error() where x implements error) and implementsErrorIface() (type-based, not restricted to error-typed variables).
  • error_compare.go: New RegularChecker handling Contains, Equal, and NotEqual cases. Guards against overlap with error-nil via isNil check on Equal(t, err, nil). No autofix is provided for the two-error Equal/NotEqual cases because ErrorIs/NotErrorIs are not symmetric and argument order cannot be safely inferred from types alone.
  • Registry: Inserted between contains and error-nil, enabled by default.
  • Testgen: Separate errored and golden templates; Contains and Equal(err.Error(), ...) cases are autofixed, while Equal/NotEqual two-error cases remain unchanged in the golden output.
  • README: Checker documented and added to the summary table.

Also reverts an unrelated change from a previous session that added a "redundant assertion" detection to require-error.

Closes #47

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Copilot AI added a commit to mmorel-35/testifylint that referenced this pull request May 28, 2026
mmorel-35 pushed a commit to mmorel-35/testifylint that referenced this pull request May 28, 2026
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

@ccoVeille ccoVeille left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix of what I reported you

@Antonboom Antonboom added the llm-based LLM shit label Jun 22, 2026
@thespags

Copy link
Copy Markdown

@Antonboom , some numbers
Repo:

Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Go                            1333          48443          22319         219186

Usages

  • require.EqualError 153
  • require.ErrorIs 97
  • require.ErrorContains 398

I agree that I'd prefer EqualError or ErrorIs over ErrorContains, but for a large org, I'm okay allowing engineers to use ErrorContains.

Regardless of preference the err.Error() check would be great. A good compromise could be a follow-up to enable/disable banning require.ErrorContains. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llm-based LLM shit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error-compare: detect contains of err.Error()

4 participants