Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/printers/text.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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+") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as I could tell, nothing was using this logic that appears to have to do with matching line numbers.

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
Original file line number Diff line number Diff line change
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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the linter name to verify it

"--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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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 ..."`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicitly indicating the expected linter here

// 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
package testdata

func todoLeftInCode() {
// TODO implement me // ERROR godox.go:6: Line contains FIXME/TODO: "TODO implement me"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the only thing being matched here was the stuff between double-quotes.

//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
Original file line number Diff line number Diff line change
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"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes this a valid regex in a go string

}

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
Loading