Skip to content

Commit ec46f42

Browse files
ashanbrownashanbrown
authored
Verify linter name in integration tests (#1595)
1 parent 257eb95 commit ec46f42

22 files changed

+158
-131
lines changed

‎pkg/printers/text.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package printers
33
import (
44
"context"
55
"fmt"
6+
"strings"
67

78
"github.com/fatih/color"
89

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

5455
func (p Text) printIssue(i *result.Issue) {
55-
text := p.SprintfColored(color.FgRed, "%s", i.Text)
56+
text := p.SprintfColored(color.FgRed, "%s", strings.TrimSpace(i.Text))
5657
if p.printLinterName {
5758
text += fmt.Sprintf(" (%s)", i.FromLinter)
5859
}

‎test/errchk.go

+60-53
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"strings"
1212
)
1313

14+
var errorLineRx = regexp.MustCompile(`^\S+?: (.*)\((\S+?)\)$`)
15+
1416
// errorCheck matches errors in outStr against comments in source files.
1517
// For each line of the source files which should generate an error,
1618
// there should be a comment of the form // ERROR "regexp".
@@ -22,8 +24,8 @@ import (
2224
//
2325
// Sources files are supplied as fullshort slice.
2426
// It consists of pairs: full path to source file and its base name.
25-
//nolint:gocyclo
26-
func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
27+
//nolint:gocyclo,funlen
28+
func errorCheck(outStr string, wantAuto bool, defaultWantedLinter string, fullshort ...string) (err error) {
2729
var errs []error
2830
out := splitOutput(outStr, wantAuto)
2931
// Cut directory name.
@@ -37,9 +39,16 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
3739
var want []wantedError
3840
for j := 0; j < len(fullshort); j += 2 {
3941
full, short := fullshort[j], fullshort[j+1]
40-
want = append(want, wantedErrors(full, short)...)
42+
want = append(want, wantedErrors(full, short, defaultWantedLinter)...)
4143
}
4244
for _, we := range want {
45+
if we.linter == "" {
46+
err := fmt.Errorf("%s:%d: no expected linter indicated for test",
47+
we.file, we.lineNum)
48+
errs = append(errs, err)
49+
continue
50+
}
51+
4352
var errmsgs []string
4453
if we.auto {
4554
errmsgs, out = partitionStrings("<autogenerated>", out)
@@ -51,25 +60,35 @@ func errorCheck(outStr string, wantAuto bool, fullshort ...string) (err error) {
5160
continue
5261
}
5362
matched := false
54-
n := len(out)
5563
var textsToMatch []string
5664
for _, errmsg := range errmsgs {
57-
// Assume errmsg says "file:line: foo".
58-
// Cut leading "file:line: " to avoid accidental matching of file name instead of message.
59-
text := errmsg
60-
if i := strings.Index(text, " "); i >= 0 {
61-
text = text[i+1:]
65+
// Assume errmsg says "file:line: foo (<linter>)".
66+
matches := errorLineRx.FindStringSubmatch(errmsg)
67+
if len(matches) == 0 {
68+
err := fmt.Errorf("%s:%d: unexpected error line: %s",
69+
we.file, we.lineNum, errmsg)
70+
errs = append(errs, err)
71+
continue
6272
}
73+
74+
text, actualLinter := matches[1], matches[2]
75+
6376
if we.re.MatchString(text) {
6477
matched = true
6578
} else {
6679
out = append(out, errmsg)
6780
textsToMatch = append(textsToMatch, text)
6881
}
82+
83+
if actualLinter != we.linter {
84+
err := fmt.Errorf("%s:%d: expected error from %q but got error from %q in:\n\t%s",
85+
we.file, we.lineNum, we.linter, actualLinter, strings.Join(out, "\n\t"))
86+
errs = append(errs, err)
87+
}
6988
}
7089
if !matched {
7190
err := fmt.Errorf("%s:%d: no match for %#q vs %q in:\n\t%s",
72-
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out[n:], "\n\t"))
91+
we.file, we.lineNum, we.reStr, textsToMatch, strings.Join(out, "\n\t"))
7392
errs = append(errs, err)
7493
continue
7594
}
@@ -150,18 +169,18 @@ type wantedError struct {
150169
auto bool // match <autogenerated> line
151170
file string
152171
prefix string
172+
linter string
153173
}
154174

155175
var (
156-
errRx = regexp.MustCompile(`// (?:GC_)?ERROR (.*)`)
157-
errAutoRx = regexp.MustCompile(`// (?:GC_)?ERRORAUTO (.*)`)
158-
errQuotesRx = regexp.MustCompile(`"([^"]*)"`)
159-
lineRx = regexp.MustCompile(`LINE(([+-])(\d+))?`)
176+
errRx = regexp.MustCompile(`// (?:GC_)?ERROR (.*)`)
177+
errAutoRx = regexp.MustCompile(`// (?:GC_)?ERRORAUTO (.*)`)
178+
linterPrefixRx = regexp.MustCompile("^\\s*([^\\s\"`]+)")
160179
)
161180

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

167186
src, err := ioutil.ReadFile(file)
@@ -184,47 +203,35 @@ func wantedErrors(file, short string) (errs []wantedError) {
184203
if m == nil {
185204
continue
186205
}
187-
all := m[1]
188-
mm := errQuotesRx.FindAllStringSubmatch(all, -1)
189-
if mm == nil {
190-
log.Fatalf("%s:%d: invalid errchk line: %s", file, lineNum, line)
206+
rest := m[1]
207+
linter := defaultLinter
208+
if lm := linterPrefixRx.FindStringSubmatch(rest); lm != nil {
209+
linter = lm[1]
210+
rest = rest[len(lm[0]):]
211+
}
212+
rx, err := strconv.Unquote(strings.TrimSpace(rest))
213+
if err != nil {
214+
log.Fatalf("%s:%d: invalid errchk line: %s, %v", file, lineNum, line, err)
191215
}
192-
for _, m := range mm {
193-
replacedOnce := false
194-
rx := lineRx.ReplaceAllStringFunc(m[1], func(m string) string {
195-
if replacedOnce {
196-
return m
197-
}
198-
replacedOnce = true
199-
n := lineNum
200-
if strings.HasPrefix(m, "LINE+") {
201-
delta, _ := strconv.Atoi(m[5:])
202-
n += delta
203-
} else if strings.HasPrefix(m, "LINE-") {
204-
delta, _ := strconv.Atoi(m[5:])
205-
n -= delta
206-
}
207-
return fmt.Sprintf("%s:%d", short, n)
208-
})
209-
re := cache[rx]
210-
if re == nil {
211-
var err error
212-
re, err = regexp.Compile(rx)
213-
if err != nil {
214-
log.Fatalf("%s:%d: invalid regexp \"%#q\" in ERROR line: %v", file, lineNum, rx, err)
215-
}
216-
cache[rx] = re
216+
re := cache[rx]
217+
if re == nil {
218+
var err error
219+
re, err = regexp.Compile(rx)
220+
if err != nil {
221+
log.Fatalf("%s:%d: invalid regexp \"%#q\" in ERROR line: %v", file, lineNum, rx, err)
217222
}
218-
prefix := fmt.Sprintf("%s:%d", short, lineNum)
219-
errs = append(errs, wantedError{
220-
reStr: rx,
221-
re: re,
222-
prefix: prefix,
223-
auto: auto,
224-
lineNum: lineNum,
225-
file: short,
226-
})
223+
cache[rx] = re
227224
}
225+
prefix := fmt.Sprintf("%s:%d", short, lineNum)
226+
errs = append(errs, wantedError{
227+
reStr: rx,
228+
re: re,
229+
prefix: prefix,
230+
auto: auto,
231+
lineNum: lineNum,
232+
file: short,
233+
linter: linter,
234+
})
228235
}
229236

230237
return

‎test/linters_test.go

+28-8
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"github.com/golangci/golangci-lint/test/testshared"
1616
)
1717

18-
func runGoErrchk(c *exec.Cmd, files []string, t *testing.T) {
18+
func runGoErrchk(c *exec.Cmd, defaultExpectedLinter string, files []string, t *testing.T) {
1919
output, err := c.CombinedOutput()
2020
// The returned error will be nil if the test file does not have any issues
2121
// 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) {
3333
fullshort = append(fullshort, f, filepath.Base(f))
3434
}
3535

36-
err = errorCheck(string(output), false, fullshort...)
36+
err = errorCheck(string(output), false, defaultExpectedLinter, fullshort...)
3737
assert.NoError(t, err)
3838
}
3939

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

157156
cmd := exec.Command(binName, caseArgs...)
158157
t.Log(caseArgs)
159-
runGoErrchk(cmd, []string{sourcePath}, t)
158+
runGoErrchk(cmd, rc.expectedLinter, []string{sourcePath}, t)
160159
}
161160
}
162161

163162
type runContext struct {
164-
args []string
165-
config map[string]interface{}
166-
configPath string
163+
args []string
164+
config map[string]interface{}
165+
configPath string
166+
expectedLinter string
167167
}
168168

169169
func buildConfigFromShortRepr(t *testing.T, repr string, config map[string]interface{}) {
@@ -213,7 +213,7 @@ func extractRunContextFromComments(t *testing.T, sourcePath string) *runContext
213213
continue
214214
}
215215
if !strings.HasPrefix(line, "//") {
216-
return rc
216+
break
217217
}
218218

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

245+
if strings.HasPrefix(line, "expected_linter: ") {
246+
expectedLinter := strings.TrimPrefix(line, "expected_linter: ")
247+
assert.NotEmpty(t, expectedLinter)
248+
rc.expectedLinter = expectedLinter
249+
continue
250+
}
251+
245252
assert.Fail(t, "invalid prefix of comment line %s", line)
246253
}
247254

255+
// guess the expected linter if none is specified
256+
if rc.expectedLinter == "" {
257+
for _, arg := range rc.args {
258+
if strings.HasPrefix(arg, "-E") && !strings.Contains(arg, ",") {
259+
if rc.expectedLinter != "" {
260+
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
261+
break
262+
}
263+
rc.expectedLinter = arg[2:]
264+
}
265+
}
266+
}
267+
248268
return rc
249269
}
250270

‎test/testdata/asciicheck.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
//args: -Easciicheck
22
package testdata
33

4-
type TеstStruct struct{} // ERROR `identifier "TеstStruct" contain non-ASCII character: U+0435 'е'`
4+
type TеstStruct struct{} // ERROR `identifier "TеstStruct" contain non-ASCII character: U\+0435 'е'`

‎test/testdata/default_exclude.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
/*Package testdata ...*/
55
package testdata
66

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

13-
// InvalidFuncComment // ERROR `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."`
13+
// InvalidFuncComment // ERROR stylecheck `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."`
1414
// nolint:golint
1515
func ExportedFunc2() {
1616
}

‎test/testdata/exportloopref.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ func dummyFunction() {
1010
println("loop expecting 10, 11, 12, 13")
1111
for i, p := range []int{10, 11, 12, 13} {
1212
printp(&p)
13-
slice = append(slice, &p) // ERROR : "exporting a pointer for the loop variable p"
14-
array[i] = &p // ERROR : "exporting a pointer for the loop variable p"
13+
slice = append(slice, &p) // ERROR "exporting a pointer for the loop variable p"
14+
array[i] = &p // ERROR "exporting a pointer for the loop variable p"
1515
if i%2 == 0 {
16-
ref = &p // ERROR : "exporting a pointer for the loop variable p"
17-
str.x = &p // ERROR : "exporting a pointer for the loop variable p"
16+
ref = &p // ERROR "exporting a pointer for the loop variable p"
17+
str.x = &p // ERROR "exporting a pointer for the loop variable p"
1818
}
1919
var vStr struct{ x *int }
2020
var vArray [4]*int

‎test/testdata/forbidigo.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ package testdata
55
import "fmt"
66

77
func Forbidigo() {
8-
fmt.Printf("too noisy!!!") // ERROR "use of `fmt.Printf` forbidden by pattern `fmt\\.Print.*`"
8+
fmt.Printf("too noisy!!!") // ERROR "use of `fmt\\.Printf` forbidden by pattern `fmt\\\\.Print\\.\\*`"
99
}

‎test/testdata/funlen.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//config: linters-settings.funlen.statements=10
44
package testdata
55

6-
func TooManyLines() { // ERROR "Function 'TooManyLines' is too long \(22 > 20\)"
6+
func TooManyLines() { // ERROR `Function 'TooManyLines' is too long \(22 > 20\)`
77
t := struct {
88
A string
99
B string
@@ -28,7 +28,7 @@ func TooManyLines() { // ERROR "Function 'TooManyLines' is too long \(22 > 20\)"
2828
_ = t
2929
}
3030

31-
func TooManyStatements() { // ERROR "Function 'TooManyStatements' has too many statements \(11 > 10\)"
31+
func TooManyStatements() { // ERROR `Function 'TooManyStatements' has too many statements \(11 > 10\)`
3232
a := 1
3333
b := a
3434
c := b

‎test/testdata/go-header_bad.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*MY TITLE!*/ // ERROR "Expected:TITLE., Actual: TITLE!"
1+
/*MY TITLE!*/ // ERROR `Expected:TITLE\., Actual: TITLE!`
22

33
//args: -Egoheader
44
//config_path: testdata/configs/go-header.yml

‎test/testdata/gocritic.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"log"
88
)
99

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

1212
type size1 struct {
1313
a bool

‎test/testdata/godox.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
package testdata
44

55
func todoLeftInCode() {
6-
// TODO implement me // ERROR godox.go:6: Line contains FIXME/TODO: "TODO implement me"
7-
//TODO no space // ERROR godox.go:7: Line contains FIXME/TODO: "TODO no space"
8-
// TODO(author): 123 // ERROR godox.go:8: Line contains FIXME/TODO: "TODO\(author\): 123 // ERROR godox.go:8: L..."
9-
//TODO(author): 123 // ERROR godox.go:9: Line contains FIXME/TODO: "TODO\(author\): 123 // ERROR godox.go:9: L..."
10-
//TODO(author) 456 // ERROR godox.go:10: Line contains FIXME/TODO: "TODO\(author\) 456 // ERROR godox.go:10: L..."
11-
// TODO: qwerty // ERROR godox.go:11: Line contains FIXME/TODO: "TODO: qwerty // ERROR godox.go:11: Line ..."
12-
// todo 789 // ERROR godox.go:12: Line contains FIXME/TODO: "todo 789"
6+
// TODO implement me // ERROR `Line contains FIXME/TODO: "TODO implement me`
7+
//TODO no space // ERROR `Line contains FIXME/TODO: "TODO no space`
8+
// TODO(author): 123 // ERROR `Line contains FIXME/TODO: "TODO\(author\): 123`
9+
//TODO(author): 123 // ERROR `Line contains FIXME/TODO: "TODO\(author\): 123`
10+
//TODO(author) 456 // ERROR `Line contains FIXME/TODO: "TODO\(author\) 456`
11+
// TODO: qwerty // ERROR `Line contains FIXME/TODO: "TODO: qwerty`
12+
// todo 789 // ERROR `Line contains FIXME/TODO: "todo 789`
1313
}

‎test/testdata/goerr113.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@ package testdata
44
import "os"
55

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

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

1414
func CheckGoerr13Import(e error) bool {
1515
f, err := os.Create("f.txt")
1616
if err != nil {
17-
return err == e // ERROR `err113: do not compare errors directly, use errors.Is() instead: "err == e"`
17+
return err == e // ERROR `err113: do not compare errors directly, use errors.Is\(\) instead: "err == e"`
1818
}
1919
f.Close()
2020
return false

0 commit comments

Comments
 (0)