Verify linter name in integration tests (golangci#1595)

Author: ashanbrown

pkg/printers/text.go

@@ -3,6 +3,7 @@ package printers
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/fatih/color"
 
@@ -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)
 	}

test/errchk.go

@@ -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".
@@ -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.
@@ -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)
@@ -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
 		}
@@ -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)
@@ -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

test/linters_test.go

@@ -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
@@ -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)
 }
 
@@ -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",
 	}
@@ -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{}) {
@@ -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, "//"), " ")
@@ -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
 }
 

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 'е'`

test/testdata/default_exclude.go

@@ -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() {
 }

test/testdata/exportloopref.go

@@ -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

test/testdata/forbidigo.go

@@ -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\\.\\*`"
 }

test/testdata/funlen.go

@@ -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
@@ -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

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

test/testdata/gocritic.go

@@ -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

test/testdata/godox.go

@@ -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`
 }

test/testdata/goerr113.go

@@ -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