Skip to content

Commit e2bbe2f

Browse files
committed
Support custom forbid reason messages
In order to allow users to communicate intent to collaborators, optionally embed custom messages into each forbidden pattern. The syntax is as follows: `#[message goes here]identifier` Example: `#[Please don't use this!]fmt\.Errorf` The message section is optional, so this change should be backwards compatible with all valid identifiers.
1 parent e0c18fe commit e2bbe2f

File tree

4 files changed

+89
-10
lines changed

4 files changed

+89
-10
lines changed

forbidigo/forbidigo.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type UsedIssue struct {
2424
identifier string
2525
pattern string
2626
position token.Position
27+
customMsg string
2728
}
2829

2930
func (a UsedIssue) Details() string {
@@ -36,13 +37,17 @@ func (a UsedIssue) Position() token.Position {
3637

3738
func (a UsedIssue) String() string { return toString(a) }
3839

39-
func toString(i Issue) string {
40-
return fmt.Sprintf("%s at %s", i.Details(), i.Position())
40+
func toString(i UsedIssue) string {
41+
s := fmt.Sprintf("%s at %s", i.Details(), i.Position())
42+
if i.customMsg != "" {
43+
s += ": " + i.customMsg
44+
}
45+
return s
4146
}
4247

4348
type Linter struct {
4449
cfg config
45-
patterns []*regexp.Regexp
50+
patterns []*pattern
4651
}
4752

4853
func DefaultPatterns() []string {
@@ -65,13 +70,13 @@ func NewLinter(patterns []string, options ...Option) (*Linter, error) {
6570
if len(patterns) == 0 {
6671
patterns = DefaultPatterns()
6772
}
68-
compiledPatterns := make([]*regexp.Regexp, 0, len(patterns))
69-
for _, p := range patterns {
70-
re, err := regexp.Compile(p)
73+
compiledPatterns := make([]*pattern, 0, len(patterns))
74+
for _, ptrn := range patterns {
75+
p, err := parse(ptrn)
7176
if err != nil {
72-
return nil, fmt.Errorf("unable to compile pattern `%s`: %s", p, err)
77+
return nil, err
7378
}
74-
compiledPatterns = append(compiledPatterns, re)
79+
compiledPatterns = append(compiledPatterns, p)
7580
}
7681
return &Linter{
7782
cfg: cfg,
@@ -158,11 +163,12 @@ func (v *visitor) Visit(node ast.Node) ast.Visitor {
158163
return v
159164
}
160165
for _, p := range v.linter.patterns {
161-
if p.MatchString(v.textFor(node)) && !v.permit(node) {
166+
if p.pattern.MatchString(v.textFor(node)) && !v.permit(node) {
162167
v.issues = append(v.issues, UsedIssue{
163168
identifier: v.textFor(node),
164-
pattern: p.String(),
169+
pattern: p.pattern.String(),
165170
position: v.fset.Position(node.Pos()),
171+
customMsg: p.msg,
166172
})
167173
}
168174
}

forbidigo/forbidigo_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ func foo() {
1919
}`, "use of `fmt.Printf` forbidden by pattern `fmt\\.Printf` at testing.go:5:2")
2020
})
2121

22+
t.Run("displays custom messages", func(t *testing.T) {
23+
linter, _ := NewLinter([]string{`#[Please don't use this!]fmt\.Printf`})
24+
expectIssues(t, linter, `
25+
package bar
26+
27+
func foo() {
28+
fmt.Printf("here i am")
29+
}`, "use of `fmt.Printf` forbidden by pattern `fmt\\.Printf` at testing.go:5:2: Please don't use this!")
30+
})
31+
2232
t.Run("it doesn't require a package on the identifier", func(t *testing.T) {
2333
linter, _ := NewLinter([]string{`Printf`})
2434
expectIssues(t, linter, `

forbidigo/patterns.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package forbidigo
2+
3+
import (
4+
"fmt"
5+
"regexp"
6+
)
7+
8+
var ptrnWithMsg = regexp.MustCompile(`(#\[(?P<msg>[^\]]+)\])?(?P<pattern>.+)`)
9+
10+
type pattern struct {
11+
pattern *regexp.Regexp
12+
msg string
13+
}
14+
15+
func parse(ptrn string) (*pattern, error) {
16+
p := &pattern{}
17+
matches := ptrnWithMsg.FindStringSubmatch(ptrn)
18+
for i, name := range ptrnWithMsg.SubexpNames() {
19+
if name == "msg" {
20+
p.msg = matches[i]
21+
} else if name == "pattern" {
22+
re, err := regexp.Compile(matches[i])
23+
if err != nil {
24+
return nil, fmt.Errorf("unable to compile pattern `%s`: %s", matches[i], err)
25+
}
26+
p.pattern = re
27+
}
28+
}
29+
30+
return p, nil
31+
}

forbidigo/patterns_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package forbidigo
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestParseValidPattern(t *testing.T) {
11+
ptrn, err := parse(`fmt\.Errorf`)
12+
require.Nil(t, err)
13+
assert.Equal(t, `fmt\.Errorf`, ptrn.pattern.String())
14+
}
15+
16+
func TestParseValidPatternThatUsesSquareBrackets(t *testing.T) {
17+
ptrn, err := parse(`[f]mt\.Errorf`)
18+
require.Nil(t, err)
19+
assert.Equal(t, `[f]mt\.Errorf`, ptrn.pattern.String())
20+
}
21+
22+
func TestParseValidPatternWithCustomMessage(t *testing.T) {
23+
ptrn, err := parse(`#[Please don't use this!]fmt\.Println`)
24+
require.Nil(t, err)
25+
assert.Equal(t, `fmt\.Println`, ptrn.pattern.String())
26+
assert.Equal(t, "Please don't use this!", ptrn.msg)
27+
}
28+
29+
func TestParseInvalidPattern_ReturnsError(t *testing.T) {
30+
_, err := parse(`fmt\`)
31+
assert.NotNil(t, err)
32+
}

0 commit comments

Comments
 (0)