Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignorePackageGlobs doesn't appear to be working #34

Closed
Southclaws opened this issue Aug 29, 2022 · 9 comments · Fixed by #36
Closed

ignorePackageGlobs doesn't appear to be working #34

Southclaws opened this issue Aug 29, 2022 · 9 comments · Fixed by #36

Comments

@Southclaws
Copy link

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:

  • 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!

@tomarrell
Copy link
Owner

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
Copy link
Owner

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

@hackerwins
Copy link

@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
Copy link

A workaround you could do is until there is a fix

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

@tomarrell
Copy link
Owner

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

@tomarrell
Copy link
Owner

tomarrell commented Oct 7, 2022

@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
Copy link
Author

Awesome stuff, thanks!

@hackerwins
Copy link

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

@tomarrell
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants