search

OpenPeeDeeP / depguard

Go linter that checks if package imports are in a list of acceptable packages.
GNU General Public License v3.0
147 stars 16 forks source link

Intermittent incorrect "`xyz` is in the denylist" errors #25

Closed bmhatfield closed 1 year ago

bmhatfield commented 2 years ago

Hi there,

We're using depguard as part of golangci-lint (v1.47.3). We've been getting intermittent false positives - given the same code and same rules, re-running multiple times will produce different results. Each of the false positives do not match any of our actual rules (of which we have 3). The false positives themselves change - they don't seem to be centered around any particular package in our repository. True positives (ie, a rule that has legitimately been broken) seem reliable.

I haven't been able to track down if any particular condition seems to cause this false positive behavior. Clearing the go and golangci-lint caches doesn't seem to influence the behavior. It seems to have become more prevalent on more recent versions of Go, especially since 1.18 but increasing in 1.19. It occurs both locally (MacOS) and in CI (CircleCI, Alpine Linux container).

I'm painfully aware that this issue description is light on technical detail, because I haven't been able to diagnose this issue so far. I've been delaying filing it for some time because of this, but nobody else seems to have filed this as an issue, so I thought I would at least start the conversation. Is anyone else seeing this?

bmhatfield commented 2 years ago

It looks like this issue mentions the itermittent denylisting in the body, however, we are not observing the NPEs discussed in that issue.

ldez commented 2 years ago

hello @bmhatfield, I'm interested in a reproducible use case, have you something to share?

jerome-laforge commented 2 years ago

Is anyone else seeing this?

I am facing with the same issue. And as @bmhatfield, this issue is more prevalent with Go 1.19 on my local machine (Linux) and in our CI (on-premise gitlab).

bmhatfield commented 2 years ago

hello @bmhatfield, I'm interested in a reproducible use case, have you something to share?

Here's an example snippet of our config, though I imagine anything would do this. Because this is intermittent / random, I don't really have a reliable reproduce case beyond golangci-lint, go 1.19, and maybe linux-based containers.

linters-settings:
  depguard:
    list-type: denylist
    packages:
      - log
      - "github.com/gogo/protobuf"
    packages-with-error-message:
      - log: "Use shared/logger"
      - github.com/gogo/protobuf: "Use golang/protobuf"
    include-go-root: true
jerome-laforge commented 2 years ago

As workaround, we have disabled the depguard linter into our golangci-linter configuration.

ldez commented 2 years ago

Disabling a linter is not really a workaround for this linter :wink:

You just stopped to use the linter.

jerome-laforge commented 2 years ago

You 're right ;) It just allow us to have reliable CI ;)

dixonwille commented 2 years ago

Thanks for feedback. I have a WIP V2 branch that is very early in development (past few hours). I think the intermittent issues comes with golangci-lint making concurrent calls with depguard's single configuration (which it was not designed to handle back in golangci-lint v1.4). I believe these concurrent issues are what you are seeing.

dixonwille commented 2 years ago

So I have a working beta v2.0.0-beta.2

go install github.com/OpenPeeDeeP/depguard/v2/cmd/depguard@v2.0.0-beta.2

I have tested on a few repositories. But would like others to try it out to.

bmhatfield commented 2 years ago

It looks like https://github.com/OpenPeeDeeP/depguard/pull/23 has been pulled in by golangci-lint, which appears to resolve this problem. We did not realize that the main branch had previously been updated and then pulled into golangci-lint. We'll test for a couple days and close this issue if it's working!

dixonwille commented 1 year ago

I have just released version 2