OpenPeeDeeP / depguard

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

Panic: depguard: package \"rabbitmq\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: index out of range [-1] #74

Closed Creatone closed 9 months ago

Creatone commented 10 months ago

Got this using golangci-lint v1.55.1

level=error msg="[runner] Panic: depguard: package \"rabbitmq\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: index out of range [-1]: goroutine 4592 [running]:\nruntime/debug.Stack()\n\truntime/debug/stack.go:24 +0x5e\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:109 +0x27c\npanic({0x10c4f60?, 0xc0133c1158?})\n\truntime/panic.go:914 +0x21f\ngithub.com/OpenPeeDeeP/depguard/v2.strInPrefixList({0xc00149af01, 0xb}, {0xc000dc30e0?, 0x1, 0x1?})\n\tgithub.com/OpenPeeDeeP/depguard/v2@v2.1.0/settings.go:203 +0x125\ngithub.com/OpenPeeDeeP/depguard/v2.(*list).importAllowed(0xc00088f7a0, {0xc00149af01?, 0xb?})\n\tgithub.com/OpenPeeDeeP/depguard/v2@v2.1.0/settings.go:120 +0x7b\ngithub.com/OpenPeeDeeP/depguard/v2.linterSettings.run({0xc000dc2fa0, 0x2, 0x2?}, 0xc00c571ad0)\n\tgithub.com/OpenPeeDeeP/depguard/v2@v2.1.0/depguard.go:75 +0x338\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc001348000)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:195 +0x9d6\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:113 +0x17\ngithub.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc000389c70, {0x113b07f, 0x8}, 0xc0018ef748)\n\tgithub.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x44\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc000e7a000?)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:112 +0x7a\ngithub.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc001348000)\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xa8\ncreated by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze in goroutine 87\n\tgithub.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x205\n"
dixonwille commented 10 months ago

Can you post your config as well (Just the depguard one).

The following is where the panic occured, but ioc is package in your config. This can only be -1 if length of ioc is 0 meaning it was an empty string.

https://github.com/OpenPeeDeeP/depguard/blob/0d4cbc4e3f1b6e040fdc3aac915aaef3093a6a0e/settings.go#L203C5-L203C20

Creatone commented 10 months ago
 depguard:
    rules:
      Main:
        files:
        - $all
        - "!$test"
        allow:
        - $gostd
        - github.com/sirupsen/logrus
        - github.com/streadway/amqp
        - github.com/rabbitmq/amqp091-go
        deny:
          reflect: Please don't use reflect package
      Test:
        files:
        - $test
        allow:
        - $gostd
        - github.com/stretchr/testify
        deny:
          reflect: Please don't use reflect package
dixonwille commented 10 months ago

@Creatone thanks! I will take a look as soon as I can. I can't see anything wrong with the configs right off the top of my head.

dixonwille commented 10 months ago

@Creatone using your configurations and running against depguard repository (the tagged v2.1.0), I am unable to reproduce the panic.

Is there a repository I can reproduce your configuration against? What OS are you running? Any other bit of information like, did it work partially for some packages but not all packages.

oliverpool commented 9 months ago

I was able to reproduce this bug and find a workaround.

The problem lies within golangci-run, which expects a deny list, because golangci.yml keys can't have a dot (viper limitation)

        deny:
          - pkg: "github.com/sirupsen/logrus"
            desc: not allowed
          - pkg: "github.com/pkg/errors"
            desc: Should be replaced by standard lib errors package

Whereas the config for depguard standalone is a deny map:

  deny:
    reflect: Please don't use reflect package

BTW the last README example uses a list syntax:

Main:
  deny:
  - github.com/OpenPeeDeeP/depguard$
oliverpool commented 9 months ago

I propose to add a comment to the golangci documentation: https://github.com/golangci/golangci-lint/pull/4227

oliverpool commented 9 months ago

Since the maintainer of golangci refuses to draw attention to this discrepancy, it might be wise to add it to your README.

Creatone commented 9 months ago

@oliverpool thaaanks @dixonwille it's working now :)

dixonwille commented 9 months ago

Since the maintainer of golangci refuses to draw attention to this discrepancy, it might be wise to add it to your README.

Sounds good to me.