tomarrell / wrapcheck

A Go linter to check that errors from external packages are wrapped
https://blog.tomarrell.com/post/introducing_wrapcheck_linter_for_go
MIT License
291 stars 26 forks source link

ignorePackageGlobs doesn't appear to be working #34

Closed Southclaws closed 1 year ago

Southclaws commented 1 year ago

Hey, great tool! This has been really useful to track down places I can apply some new error context packages I've been working on to provide additional structured information to errors.

After applying error contexts across our project I noticed a few places where it was unnecessary so I decided to disable reporting of imports from local packages (/pkg, etc)

However, maybe I'm misunderstanding this config option, but it doesn't appear to do what I expected.

Here's the configuration file: https://github.com/Southclaws/storyden/blob/main/.golangci.yml#L28

And here are the offending lines:

I assumed that a ignorePackageGlobs value of github.com/Southclaws/storyden/* would ignore these two lines as they are functions that are imported from within this pattern:

The actual output from running golangci-lint run:

storyden on  main [!] via 🐹 v1.18.3 on ☁️  (eu-central-1) 
❯ golangci-lint run

pkg/transports/http/bindings/threads.go:39:10: error returned from interface method should be wrapped: sig: func (github.com/Southclaws/storyden/pkg/services/thread.Service).Create(ctx context.Context, title string, body string, authorID github.com/Southclaws/storyden/pkg/resources/account.AccountID, categoryID github.com/Southclaws/storyden/pkg/resources/category.CategoryID, tags []string) (*github.com/Southclaws/storyden/pkg/resources/thread.Thread, error) (wrapcheck)
                return err
                       ^
pkg/transports/http/bindings/threads.go:52:10: error returned from interface method should be wrapped: sig: func (github.com/Southclaws/storyden/pkg/services/thread.Service).ListAll(ctx context.Context, before time.Time, max int) ([]*github.com/Southclaws/storyden/pkg/resources/thread.Thread, error) (wrapcheck)
                return err
                       ^

I wondered if it was golangci lint just using an outdated version but this occurs on the latest standalone version of wrapcheck too.

I tried with a .wrapcheck.yaml file with:

ignorePackageGlobs:
  - github.com/Southclaws/storyden/*
  - github.com/Southclaws/storyden/pkg/*
  - pkg/*
  - ./pkg/*

but I can't seem to get any patterns to ignore correctly.

Thanks!

tomarrell commented 1 year ago

G'day @Southclaws, cheers for the report. I've done some digging, and have a suggestion.

Would you be able to try the config below?

ignorePackageGlobs:
  - "*github.com/Southclaws/storyden/pkg*"

I suspect it may be because of the glob matching when it comes to leading characters, in the tests there is nothing but a space before the package name in the signature, however in yours there is a parenthesis (.

I agree that this isn't a particularly good experience, and I've found some usability improvements which could be made to the glob matching which I'll work on, but in the meantime this should get you moving forward.

Cheers

tomarrell commented 1 year ago

@Southclaws, would you be able to try out the above?

hackerwins commented 1 year ago

@tomarrell I also had the same problem. In my case, adding * back and forth doesn't fix the problem.

golangci.yml: https://github.com/yorkie-team/yorkie/pull/412/files#diff-6179837f7df53a6f05c522b6b7bb566d484d5465d9894fb04910dd08bb40dcc9R36

Error: server/packs/pushpull.go:181:41: error returned from interface method should be wrapped: sig: func (github.com/yorkie-team/yorkie/server/backend/database.Database).FindChangeInfosBetweenServerSeqs(ctx context.Context, docID github.com/yorkie-team/yorkie/api/types.ID, from int64, to int64) ([]*github.com/yorkie-team/yorkie/server/backend/database.ChangeInfo, error) (wrapcheck) https://github.com/yorkie-team/yorkie/actions/runs/3158212376/jobs/5139990031#step:6:6

yogeshlonkar commented 1 year ago

A workaround you could do is until there is a fix

 ignoreSigRegexps:
   - '.*github.com/your-org/project/.*'
tomarrell commented 1 year ago

Thanks for the info @hackerwins, I'll look at reproducing your case and pushing a fix soon.

tomarrell commented 1 year ago

@hackerwins @Southclaws I've just pushed release v2.7.0 which makes this configuration value apply to functions called through interfaces, hopefully making it a bit more intuitive. Let me know if you run into any further issues.

Cheers

Southclaws commented 1 year ago

Awesome stuff, thanks!

hackerwins commented 1 year ago

@tomarrell Thanks for the update. I am using wrapcheck with golangci-lint. I will update it when it is released.

tomarrell commented 1 year ago

@hackerwins no worries, it should be out in the next release. The update PR has already been merged into golangci-lint https://github.com/golangci/golangci-lint/pull/3287