Proposal: ignore returning value of specify package's function

[sanposhiho]

Hello 👋
Thank you for developing such a great tool!

I'm using a library that wraps and manages errors like below.

https://github.com/morikuni/failure
https://github.com/pkg/errors

Currently, wrapcheck also reports for these librarie's function.

example

error returned from external package is unwrapped (wrapcheck)
                return nil, errors.Wrap(err, "failed to do something")
                                  ^

This is, of course, the correct behavior. But I don't need these report from wrapcheck.
So, I ignore these reports with golangci-lint's exclude-rules like below.

issues:
  exclude-rules:
   - source: "errors"
      linters:
        - wrapcheck
    - source: "failure"
      linters:
        - wrapcheck

https://golangci-lint.run/usage/configuration/

It would be very helpful to be able to configure these settings on the wrapcheck side.

(This proposal may be similar to #2)

Thanks

[jkowalski]

To second that - github.com/pkg/errors is very commonly used (imported by over 100K packages). It would be really great to support this either by as a default exception or as configurable package to ignore.

Right now my codebase can't upgrade from golangci-lint 1.37 to 1.39 because of recent change in wrapcheck which started flagging return errors.Wrap(err, "..."). I would really appreciate if there was a way to move forward without disabling this linter, which I really like.

[tomarrell]

Thanks a lot for the error. That's a very good point about pkg/errors, it was not my intention to break it.

I also agree that making wrapcheck signature checks configurable. I will work on this and push a new build.

[tomarrell]

In order to pre-emptively prevent other having the same issue. I've forcefully retagged v1.0.0 which was released with golangci-lint v1.39 to point to a version which fixes this for github.com/pkg/errors at the very least.

I will author a new version supporting configurable params shortly.

[hsblhsn]

I am having the same issue with github.com/rotisserie/eris.Wrap().

[jkowalski]

Wondering if it would make sense to recognize function signatures like

func (error,string) error

and

func (errror,string,...interface{}) error

regardless of package they come from? Basically If a function takes error as its first parameter + a string and returns an error it's most likely wrapping it for the purpose of this linter.

[tomarrell]

Hey @hsblhsn would you be able to confirm an issue with a fresh copy of golangci-lint and specifically with the v1.0.0 tag of wrapcheck at commit hash 1483c06

This should at least ignore anything that includes .Wrap(.

[tomarrell]

@jkowalski that sounds like quite a good suggestion to cover a general use-case. I'll look into implementing this ASAP and ship a new release.

[Aneurysm9]

In order to pre-emptively prevent other having the same issue. I've forcefully retagged v1.0.0 which was released with golangci-lint v1.39 to point to a version which fixes this for github.com/pkg/errors at the very least.

I will author a new version supporting configurable params shortly.

This has caused issues for those of us who had previously pulled v1.0.0 and are now seeing conflicts in go.sum. Can you please restore v1.0.0 and tag a v1.0.1 instead? See #10

[tomarrell]

@Aneurysm9 I've restored the tag to point to the original hash and released v1.1.0 with the more flexible default cases to ignore.

I'll release another version soon with some updates to enable even more flexibility.

[dackroyd]

Also seeing issues with Testify mocks using args.Error(n) in many cases with an args.Get(0).(<type>)

Neither of these cases flag issues:

func (m *myMock) Value() (interface{}, error) {
	args := m.Called()
	return args.Get(0), args.Error(1)
}

func (m *myMock) Name() (string, error) {
	args := m.Called()
	return args.String(), args.Error(1)
}

When there is a type assertion the args.Error(1) gets flagged:

func (m *myMock) Value() (interface{}, error) {
	args := m.Called()
	return args.Get(0).(struct{}), args.Error(1)
}

error returned from external package is unwrapped (wrapcheck)
        return args.Get(0).(struct{}), args.Error(1)

This is true for cases other than struct{} also

[krishicks]

👋 To add to the above examples, another that I've come across is when the error cannot be wrapped, e.g. in grpc-go, service handlers are meant to return a status.Error:

// Package status implements errors returned by gRPC.  These errors are
// serialized and transmitted on the wire between server and client, and allow
// for additional data to be transmitted via the Details field in the status
// proto.  gRPC service handlers should return an error created by this
// package, and gRPC clients should expect a corresponding error to be
// returned from the RPC call.

Presently wrapcheck fails on lines like

return nil, status.Errorf(codes.Unknown, "some error happened: %v", err)

But because this is the grpc-go service handler contract, it should not be changed.

[tomarrell]

Hey @krishicks, thanks for pointing that out. I've made the default ignore rules a little more flexible to cover this case. The next release I ship for this will include the ability to customise the signatures to ignore.

But in the meantime, I hope this helps a little. I'll try get this into golangci-lint v1.40.

[tomarrell]

G'day folks, v2 has been shipped. It allows you to define custom ignore signature substrings. Please check it out and provide any feedback.

https://github.com/tomarrell/wrapcheck/releases/tag/v2.1.0

I'm going to consider this issue closed. Please report anything further in a new issue so that it can be more easily tracked.