dgryski/semgrep-go

The ruleguard rules seem broken with newer versions

ainar-g opened this issue · 4 comments

Using rulequard v0.3.0 I get this message:

$ ruleguard -c 0 --rules "${HOME}/dev/semgrep-go/ruleguard.rules.go" ./...
ruleguard: load rules: parse rules file: only functions with a single non-void results are supported

While with ruleguard v0.2.1 I get a lot of messages like this:

$ ruleguard -c 0 --rules "${HOME}/dev/semgrep-go/ruleguard.rules.go" ./...
ruleguard: load rules: parse rules file: /home/ainar/dev/semgrep-go/ruleguard.rules.go:415: m["mu1"].Text == m["mu2"].Text is not a valid filter expression

v0.3.0 broke some things. I'm planning to help to fix the code this week (unless someone does it before I get to it).

Changes required to the ruleguard files:

  1. Change the dsl import from github.com/quasilyte/go-ruleguard/dsl/fluent => github.com/quasilyte/go-ruleguard/dsl
  2. Use dsl.Matcher instead of fluent.Matcher

That should be enough to make the rules work with a newer version.

To make it possible to import this rule set from other rule sets without a copy/paste, a Bundle variable should be added:

var Bundle = dsl.Bundle{}

There should also be a go module associated with it. More info here (short summary) and here (docs).

Users of github.com/dgryski/semgrep-go then could do something like this:

package gorules

import (
	"github.com/quasilyte/go-ruleguard/dsl"
	dgryskirules "github.com/dgryski/semgrep-go"
)

func init() {
	// Imported rules will have a "dgryski" prefix.
	dsl.ImportRules("dgryski", dgryskirules.Bundle)
}

If ImportRules() called with empty prefix, no prefix will be added. It's prone to collisions if you'll have your own rules in that file. If you're only using external rules, it's fine to use an empty prefix.

v0.2.1 doesn't have a patch required to write .Text filters on both sides of a binary operator.

@quasilyte That works for me with v0.3.0 for now, thanks!

Sorry for the inconvenience. I suppose we could avoid this problem if I made a v0.2.2 release before any breaking changes, this way your (2) option would work and v0.3.0 transition would be easier.

I'll try to come up with a plan for v1.0.0 to start respecting the backward-compatibility, but there might be one or two things that need to change before that.

I was planning on fixing the rules when 0.3.0 was out. I must've missed the release. I'll fix them up and add a few I have on my backlog to keep it up to date with the semgrep rules.