wrapcheck icon indicating copy to clipboard operation
wrapcheck copied to clipboard

ignorePackageGlobs doesn't appear to be working

Open Southclaws opened this issue 1 year ago • 3 comments

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:

  • https://github.com/Southclaws/storyden/blob/main/pkg/transports/http/bindings/threads.go#L39
  • https://github.com/Southclaws/storyden/blob/main/pkg/transports/http/bindings/threads.go#L52

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:

  • i.thread_svc.Create: github.com/Southclaws/storyden/pkg/services/thread.Service.Create
  • i.thread_svc.ListAll: github.com/Southclaws/storyden/pkg/services/thread.Service.Create

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!

Southclaws avatar Aug 29 '22 13:08 Southclaws

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 avatar Aug 31 '22 13:08 tomarrell

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

tomarrell avatar Sep 23 '22 13:09 tomarrell

@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

hackerwins avatar Sep 30 '22 10:09 hackerwins

A workaround you could do is until there is a fix

 ignoreSigRegexps:
   - '.*github.com/your-org/project/.*'

yogeshlonkar avatar Oct 04 '22 09:10 yogeshlonkar

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

tomarrell avatar Oct 04 '22 13:10 tomarrell

@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

tomarrell avatar Oct 07 '22 18:10 tomarrell

Awesome stuff, thanks!

Southclaws avatar Oct 10 '22 08:10 Southclaws

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

hackerwins avatar Oct 10 '22 13:10 hackerwins

@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

tomarrell avatar Oct 10 '22 14:10 tomarrell