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

all: forbid unnamed rule groups func _() #148

Merged
merged 1 commit into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,14 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func dupSubExpr(m fluent.Matcher) {
m.Match(`$x || $x`,
`$x && $x`).
Where(m["x"].Pure).
Report(`suspicious identical LHS and RHS`)
}

func boolExprSimplify(m fluent.Matcher) {
m.Match(`!($x != $y)`).Suggest(`$x == $y`)
m.Match(`!($x == $y)`).Suggest(`$x != $y`)
}
Expand Down Expand Up @@ -109,9 +111,11 @@ example.go:5:10: !(v1 != v2)

It automatically inserts `Report("$$")` into the specified pattern.

For named functions (rule groups) you can use `-debug-group <name>` flag to see explanations
You can use `-debug-group <name>` flag to see explanations
on why some rules rejected the match (e.g. which `Where()` condition failed).

The `-e` generated rule will have `e` name, so it can be debugged as well.

## How does it work?

`ruleguard` parses [gorules](docs/gorules.md) (e.g. `rules.go`) during the start to load the rule set.
Expand Down
4 changes: 2 additions & 2 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var (
func init() {
Analyzer.Flags.StringVar(&flagRules, "rules", "", "comma-separated list of gorule file paths")
Analyzer.Flags.StringVar(&flagE, "e", "", "execute a single rule from a given string")
Analyzer.Flags.StringVar(&flagDebug, "debug-group", "", "enable debug for the specified named rules group")
Analyzer.Flags.StringVar(&flagDebug, "debug-group", "", "enable debug for the specified function")
}

type parseRulesResult struct {
Expand Down Expand Up @@ -119,7 +119,7 @@ func readRules() (*parseRulesResult, error) {
ruleText := fmt.Sprintf(`
package gorules
import "github.com/quasilyte/go-ruleguard/dsl/fluent"
func _(m fluent.Matcher) {
func e(m fluent.Matcher) {
%s.Report("$$")
}`,
flagE)
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/extra/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
// We don't want to suggest int64(x) if x is already int64,
// this is why 2 rules are needed.
// Maybe there will be a way to group these 2 together in
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/filtertest/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Import(`github.com/quasilyte/go-ruleguard/analyzer/testdata/src/filtertest/foolib`)

m.Match(`typeTest($x, "contains time.Time")`).
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/gocritic/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Match(`$x = $x`).Report(`suspicious self-assignment in $$`)

m.Match(`$tmp := $x; $x = $y; $y = $tmp`).
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/golint/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Match(`errors.New(fmt.Sprintf($*_))`).
Report(`should replace error.New(fmt.Sprintf(...)) with fmt.Errorf(...)`)
m.Match(`t.Error(fmt.Sprintf($*_))`).
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/namedtype/nested/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Import(`namedtype/x/nested`)
m.Import(`extra`)

Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/namedtype/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Import(`namedtype/x/nested`)

m.Match(`sink = &$t`).
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/revive/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Match(`runtime.GC()`).Report(`explicit call to GC`)

m.Match(`$x = atomic.AddInt32(&$x, $_)`,
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/suggest/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
m.Match(`($a) || ($b)`).Suggest(`$a || $b`)
}
2 changes: 1 addition & 1 deletion analyzer/testdata/src/testvendored/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func testRules(m fluent.Matcher) {
// Test a vendored dependency can be imported successfully and used in a Where statement.
// Otherwise, the rules semantics are not important.

Expand Down
6 changes: 2 additions & 4 deletions docs/gorules.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,14 @@ package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
func regexpMust(m fluent.Matcher) {
m.Match(`regexp.Compile($pat)`,
`regexp.CompilePOSIX($pat)`).
Where(m["pat"].Const).
Report(`can use MustCompile for const patterns`)
}
```

A rule group that has `_` function name is called **anonymous**. You can have as much anonymous groups as you like.

A `Report` argument string can use `$<varname>` notation to interpolate the named pattern submatches into the report message.
There is a special case of `$$` which can be used to inject the entire pattern match into the message.

Expand All @@ -61,7 +59,7 @@ As everything else, statements are `Matcher` methods. [`Import()`](https://godoc
Rule group statements only affect the current rule group and last from the line they were defined until the end of a function block.

```go
func _(m fluent.Matcher) {
func testGroup(m fluent.Matcher) {
// <- Empty imports table.

m.Import(`github.com/some/pkg`)
Expand Down
3 changes: 3 additions & 0 deletions ruleguard/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ func (p *rulesParser) parseRuleGroup(f *ast.FuncDecl) (err error) {
panic(rv) // not our panic
}()

if f.Name.String() == "_" {
return p.errorf(f.Name, "`_` is not a valid rule group function name")
}
if f.Body == nil {
return p.errorf(f, "unexpected empty function body")
}
Expand Down
5 changes: 1 addition & 4 deletions rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ import "github.com/quasilyte/go-ruleguard/dsl/fluent"
//
// If you want to report any issue, please do so: https://github.com/quasilyte/go-ruleguard/issues/new

// "_" is a special "unnamed" rules group.
// It's not a good style to use these, but it can be a convenient place for
// various rules for which you can't find a good name.
func _(m fluent.Matcher) {
func miscRules(m fluent.Matcher) {
// See http://golang.org/issue/36225
m.Match(`json.NewDecoder($_).Decode($_)`).
Report(`this json.Decoder usage is erroneous`)
Expand Down