lint: want pragma to inhibit the linter prohibiting direct error casts

[andreimatei]

For err.(MyErrType), the linter says invalid direct cast on error object, but sometimes there are legitimate reasons to do so. For example, when implementing errors.RegisterLeaf{Encoder,Decoder} functions, you know what type the err argument has.

People are working around the linter through appalling filtering of the linter's output - https://github.com/cockroachdb/cockroach/blob/933264f9feac406d54154443204b0ef5356bbb83/pkg/testutils/lint/lint_test.go#L2103-L2104 I want a //nolint pragma at the cast site instead.

This linter is written using the AST traversal framework. I have reason to believe it can be taught to look for such a comment, but I don't know how to do it.

cc @knz @ajwerner

Jira issue: CRDB-8746

[ajwerner]

Try something like this:

--- a/pkg/testutils/lint/passes/errcmp/errcmp.go
+++ b/pkg/testutils/lint/passes/errcmp/errcmp.go
@@ -18,6 +18,7 @@ import (
        "go/types"
        "strings"

+       "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/passesutil"
        "golang.org/x/tools/go/analysis"
        "golang.org/x/tools/go/analysis/passes/inspect"
        "golang.org/x/tools/go/ast/inspector"
@@ -28,10 +29,12 @@ const Doc = `check for comparison of error objects`

 var errorType = types.Universe.Lookup("error").Type()

+const name = "errcmp"
+
 // Analyzer checks for usage of errors.Is instead of direct ==/!=
 // comparisons.
 var Analyzer = &analysis.Analyzer{
-       Name:     "errcmp",
+       Name:     name,
        Doc:      Doc,
        Requires: []*analysis.Analyzer{inspect.Analyzer},
        Run:      run,
@@ -77,9 +80,18 @@ func run(pass *analysis.Pass) (interface{}, error) {
        return nil, nil
 }

+func maybeReportf(
+       pass *analysis.Pass, n ast.Node, pos token.Pos, format string, args ...interface{},
+) {
+       if passesutil.HasNolintComment(pass, n, name) {
+               return
+       }
+       pass.Reportf(pos, format, args...)
+}
+
 func checkErrSwitch(pass *analysis.Pass, s *ast.SwitchStmt) {
        if pass.TypesInfo.Types[s.Tag].Type == errorType {
-               pass.Reportf(s.Switch, escNl(`invalid direct comparison of error object
+               maybeReportf(pass, s, s.Switch, escNl(`invalid direct comparison of error object
 Tip:
    switch err { case errRef:...
 -> switch { case errors.Is(err, errRef): ...
@@ -89,7 +101,7 @@ Tip:

 func checkErrCast(pass *analysis.Pass, texpr *ast.TypeAssertExpr) {
        if pass.TypesInfo.Types[texpr.X].Type == errorType {
-               pass.Reportf(texpr.Lparen, escNl(`invalid direct cast on error object
+               maybeReportf(pass, texpr, texpr.Lparen, escNl(`invalid direct cast on error object
 Alternatives:
    if _, ok := err.(*T); ok        ->   if errors.HasType(err, (*T)(nil))
    if _, ok := err.(I); ok         ->   if errors.HasInterface(err, (*I)(nil))
@@ -123,7 +135,7 @@ func checkErrCmp(pass *analysis.Pass, binaryExpr *ast.BinaryExpr) {
                                return
                        }

-                       pass.Reportf(binaryExpr.OpPos, escNl(`use errors.Is instead of a direct comparison
+                       maybeReportf(pass, binaryExpr, binaryExpr.OpPos, escNl(`use errors.Is instead of a direct comparison
 For example:
    if errors.Is(err, errMyOwnErrReference) {

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!