Welcome
How did you install golangci-lint?
Official binary
Your feature request related to a problem? Please describe
In Go, it is an extremely common idiom to defer a database transaction rollback immediately after starting it:
tx, err := db.Begin()
if err != nil {
return err
}
defer tx.Rollback() // golangci-lint reports: Error return value is not checked
If the transaction is successfully committed later in the function, the deferred Rollback() will safely do nothing and return an error (like sql.ErrTxDone), which is entirely expected and safe to ignore. Currently, golangci-lint with errcheck flags this pattern. This forces developers to either litter their codebases with //nolint:errcheck directives or manually maintain custom regex exclusions in their configuration files.
Describe the solution you'd like
I would like to propose adding Rollback to the std-error-handling exclusion preset for the errcheck linter.
Since a blanket .*Rollback regex might be too broad and cause unwanted false negatives for non-SQL rollback methods, a stricter regex scoped to database transactions (for example, (\*sql\.Tx)\.Rollback or .*[tT]x\.Rollback) would be ideal.
Because adding this to the default preset would be a breaking change (it would trigger nolintlint errors for users who already have //nolint:errcheck on these lines), this feature request is specifically targeted for evaluation in the next major release where breaking changes are batched.
Describe alternatives you've considered
- Manual exclusion in
.golangci.yml: Adding a custom exclusions rule for .*Rollback is not checked`. This works locally but requires every project to manually configure it.
- Explicitly ignoring the error: Writing
_ = tx.Rollback() instead of tx.Rollback(). This is verbose and less idiomatic for simple defers.
- Using
//nolint:errcheck: Adding the directive to every transaction defer. This creates a lot of noise in the codebase.
- Creating a new opt-in preset: Creating a new preset entirely (like sql-error-handling) that users must explicitly enable, which avoids the breaking change issue entirely but seems weird since there is already a preset for exactly this type of issue.
Additional context
I initially opened a PR to add this to the current release (PR #6407 ).
During the review, @ldez correctly pointed out that this would produce unwanted errors in existing codebases due to nolintlint, and that a broad .*Rollback creates false negatives. They suggested I open this issue so the change can be evaluated properly when the next major version is created.
Since .*Close and .*Flush are already included in the std-error-handling preset for the exact same reasons, adding transaction rollbacks in a future major release feels like a natural and helpful addition.
Supporter
Welcome
How did you install golangci-lint?
Official binary
Your feature request related to a problem? Please describe
In Go, it is an extremely common idiom to defer a database transaction rollback immediately after starting it:
If the transaction is successfully committed later in the function, the deferred
Rollback()will safely do nothing and return an error (likesql.ErrTxDone), which is entirely expected and safe to ignore. Currently, golangci-lint witherrcheckflags this pattern. This forces developers to either litter their codebases with//nolint:errcheckdirectives or manually maintain custom regex exclusions in their configuration files.Describe the solution you'd like
I would like to propose adding
Rollbackto the std-error-handling exclusion preset for the errcheck linter.Since a blanket
.*Rollbackregex might be too broad and cause unwanted false negatives for non-SQL rollback methods, a stricter regex scoped to database transactions (for example,(\*sql\.Tx)\.Rollbackor.*[tT]x\.Rollback) would be ideal.Because adding this to the default preset would be a breaking change (it would trigger
nolintlinterrors for users who already have//nolint:errcheckon these lines), this feature request is specifically targeted for evaluation in the next major release where breaking changes are batched.Describe alternatives you've considered
.golangci.yml: Adding a custom exclusions rule for.*Rollback is not checked`. This works locally but requires every project to manually configure it._ = tx.Rollback()instead oftx.Rollback(). This is verbose and less idiomatic for simple defers.//nolint:errcheck: Adding the directive to every transaction defer. This creates a lot of noise in the codebase.Additional context
I initially opened a PR to add this to the current release (PR #6407 ).
During the review, @ldez correctly pointed out that this would produce unwanted errors in existing codebases due to
nolintlint, and that a broad.*Rollbackcreates false negatives. They suggested I open this issue so the change can be evaluated properly when the next major version is created.Since
.*Closeand.*Flushare already included in thestd-error-handlingpreset for the exact same reasons, adding transaction rollbacks in a future major release feels like a natural and helpful addition.Supporter