go-ruleguard icon indicating copy to clipboard operation
go-ruleguard copied to clipboard

Shouldn't `github.com/quasilinte/go-ruleguard/ruleguard` import `github.com/quasilyte/go-ruleguard/dsl`? 🤔

Open Eun opened this issue 3 years ago • 7 comments

It is stated in the README.md that it is required to run go get -v -u github.com/quasilyte/go-ruleguard/dsl before the use of ruleguard.

Programs that utilize ruleguard using the github.com/quasilinte/go-ruleguard/ruleguard package (like go-critic or golangci-lint) certainly not called the previous mentioned go get command. Therefore it would be necessary for users to run the go get by themself before running one of these programs. In a dockerzied version of these programs this becomes more difficult.

A solution would be that those programs import github.com/quasilyte/go-ruleguard/dsl, however I feel it would be better that github.com/quasilinte/go-ruleguard/ruleguard should import the dsl package by itself and therefore the programs have everything ready by design.

Eun avatar Apr 14 '21 11:04 Eun

Hmm. I need to think about it.

I don't want every user to have ruleguard package as a dependency (it's not lightweight). But every user will need dsl package as a go.mod dependency (it's super lightweight and dependency-free). This is why we have 2 separate modules.

But if someone is already using ruleguard, maybe it's OK to let dsl through as well.

@cristaloleg do you have any thoughts on this?

quasilyte avatar Apr 22 '21 10:04 quasilyte

We can add go.mod in dsl so this will became another module.

cristaloleg avatar Apr 22 '21 10:04 cristaloleg

As far as I can see it totally makes sense: Users do not need the ruleguard module in order to use dsl module. But users working with the ruleguard module certainly want the dsl module.

I don’t get @cristaloleg response since a go.mod file already exists in dsl.

Eun avatar Apr 23 '21 07:04 Eun

I came here to +1. We spent hours trying to figure out why golangci-lint was not running ruleguard. This is why.

thockin avatar Aug 20 '21 05:08 thockin

I think it won't work for golangci-lint to include dsl as a (indirect or direct) dependency. We're building golangci-lint binary in its own module context. Sure, it will have dsl package and it would be possible to use it there.

But as it's no GOPATH era anymore, Go modules are separate contexts.

When you're checking a project with golangci-lint, package lookup depends on that very module context, so it will have to include dsl package in its dependencies. So, while I see the logic in the original message, it won't solve the underlying problem. ruleguard "loads" the rules at startup. This is why it's different from most linters - it allows rules changes without linter binary recompilation. That means we're doing things at run time. To resolve the rules.go file, it needs the dsl package. I'm using standard go/* packages to do this, they're having their own opinion on how to do this job and with modules they would expect dsl to be a current module context dependency. There are ways to hack around this, but they usually break somewhere as well (I tried several times to provide a working embedded "default" dsl package).

Any technical (go/*) hints are welcome. If anyone can implement it so all the tests are passing, I would merge that patch and express my gratitude. :)

quasilyte avatar Oct 23 '21 11:10 quasilyte

Is there any other way to use ruleguard check my project code without modifying the go.mod file in the project. Because I found that every time I run it, the dsl package is automatically added to go.mod? @quasilyte

toddwyl avatar May 05 '23 11:05 toddwyl

https://github.com/quasilyte/go-ruleguard/issues/253#issuecomment-977049595

thockin avatar May 05 '23 21:05 thockin