diff --git a/test/errchk.go b/test/errchk.go new file mode 100644 index 00000000..f42cd63c --- /dev/null +++ b/test/errchk.go @@ -0,0 +1,227 @@ +package test + +import ( + "bytes" + "errors" + "fmt" + "io/ioutil" + "log" + "regexp" + "strconv" + "strings" +) + +// 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". +// If outStr has an error for a line which has no such comment, +// this function will report an error. +// Likewise if outStr does not have an error for a line which has a comment, +// or if the error message does not match the <regexp>. +// The <regexp> syntax is Perl but it's best to stick to egrep. +// +// 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) { + var errs []error + out := splitOutput(outStr, wantAuto) + // Cut directory name. + for i := range out { + for j := 0; j < len(fullshort); j += 2 { + full, short := fullshort[j], fullshort[j+1] + out[i] = strings.ReplaceAll(out[i], full, short) + } + } + + var want []wantedError + for j := 0; j < len(fullshort); j += 2 { + full, short := fullshort[j], fullshort[j+1] + want = append(want, wantedErrors(full, short)...) + } + for _, we := range want { + var errmsgs []string + if we.auto { + errmsgs, out = partitionStrings("<autogenerated>", out) + } else { + errmsgs, out = partitionStrings(we.prefix, out) + } + if len(errmsgs) == 0 { + errs = append(errs, fmt.Errorf("%s:%d: missing error %q", we.file, we.lineNum, we.reStr)) + continue + } + matched := false + n := len(out) + 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:] + } + if we.re.MatchString(text) { + matched = true + } else { + out = append(out, errmsg) + } + } + if !matched { + errs = append(errs, fmt.Errorf("%s:%d: no match for %#q in:\n\t%s", we.file, we.lineNum, we.reStr, strings.Join(out[n:], "\n\t"))) + continue + } + } + + if len(out) > 0 { + errs = append(errs, fmt.Errorf("unmatched errors")) + for _, errLine := range out { + errs = append(errs, fmt.Errorf("%s", errLine)) + } + } + + if len(errs) == 0 { + return nil + } + if len(errs) == 1 { + return errs[0] + } + var buf bytes.Buffer + fmt.Fprintf(&buf, "\n") + for _, err := range errs { + fmt.Fprintf(&buf, "%s\n", err.Error()) + } + return errors.New(buf.String()) +} + +func splitOutput(out string, wantAuto bool) []string { + // gc error messages continue onto additional lines with leading tabs. + // Split the output at the beginning of each line that doesn't begin with a tab. + // <autogenerated> lines are impossible to match so those are filtered out. + var res []string + for _, line := range strings.Split(out, "\n") { + line = strings.TrimSuffix(line, "\r") // normalize Windows output + if strings.HasPrefix(line, "\t") { //nolint:gocritic + res[len(res)-1] += "\n" + line + } else if strings.HasPrefix(line, "go tool") || strings.HasPrefix(line, "#") || !wantAuto && strings.HasPrefix(line, "<autogenerated>") { + continue + } else if strings.TrimSpace(line) != "" { + res = append(res, line) + } + } + return res +} + +// matchPrefix reports whether s starts with file name prefix followed by a :, +// and possibly preceded by a directory name. +func matchPrefix(s, prefix string) bool { + i := strings.Index(s, ":") + if i < 0 { + return false + } + j := strings.LastIndex(s[:i], "/") + s = s[j+1:] + if len(s) <= len(prefix) || s[:len(prefix)] != prefix { + return false + } + if s[len(prefix)] == ':' { + return true + } + return false +} + +func partitionStrings(prefix string, strs []string) (matched, unmatched []string) { + for _, s := range strs { + if matchPrefix(s, prefix) { + matched = append(matched, s) + } else { + unmatched = append(unmatched, s) + } + } + return +} + +type wantedError struct { + reStr string + re *regexp.Regexp + lineNum int + auto bool // match <autogenerated> line + file string + prefix string +} + +var ( + errRx = regexp.MustCompile(`// (?:GC_)?ERROR (.*)`) + errAutoRx = regexp.MustCompile(`// (?:GC_)?ERRORAUTO (.*)`) + errQuotesRx = regexp.MustCompile(`"([^"]*)"`) + lineRx = regexp.MustCompile(`LINE(([+-])([0-9]+))?`) +) + +// wantedErrors parses expected errors from comments in a file. +//nolint:nakedret,gocyclo +func wantedErrors(file, short string) (errs []wantedError) { + cache := make(map[string]*regexp.Regexp) + + src, err := ioutil.ReadFile(file) + if err != nil { + log.Fatal(err) + } + for i, line := range strings.Split(string(src), "\n") { + lineNum := i + 1 + if strings.Contains(line, "////") { + // double comment disables ERROR + continue + } + var auto bool + m := errAutoRx.FindStringSubmatch(line) + if m != nil { + auto = true + } else { + m = errRx.FindStringSubmatch(line) + } + 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) + } + 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 + } + prefix := fmt.Sprintf("%s:%d", short, lineNum) + errs = append(errs, wantedError{ + reStr: rx, + re: re, + prefix: prefix, + auto: auto, + lineNum: lineNum, + file: short, + }) + } + } + + return +} diff --git a/test/linters_test.go b/test/linters_test.go index e08480d4..ec216647 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -2,15 +2,15 @@ package test import ( "bufio" - "bytes" "io/ioutil" "os" "os/exec" "path/filepath" - "runtime" "strings" "testing" + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/test/testshared" assert "github.com/stretchr/testify/require" @@ -18,12 +18,19 @@ import ( yaml "gopkg.in/yaml.v2" ) -func runGoErrchk(c *exec.Cmd, t *testing.T) { +func runGoErrchk(c *exec.Cmd, files []string, t *testing.T) { output, err := c.CombinedOutput() - assert.NoError(t, err, "Output:\n%s", output) + exitErr, ok := err.(*exec.ExitError) + assert.True(t, ok) + assert.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode()) - // Can't check exit code: tool only prints to output - assert.False(t, bytes.Contains(output, []byte("BUG")), "Output:\n%s", output) + fullshort := make([]string, 0, len(files)*2) + for _, f := range files { + fullshort = append(fullshort, f, filepath.Base(f)) + } + + err = errorCheck(string(output), false, fullshort...) + assert.NoError(t, err) } func testSourcesFromDir(t *testing.T, dir string) { @@ -92,9 +99,8 @@ func saveConfig(t *testing.T, cfg map[string]interface{}) (cfgPath string, finis } func testOneSource(t *testing.T, sourcePath string) { - goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk") args := []string{ - binName, "run", + "run", "--disable-all", "--print-issued-lines=false", "--print-linter-name=false", @@ -126,9 +132,9 @@ func testOneSource(t *testing.T, sourcePath string) { caseArgs = append(caseArgs, sourcePath) - cmd := exec.Command(goErrchkBin, caseArgs...) + cmd := exec.Command(binName, caseArgs...) t.Log(caseArgs) - runGoErrchk(cmd, t) + runGoErrchk(cmd, []string{sourcePath}, t) } } diff --git a/test/testdata/dupl.go b/test/testdata/dupl.go index 84f3d82f..14edae39 100644 --- a/test/testdata/dupl.go +++ b/test/testdata/dupl.go @@ -11,7 +11,7 @@ func (DuplLogger) level() int { func (DuplLogger) Debug(args ...interface{}) {} func (DuplLogger) Info(args ...interface{}) {} -func (logger *DuplLogger) First(args ...interface{}) { // ERROR "14-23 lines are duplicate of `testdata/dupl.go:25-34`" +func (logger *DuplLogger) First(args ...interface{}) { // ERROR "14-23 lines are duplicate of `.*dupl.go:25-34`" if logger.level() >= 0 { logger.Debug(args...) logger.Debug(args...) @@ -22,7 +22,7 @@ func (logger *DuplLogger) First(args ...interface{}) { // ERROR "14-23 lines are } } -func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "25-34 lines are duplicate of `testdata/dupl.go:14-23`" +func (logger *DuplLogger) Second(args ...interface{}) { // ERROR "25-34 lines are duplicate of `.*dupl.go:14-23`" if logger.level() >= 1 { logger.Info(args...) logger.Info(args...) diff --git a/test/testdata/govet.go b/test/testdata/govet.go index fea8959a..afb07a16 100644 --- a/test/testdata/govet.go +++ b/test/testdata/govet.go @@ -13,7 +13,7 @@ func Govet() error { func GovetShadow(f io.Reader, buf []byte) (err error) { if f != nil { - _, err := f.Read(buf) // ERROR "declaration of .err. shadows declaration at testdata/govet.go:\d+" + _, err := f.Read(buf) // ERROR "declaration of .err. shadows declaration at .*govet.go:\d+" if err != nil { return err }