Skip to content

Commit

Permalink
analyzer/testdata: use \Q regexp quoting everywhere (#279)
Browse files Browse the repository at this point in the history
In analysis test we're using `want` directive to match warnings.
I personally thing that 99% of time we need a string matching,
not a regexp matching. So this test checks that we do `\Q`
escaping everywhere, making our patterns matching string "as is";
so you don't have to escape `(` and `)` in the patterns.
  • Loading branch information
quasilyte committed Oct 14, 2021
1 parent 2fa85af commit 1b62232
Show file tree
Hide file tree
Showing 20 changed files with 237 additions and 195 deletions.
42 changes: 42 additions & 0 deletions analyzer/analyzer_test.go
Expand Up @@ -4,10 +4,13 @@ import (
"bytes"
"fmt"
"go/token"
"io/fs"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -42,6 +45,45 @@ var tests = []struct {
{name: "goversion", flags: map[string]string{"go": "1.16"}},
}

func TestDirectiveComments(t *testing.T) {
testdata := analysistest.TestData()
badDirectiveRe := regexp.MustCompile("// want `[^\\\\][^Q].*")
err := filepath.WalkDir(testdata, func(filename string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
if !strings.HasSuffix(filename, ".go") {
return nil
}
data, err := os.ReadFile(filename)
if err != nil {
return err
}
lines := bytes.Split(data, []byte("\n"))
for i, l := range lines {
m := badDirectiveRe.Find(l)
if m == nil {
continue
}
if bytes.HasPrefix(m, []byte("// want `true")) {
continue
}
if bytes.HasPrefix(m, []byte("// want `false")) {
continue
}
relName := strings.TrimPrefix(filename, filepath.Join(testdata, "src")+"/")
t.Errorf("%s:%d: escape 'want' text with \\Q in %s", relName, i+1, m)
}
return nil
})
if err != nil {
t.Fatal(err)
}
}

func TestAnalyzer(t *testing.T) {
analyzer.ForceNewEngine = true
for i := range tests {
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/comments/target.go
Expand Up @@ -18,7 +18,7 @@ func f() {
//go:noinline
//go:generate foo bar

//nolint:gocritic // want `hey, this is kinda upsetting`
//nolint:gocritic // want `\Qhey, this is kinda upsetting`

// This is a begining // want `\Q"begining" may contain a typo`
// Of a bizzare text with typos. // want `\Q"bizzare" may contain a typo`
Expand Down
2 changes: 1 addition & 1 deletion analyzer/testdata/src/comments/target_foo.go
@@ -1,3 +1,3 @@
package comments

//go:embed data.txt // want `don't use go:embed in _foo files`
//go:embed data.txt // want `\Qdon't use go:embed in _foo files`
2 changes: 1 addition & 1 deletion analyzer/testdata/src/dgryski/learngo.go
Expand Up @@ -179,7 +179,7 @@ func learnFlowControl() {
}
// Use switch in preference to chained if statements.
x := 42.0
switch x { // want `floating point as switch expression`
switch x { // want `\Qfloating point as switch expression`
case 0:
case 1, 2: // Can have multiple matches on one case
case 42:
Expand Down
26 changes: 13 additions & 13 deletions analyzer/testdata/src/extra/file.go
Expand Up @@ -67,7 +67,7 @@ func testFormatBool() {

func testBlankAssign() {
x := foo()
_ = x // want `please remove the assignment to _`
_ = x // want `\Qplease remove the assignment to _`

// This is OK, could be for side-effects.
_ = foo()
Expand All @@ -92,10 +92,10 @@ func nilErrCheck() {
}

func unparen(x, y int) {
if (x == 0) || (y == 0) { // want `rewrite as 'x == 0 || y == 0'`
if (x == 0) || (y == 0) { // want `\Qrewrite as 'x == 0 || y == 0'`
}

if (x != 5) && (y == 5) { // want `rewrite as 'x != 5 && y == 5'`
if (x != 5) && (y == 5) { // want `\Qrewrite as 'x != 5 && y == 5'`
}
}

Expand Down Expand Up @@ -148,7 +148,7 @@ func makeExpr() {

func chanRange() int {
ch := make(chan int)
for { // want `can use for range over ch`
for { // want `\Qcan use for range over ch`
select {
case c := <-ch:
return c
Expand Down Expand Up @@ -184,7 +184,7 @@ func argOrder() {

func stringsReplace() {
var s string
_ = strings.Replace(s, " ", " ", -1) // want `replace 'old' and 'new' parameters are identical`
_ = strings.Replace(s, " ", " ", -1) // want `\Qreplace 'old' and 'new' parameters are identical`
}

func stringsRepeat() {
Expand All @@ -209,11 +209,11 @@ func stringsRepeat() {
func stringsCompare() {
var s1, s2 string

_ = strings.Compare(s1, s2) == 0 // want `suggestion: s1 == s2`
_ = strings.Compare(s1, s2) < 0 // want `suggestion: s1 < s2`
_ = strings.Compare(s1, s2) == -1 // want `suggestion: s1 < s2`
_ = strings.Compare(s1, s2) > 0 // want `suggestion: s1 > s2`
_ = strings.Compare(s1, s2) == 1 // want `suggestion: s1 > s2`
_ = strings.Compare(s1, s2) == 0 // want `\Qsuggestion: s1 == s2`
_ = strings.Compare(s1, s2) < 0 // want `\Qsuggestion: s1 < s2`
_ = strings.Compare(s1, s2) == -1 // want `\Qsuggestion: s1 < s2`
_ = strings.Compare(s1, s2) > 0 // want `\Qsuggestion: s1 > s2`
_ = strings.Compare(s1, s2) == 1 // want `\Qsuggestion: s1 > s2`

if s1 == s2 {
}
Expand Down Expand Up @@ -298,7 +298,7 @@ func testCtx(ctx context.Context) error {
return nil
}

type errDontLog error // want `error as underlying type is probably a mistake`
type errDontLog error // want `\Qerror as underlying type is probably a mistake`

var ( // want `\Qempty var() block`
// Empty decl...
Expand Down Expand Up @@ -436,8 +436,8 @@ func redundantLenCheck(xs []int, v int) {
}

func emptyError() {
_ = errors.New("") // want `empty error`
_ = errors.New(``) // want `empty error`
_ = errors.New("") // want `\Qempty error`
_ = errors.New(``) // want `\Qempty error`
}

func contextWithValue(ctx context.Context) {
Expand Down

0 comments on commit 1b62232

Please sign in to comment.