validator icon indicating copy to clipboard operation
validator copied to clipboard

go-errorlint suggest to exchange err.(validator.ValidationErrors) to errors.As

Open renta opened this issue 4 years ago • 6 comments

Package version eg. v9, v10:

github.com/go-playground/locales v0.13.0
github.com/go-playground/universal-translator v0.17.0
github.com/go-playground/validator/v10 v10.2.0

Issue, Question or Enhancement:

Golangci-lint (https://github.com/golangci/golangci-lint) comes with a errorlint linter (https://github.com/polyfloyd/go-errorlint). When I switched it on, I got this issue from the linter:

validator/goplayground.go:35:21: type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint) for _, e := range err.(validator.ValidationErrors) {

Is there a way to change err.(validator.ValidationErrors) on errors.As as suggested linter or it's a false-positive issue? Because I could not find examples other than err.(validator.ValidationErrors) in this repo.

Code sample, to showcase or reproduce:

func (v *Validator) Validate(toValidate interface{}) []string {
	err := v.validate.Struct(toValidate)
	var errors []string
	if err != nil {
		for _, e := range err.(validator.ValidationErrors) {
			errors = append(errors, e.Translate(v.translator))
		}
	}
	return errors
}

renta avatar Mar 24 '21 16:03 renta

@deankarn Is any way I could help in solving this issue?

renta avatar May 15 '21 09:05 renta

Yep @renta a PR to change to use errors.As in the example should do it :)

deankarn avatar May 17 '21 01:05 deankarn

@deankarn Sorry for many words, but I'll try to specify issues that I've faced:

  • as linter said we can not use direct type assertion in for loop because err could be wrapped into other one
  • I've added if check that err is a type of validator.ValidationErrors like so:
func (v *Validator) Validate(toValidate interface{}) []string {
	err := v.validate.Struct(toValidate)
	var errs []string
	if err != nil {
		var valErrs validator.ValidationErrors
		if errors.As(err, &valErrs) {
			for _, e := range err.(validator.ValidationErrors) {
				errs = append(errs, e.Translate(v.translator))
			}
		}
	}
	return errs
}

but looping still requires type assertion to explain the compiler that err would be of ValidationErrors type because as it said in Struct method docs:

It returns InvalidValidationError for bad values passed in and nil or ValidationErrors as error otherwise. You will need to assert the error if it's not nil eg. err.(validator.ValidationErrors) to access the array of errors.

So errorlint continue to argue on this code. Is there a way to explicitly convert err from validate.Struct to validator.ValidationErrors without assertion? Or maybe I should ask polyfloyd/go-errorlint to check that errors.As wrap underlying error type assertion, so there could not be issue with error wrapping in code like this?

renta avatar May 17 '21 08:05 renta

Hello @renta,

I came across your issue and found out you don't need type assertions to loop over the slice, you could use it like this:

func (v *Validator) Validate(toValidate interface{}) []string {
   err := v.validate.Struct(toValidate)
   var errs []string
   if err != nil {
   	var valErrs validator.ValidationErrors
   	if errors.As(err, &valErrs) {
   		for _, e := range valErrs {
   			errs = append(errs, e.Translate(v.translator))
   		}
   	}
   }
   return errs
}

As mentioned in the documentation of pkg/errors:

sets target to that error value and returns true.

errors.As(...) could return true and err.(validator.ValidationErrors) could not work if the error was wrapped.

rotta-f avatar Jun 24 '21 13:06 rotta-f

Thank you for the answer @rotta-f ! @deankarn What do you think of using this code example in the Readme of the library?

renta avatar Jun 24 '21 14:06 renta

This code in the golang documentation throws this error:

https://pkg.go.dev/github.com/pkg/errors#section-readme

type stackTracer interface {
        StackTrace() errors.StackTrace
}
if err, ok := err.(stackTracer); ok {
        for _, f := range err.StackTrace() {
                fmt.Printf("%+s:%d\n", f, f)
        }
}

Do you have a better suggestion for writing this function?

How can I suppress this error if I don't agree with it?

I suppose like this?

type stackTracer interface {
        StackTrace() errors.StackTrace
}
var stackErr stackTracer
if errors.As(err, &stackTracer) {
        for _, f := range stackErr.StackTrace() {
                fmt.Printf("%+s:%d\n", f, f)
        }
}

ryanpeach avatar Nov 22 '22 20:11 ryanpeach