Skip to content

Commit

Permalink
Verify linter name in integration tests (#1595)
Browse files Browse the repository at this point in the history
  • Loading branch information
ashanbrown committed Jan 15, 2021
1 parent 257eb95 commit ec46f42
Show file tree
Hide file tree
Showing 22 changed files with 158 additions and 131 deletions.
3 changes: 2 additions & 1 deletion pkg/printers/text.go
Expand Up @@ -3,6 +3,7 @@ package printers
import (
"context"
"fmt"
"strings"

"github.com/fatih/color"

Expand Down Expand Up @@ -52,7 +53,7 @@ func (p *Text) Print(ctx context.Context, issues []result.Issue) error {
}

func (p Text) printIssue(i *result.Issue) {
text := p.SprintfColored(color.FgRed, "%s", i.Text)
text := p.SprintfColored(color.FgRed, "%s", strings.TrimSpace(i.Text))
if p.printLinterName {
text += fmt.Sprintf(" (%s)", i.FromLinter)
}
Expand Down
113 changes: 60 additions & 53 deletions test/errchk.go
Expand Up @@ -11,6 +11,8 @@ import (
"strings"
)

var errorLineRx = regexp.MustCompile(`^\S+?: (.*)\((\S+?)\)$`)

// errorCheck matches errors in outStr against comments in source files.
// For each line of the source files which should generate an error,
// there should be a comment of the form // ERROR "regexp".
Expand All @@ -22,8 +24,8 @@ import (
//
// Sources files are supplied as fullshort slice.
// It consists of pairs: full path to source file and its base name.
//nolint:gocyclo
func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
//nolint:gocyclo,funlen
func errorCheck(outStr string, wantAuto bool, defaultWantedLinter string, fullshort ...string) (err error) {
var errs []error
out := splitOutput(outStr, wantAuto)
// Cut directory name.
Expand All @@ -37,9 +39,16 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
var want []wantedError
for j := 0; j < len(fullshort); j += 2 {
full, short := fullshort[j], fullshort[j+1]
want = append(want, wantedErrors(full, short)...)
want = append(want, wantedErrors(full, short, defaultWantedLinter)...)
}
for _, we := range want {
if we.linter == "" {
err := fmt.Errorf("%s:%d: no expected linter indicated for test",
we.file, we.lineNum)
errs = append(errs, err)
continue
}

var errmsgs []string
if we.auto {
errmsgs, out = partitionStrings("<autogenerated>", out)
Expand All @@ -51,25 +60,35 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
continue
}
matched := false
n := len(out)
var textsToMatch []string
for _, errmsg := range errmsgs {
// Assume errmsg says "file:line: foo".
// Cut leading "file:line: " to avoid accidental matching of file name instead of message.
text := errmsg
if i := strings.Index(text, " "); i >= 0 {
text = text[i+1:]
// Assume errmsg says "file:line: foo (<linter>)".
matches := errorLineRx.FindStringSubmatch(errmsg)
if len(matches) == 0 {
err := fmt.Errorf("%s:%d: unexpected error line: %s",
we.file, we.lineNum, errmsg)
errs = append(errs, err)
continue
}

text, actualLinter := matches[1], matches[2]

if we.re.MatchString(text) {
matched = true
} else {
out = append(out, errmsg)
textsToMatch = append(textsToMatch, text)
}

if actualLinter != we.linter {
err := fmt.Errorf("%s:%d: expected error from %q but got error from %q in:\n\t%s",
we.file, we.lineNum, we.linter, actualLinter, strings.Join(out, "\n\t"))
errs = append(errs, err)
}
}
if !matched {
err := fmt.Errorf("%s:%d: no match for %#q vs %q in:\n\t%s",
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out[n:], "\n\t"))
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out, "\n\t"))
errs = append(errs, err)
continue
}
Expand Down Expand Up @@ -150,18 +169,18 @@ type wantedError struct {
auto bool // match <autogenerated> line
file string
prefix string
linter string
}

var (
errRx = regexp.MustCompile(`// (?:GC_)?ERROR (.*)`)
errAutoRx = regexp.MustCompile(`// (?:GC_)?ERRORAUTO (.*)`)
errQuotesRx = regexp.MustCompile(`"([^"]*)"`)
lineRx = regexp.MustCompile(`LINE(([+-])(\d+))?`)
errRx = regexp.MustCompile(`// (?:GC_)?ERROR (.*)`)
errAutoRx = regexp.MustCompile(`// (?:GC_)?ERRORAUTO (.*)`)
linterPrefixRx = regexp.MustCompile("^\\s*([^\\s\"`]+)")
)

// wantedErrors parses expected errors from comments in a file.
//nolint:nakedret
func wantedErrors(file, short string) (errs []wantedError) {
func wantedErrors(file, short, defaultLinter string) (errs []wantedError) {
cache := make(map[string]*regexp.Regexp)

src, err := ioutil.ReadFile(file)
Expand All @@ -184,47 +203,35 @@ func wantedErrors(file, short string) (errs []wantedError) {
if m == nil {
continue
}
all := m[1]
mm := errQuotesRx.FindAllStringSubmatch(all, -1)
if mm == nil {
log.Fatalf("%s:%d: invalid errchk line: %s", file, lineNum, line)
rest := m[1]
linter := defaultLinter
if lm := linterPrefixRx.FindStringSubmatch(rest); lm != nil {
linter = lm[1]
rest = rest[len(lm[0]):]
}
rx, err := strconv.Unquote(strings.TrimSpace(rest))
if err != nil {
log.Fatalf("%s:%d: invalid errchk line: %s, %v", file, lineNum, line, err)
}
for _, m := range mm {
replacedOnce := false
rx := lineRx.ReplaceAllStringFunc(m[1], func(m string) string {
if replacedOnce {
return m
}
replacedOnce = true
n := lineNum
if strings.HasPrefix(m, "LINE+") {
delta, _ := strconv.Atoi(m[5:])
n += delta
} else if strings.HasPrefix(m, "LINE-") {
delta, _ := strconv.Atoi(m[5:])
n -= delta
}
return fmt.Sprintf("%s:%d", short, n)
})
re := cache[rx]
if re == nil {
var err error
re, err = regexp.Compile(rx)
if err != nil {
log.Fatalf("%s:%d: invalid regexp \"%#q\" in ERROR line: %v", file, lineNum, rx, err)
}
cache[rx] = re
re := cache[rx]
if re == nil {
var err error
re, err = regexp.Compile(rx)
if err != nil {
log.Fatalf("%s:%d: invalid regexp \"%#q\" in ERROR line: %v", file, lineNum, rx, err)
}
prefix := fmt.Sprintf("%s:%d", short, lineNum)
errs = append(errs, wantedError{
reStr: rx,
re: re,
prefix: prefix,
auto: auto,
lineNum: lineNum,
file: short,
})
cache[rx] = re
}
prefix := fmt.Sprintf("%s:%d", short, lineNum)
errs = append(errs, wantedError{
reStr: rx,
re: re,
prefix: prefix,
auto: auto,
lineNum: lineNum,
file: short,
linter: linter,
})
}

return
Expand Down
36 changes: 28 additions & 8 deletions test/linters_test.go
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/golangci/golangci-lint/test/testshared"
)

func runGoErrchk(c *exec.Cmd, files []string, t *testing.T) {
func runGoErrchk(c *exec.Cmd, defaultExpectedLinter string, files []string, t *testing.T) {
output, err := c.CombinedOutput()
// The returned error will be nil if the test file does not have any issues
// and thus the linter exits with exit code 0. So perform the additional
Expand All @@ -33,7 +33,7 @@ func runGoErrchk(c *exec.Cmd, files []string, t *testing.T) {
fullshort = append(fullshort, f, filepath.Base(f))
}

err = errorCheck(string(output), false, fullshort...)
err = errorCheck(string(output), false, defaultExpectedLinter, fullshort...)
assert.NoError(t, err)
}

Expand Down Expand Up @@ -124,7 +124,6 @@ func testOneSource(t *testing.T, sourcePath string) {
"--allow-parallel-runners",
"--disable-all",
"--print-issued-lines=false",
"--print-linter-name=false",
"--out-format=line-number",
"--max-same-issues=100",
}
Expand Down Expand Up @@ -156,14 +155,15 @@ func testOneSource(t *testing.T, sourcePath string) {

cmd := exec.Command(binName, caseArgs...)
t.Log(caseArgs)
runGoErrchk(cmd, []string{sourcePath}, t)
runGoErrchk(cmd, rc.expectedLinter, []string{sourcePath}, t)
}
}

type runContext struct {
args []string
config map[string]interface{}
configPath string
args []string
config map[string]interface{}
configPath string
expectedLinter string
}

func buildConfigFromShortRepr(t *testing.T, repr string, config map[string]interface{}) {
Expand Down Expand Up @@ -213,7 +213,7 @@ func extractRunContextFromComments(t *testing.T, sourcePath string) *runContext
continue
}
if !strings.HasPrefix(line, "//") {
return rc
break
}

line = strings.TrimLeft(strings.TrimPrefix(line, "//"), " ")
Expand Down Expand Up @@ -242,9 +242,29 @@ func extractRunContextFromComments(t *testing.T, sourcePath string) *runContext
continue
}

if strings.HasPrefix(line, "expected_linter: ") {
expectedLinter := strings.TrimPrefix(line, "expected_linter: ")
assert.NotEmpty(t, expectedLinter)
rc.expectedLinter = expectedLinter
continue
}

assert.Fail(t, "invalid prefix of comment line %s", line)
}

// guess the expected linter if none is specified
if rc.expectedLinter == "" {
for _, arg := range rc.args {
if strings.HasPrefix(arg, "-E") && !strings.Contains(arg, ",") {
if rc.expectedLinter != "" {
assert.Fail(t, "could not infer expected linter for errors because multiple linters are enabled. Please use the `expected_linter: ` directive in your test to indicate the linter-under-test.") //nolint:lll
break
}
rc.expectedLinter = arg[2:]
}
}
}

return rc
}

Expand Down
2 changes: 1 addition & 1 deletion test/testdata/asciicheck.go
@@ -1,4 +1,4 @@
//args: -Easciicheck
package testdata

type TеstStruct struct{} // ERROR `identifier "TеstStruct" contain non-ASCII character: U+0435 'е'`
type TеstStruct struct{} // ERROR `identifier "TеstStruct" contain non-ASCII character: U\+0435 'е'`
4 changes: 2 additions & 2 deletions test/testdata/default_exclude.go
Expand Up @@ -4,13 +4,13 @@
/*Package testdata ...*/
package testdata

// InvalidFuncComment, both golint and stylecheck will complain about this, // ERROR `ST1020: comment on exported function ExportedFunc1 should be of the form "ExportedFunc1 ..."`
// InvalidFuncComment, both golint and stylecheck will complain about this, // ERROR stylecheck `ST1020: comment on exported function ExportedFunc1 should be of the form "ExportedFunc1 ..."`
// if include EXC0011, only the warning from golint will be ignored.
// And only the warning from stylecheck will start with "ST1020".
func ExportedFunc1() {
}

// InvalidFuncComment // ERROR `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."`
// InvalidFuncComment // ERROR stylecheck `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."`
// nolint:golint
func ExportedFunc2() {
}
Expand Down
8 changes: 4 additions & 4 deletions test/testdata/exportloopref.go
Expand Up @@ -10,11 +10,11 @@ func dummyFunction() {
println("loop expecting 10, 11, 12, 13")
for i, p := range []int{10, 11, 12, 13} {
printp(&p)
slice = append(slice, &p) // ERROR : "exporting a pointer for the loop variable p"
array[i] = &p // ERROR : "exporting a pointer for the loop variable p"
slice = append(slice, &p) // ERROR "exporting a pointer for the loop variable p"
array[i] = &p // ERROR "exporting a pointer for the loop variable p"
if i%2 == 0 {
ref = &p // ERROR : "exporting a pointer for the loop variable p"
str.x = &p // ERROR : "exporting a pointer for the loop variable p"
ref = &p // ERROR "exporting a pointer for the loop variable p"
str.x = &p // ERROR "exporting a pointer for the loop variable p"
}
var vStr struct{ x *int }
var vArray [4]*int
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/forbidigo.go
Expand Up @@ -5,5 +5,5 @@ package testdata
import "fmt"

func Forbidigo() {
fmt.Printf("too noisy!!!") // ERROR "use of `fmt.Printf` forbidden by pattern `fmt\\.Print.*`"
fmt.Printf("too noisy!!!") // ERROR "use of `fmt\\.Printf` forbidden by pattern `fmt\\\\.Print\\.\\*`"
}
4 changes: 2 additions & 2 deletions test/testdata/funlen.go
Expand Up @@ -3,7 +3,7 @@
//config: linters-settings.funlen.statements=10
package testdata

func TooManyLines() { // ERROR "Function 'TooManyLines' is too long \(22 > 20\)"
func TooManyLines() { // ERROR `Function 'TooManyLines' is too long \(22 > 20\)`
t := struct {
A string
B string
Expand All @@ -28,7 +28,7 @@ func TooManyLines() { // ERROR "Function 'TooManyLines' is too long \(22 > 20\)"
_ = t
}

func TooManyStatements() { // ERROR "Function 'TooManyStatements' has too many statements \(11 > 10\)"
func TooManyStatements() { // ERROR `Function 'TooManyStatements' has too many statements \(11 > 10\)`
a := 1
b := a
c := b
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/go-header_bad.go
@@ -1,4 +1,4 @@
/*MY TITLE!*/ // ERROR "Expected:TITLE., Actual: TITLE!"
/*MY TITLE!*/ // ERROR `Expected:TITLE\., Actual: TITLE!`

//args: -Egoheader
//config_path: testdata/configs/go-header.yml
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/gocritic.go
Expand Up @@ -7,7 +7,7 @@ import (
"log"
)

var _ = *flag.Bool("global1", false, "") // ERROR "flagDeref: immediate deref in \*flag.Bool\(.global1., false, ..\) is most likely an error; consider using flag\.BoolVar"
var _ = *flag.Bool("global1", false, "") // ERROR `flagDeref: immediate deref in \*flag.Bool\(.global1., false, ..\) is most likely an error; consider using flag\.BoolVar`

type size1 struct {
a bool
Expand Down
14 changes: 7 additions & 7 deletions test/testdata/godox.go
Expand Up @@ -3,11 +3,11 @@
package testdata

func todoLeftInCode() {
// TODO implement me // ERROR godox.go:6: Line contains FIXME/TODO: "TODO implement me"
//TODO no space // ERROR godox.go:7: Line contains FIXME/TODO: "TODO no space"
// TODO(author): 123 // ERROR godox.go:8: Line contains FIXME/TODO: "TODO\(author\): 123 // ERROR godox.go:8: L..."
//TODO(author): 123 // ERROR godox.go:9: Line contains FIXME/TODO: "TODO\(author\): 123 // ERROR godox.go:9: L..."
//TODO(author) 456 // ERROR godox.go:10: Line contains FIXME/TODO: "TODO\(author\) 456 // ERROR godox.go:10: L..."
// TODO: qwerty // ERROR godox.go:11: Line contains FIXME/TODO: "TODO: qwerty // ERROR godox.go:11: Line ..."
// todo 789 // ERROR godox.go:12: Line contains FIXME/TODO: "todo 789"
// TODO implement me // ERROR `Line contains FIXME/TODO: "TODO implement me`
//TODO no space // ERROR `Line contains FIXME/TODO: "TODO no space`
// TODO(author): 123 // ERROR `Line contains FIXME/TODO: "TODO\(author\): 123`
//TODO(author): 123 // ERROR `Line contains FIXME/TODO: "TODO\(author\): 123`
//TODO(author) 456 // ERROR `Line contains FIXME/TODO: "TODO\(author\) 456`
// TODO: qwerty // ERROR `Line contains FIXME/TODO: "TODO: qwerty`
// todo 789 // ERROR `Line contains FIXME/TODO: "todo 789`
}
6 changes: 3 additions & 3 deletions test/testdata/goerr113.go
Expand Up @@ -4,17 +4,17 @@ package testdata
import "os"

func SimpleEqual(e1, e2 error) bool {
return e1 == e2 // ERROR `err113: do not compare errors directly, use errors.Is() instead: "e1 == e2"`
return e1 == e2 // ERROR `err113: do not compare errors directly, use errors.Is\(\) instead: "e1 == e2"`
}

func SimpleNotEqual(e1, e2 error) bool {
return e1 != e2 // ERROR `err113: do not compare errors directly, use errors.Is() instead: "e1 != e2"`
return e1 != e2 // ERROR `err113: do not compare errors directly, use errors.Is\(\) instead: "e1 != e2"`
}

func CheckGoerr13Import(e error) bool {
f, err := os.Create("f.txt")
if err != nil {
return err == e // ERROR `err113: do not compare errors directly, use errors.Is() instead: "err == e"`
return err == e // ERROR `err113: do not compare errors directly, use errors.Is\(\) instead: "err == e"`
}
f.Close()
return false
Expand Down

0 comments on commit ec46f42

Please sign in to comment.