x/tools/go/analysis: show invalid usage of errors.As()

[guettli]

Is your feature request related to a problem? Please describe.

This fails:

package main

import (
	"errors"
	"fmt"
	"os"
)

type CustomError struct {
	Msg string
}

func (e CustomError) Error() string {
	return e.Msg
}

func main() {
	// You can replace this with your actual function which returns an error
	err := fmt.Errorf("this is a wrapped error: %w", &CustomError{Msg: "custom error"})

	customErr := CustomError{}
	if errors.As(err, &customErr) {
		// Here you can handle your custom error
		fmt.Println("This is a custom error:", customErr.Msg)
	} else {
		// Here you handle all other errors
		fmt.Println(":-(  sad - not exptected")
		fmt.Println(err)
		os.Exit(1)
	}
}

Describe the solution you'd like

I would like to get a warning if I provide a pointer to a struct to errors.As().

Changing the line to this works:

customErr := &CustomError{}

That's easy to get wrong, and it takes some time to find the error.

Describe alternatives you've considered

I considered the alternative: don't make mistakes. But sorry, I am not perfect.

[hyangah]

There is errorsas analyzer which is enabled by default. But this check doesn't do what you want.

A different way to fix the issue in the example program is

err := fmt.Errorf("... %w", CustomError{Msg: "custom error"})

It's hard to tell if it's a misuse or not without know how err was constructed, so I am afraid if this feature needs more expensive approaches like whole program analysis or dynamic analysis to work. cc @timothy-king @adonovan

[adonovan]

The mistake, I think, is here:

err := fmt.Errorf("this is a wrapped error: %w", &CustomError{Msg: "custom error"})

CustomError{...} is an error value, and &CustomError{} is a pointer that also happens to implement the error interface but isn't how this type is supposed to be used.

One possibility is to change the printf checker to report %w applied to a value of type *E where E implements error. That would be something we could easily evaluate using the new metrics pipeline. @timothy-king

[guettli]

...
One possibility is to change the printf checker to report %w applied to a value of type *E where E implements error. That would be something we could easily evaluate using the new metrics pipeline. @timothy-king

Wow, this would be great. If you are new to Go, then a warning in the IDE would really help. Thank you.

[timothy-king]

One possibility is to change the printf checker to report %w applied to a value of type *E where E implements error.

Just to clarify *E is also an error. I am not sure we have a solid basis for saying E is the only error implementation based on the method signature. In the example, var customErr *CustomError would be valid, and print the expected result. So such a vet check will have theoretical false positives. If we go forward with this in vet, we may want to rope in the author of errors.As for their opinion about the validity of such a rule. (Also this will need a proposal IMO.)

That would be something we could easily evaluate using the new metrics pipeline. @timothy-king

Once I am back, I can look at this pretty quickly. It would be good to have data.