diff --git a/internal/commands/run.go b/internal/commands/run.go index a4a94ef5..99c7c690 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -2,22 +2,19 @@ package commands import ( "context" - "encoding/json" "fmt" "go/build" "log" - "os" "strings" - "syscall" "time" - "github.com/fatih/color" "github.com/golangci/go-tools/ssa" "github.com/golangci/go-tools/ssa/ssautil" "github.com/golangci/golangci-lint/pkg" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/golinters" + "github.com/golangci/golangci-lint/pkg/printers" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result/processors" "github.com/sirupsen/logrus" @@ -39,6 +36,8 @@ func (e *Executor) initRun() { runCmd.Flags().StringVar(&rc.OutFormat, "out-format", config.OutFormatColoredLineNumber, fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|"))) + runCmd.Flags().BoolVar(&rc.PrintIssuedLine, "print-issued-lines", true, "Print lines of code with issue") + runCmd.Flags().IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", 1, "Exit code when issues were found") runCmd.Flags().StringSliceVar(&rc.BuildTags, "build-tags", []string{}, "Build tags (not all linters support them)") @@ -227,9 +226,15 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) { return err } - gotAnyIssues, err := outputIssues(e.cfg.Run.OutFormat, issues) + var p printers.Printer + if e.cfg.Run.OutFormat == config.OutFormatJSON { + p = printers.NewJSON() + } else { + p = printers.NewText(e.cfg.Run.PrintIssuedLine, e.cfg.Run.OutFormat == config.OutFormatColoredLineNumber) + } + gotAnyIssues, err := p.Print(issues) if err != nil { - return fmt.Errorf("can't output %d issues: %s", len(issues), err) + return fmt.Errorf("can't print %d issues: %s", len(issues), err) } if gotAnyIssues { @@ -247,43 +252,3 @@ func (e *Executor) executeRun(cmd *cobra.Command, args []string) { } } } - -func outputIssues(format string, issues chan result.Issue) (bool, error) { - stdout := os.NewFile(uintptr(syscall.Stdout), "/dev/stdout") // was set to /dev/null - if format == config.OutFormatLineNumber || format == config.OutFormatColoredLineNumber { - gotAnyIssue := false - for i := range issues { - gotAnyIssue = true - text := i.Text - if format == config.OutFormatColoredLineNumber { - text = color.RedString(text) - } - fmt.Fprintf(stdout, "%s:%d: %s\n", i.File, i.LineNumber, text) - } - - if !gotAnyIssue { - outStr := "Congrats! No issues were found." - if format == config.OutFormatColoredLineNumber { - outStr = color.GreenString(outStr) - } - fmt.Fprintln(stdout, outStr) - } - - return gotAnyIssue, nil - } - - if format == config.OutFormatJSON { - var allIssues []result.Issue - for i := range issues { - allIssues = append(allIssues, i) - } - outputJSON, err := json.Marshal(allIssues) - if err != nil { - return false, err - } - fmt.Fprint(stdout, string(outputJSON)) - return len(allIssues) != 0, nil - } - - return false, fmt.Errorf("unknown output format %q", format) -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 1b1209fc..ab68d044 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -27,7 +27,9 @@ type Run struct { BuildTags []string - OutFormat string + OutFormat string + PrintIssuedLine bool + ExitCodeIfIssuesFound int Errcheck struct { diff --git a/pkg/golinters/deadcode.go b/pkg/golinters/deadcode.go index 51f66063..aa54184d 100644 --- a/pkg/golinters/deadcode.go +++ b/pkg/golinters/deadcode.go @@ -23,8 +23,7 @@ func (d Deadcode) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, er var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: fmt.Sprintf("%s is unused", formatCode(i.UnusedIdentName, lintCtx.RunCfg())), FromLinter: d.Name(), }) diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index 299a06c6..a0e8c7bf 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -3,6 +3,7 @@ package golinters import ( "context" "fmt" + "go/token" "github.com/golangci/golangci-lint/pkg/result" duplAPI "github.com/mibk/dupl" @@ -27,8 +28,14 @@ func (d Dupl) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error) i.From.LineStart(), i.From.LineEnd(), formatCode(dupl, lintCtx.RunCfg())) res = append(res, result.Issue{ - File: i.From.Filename(), - LineNumber: i.From.LineStart(), + Pos: token.Position{ + Filename: i.From.Filename(), + Line: i.From.LineStart(), + }, + LineRange: result.Range{ + From: i.From.LineStart(), + To: i.From.LineEnd(), + }, Text: text, FromLinter: d.Name(), }) diff --git a/pkg/golinters/errcheck.go b/pkg/golinters/errcheck.go index cd9b0488..86239789 100644 --- a/pkg/golinters/errcheck.go +++ b/pkg/golinters/errcheck.go @@ -37,8 +37,7 @@ func (e Errcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, er res = append(res, result.Issue{ FromLinter: e.Name(), Text: text, - LineNumber: i.Pos.Line, - File: i.Pos.Filename, + Pos: i.Pos, }) } diff --git a/pkg/golinters/gas.go b/pkg/golinters/gas.go index f177009d..c45bfc86 100644 --- a/pkg/golinters/gas.go +++ b/pkg/golinters/gas.go @@ -3,6 +3,7 @@ package golinters import ( "context" "fmt" + "go/token" "io/ioutil" "log" "strconv" @@ -33,8 +34,10 @@ func (lint Gas) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, erro text := fmt.Sprintf("%s: %s", i.RuleID, i.What) // TODO: use severity and confidence line, _ := strconv.Atoi(i.Line) res = append(res, result.Issue{ - File: i.File, - LineNumber: line, + Pos: token.Position{ + Filename: i.File, + Line: line, + }, Text: text, FromLinter: lint.Name(), }) diff --git a/pkg/golinters/goconst.go b/pkg/golinters/goconst.go index f74d47ff..17ed46b0 100644 --- a/pkg/golinters/goconst.go +++ b/pkg/golinters/goconst.go @@ -33,8 +33,7 @@ func (lint Goconst) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, textEnd = fmt.Sprintf(", but such constant %s already exists", formatCode(i.MatchingConst, lintCtx.RunCfg())) } res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: textBegin + textEnd, FromLinter: lint.Name(), }) diff --git a/pkg/golinters/gocyclo.go b/pkg/golinters/gocyclo.go index 767072a5..505dc54d 100644 --- a/pkg/golinters/gocyclo.go +++ b/pkg/golinters/gocyclo.go @@ -24,8 +24,7 @@ func (g Gocyclo) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, err } res = append(res, result.Issue{ - File: s.Pos.Filename, - LineNumber: s.Pos.Line, + Pos: s.Pos, Text: fmt.Sprintf("cyclomatic complexity %d of func %s is high (> %d)", s.Complexity, formatCode(s.FuncName, lintCtx.RunCfg()), lintCtx.RunCfg().Gocyclo.MinComplexity), FromLinter: g.Name(), diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index dad64a4a..a52a4df3 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "go/token" gofmtAPI "github.com/golangci/gofmt/gofmt" goimportsAPI "github.com/golangci/gofmt/goimports" @@ -79,9 +80,11 @@ func (g Gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { } i := result.Issue{ FromLinter: g.Name(), - File: d.NewName, - LineNumber: deletedLine, - Text: text, + Pos: token.Position{ + Filename: d.NewName, + Line: deletedLine, + }, + Text: text, } issues = append(issues, i) } diff --git a/pkg/golinters/golint.go b/pkg/golinters/golint.go index 1e088b9c..04fc003a 100644 --- a/pkg/golinters/golint.go +++ b/pkg/golinters/golint.go @@ -89,9 +89,8 @@ func lintFiles(minConfidence float64, filenames ...string) ([]result.Issue, erro for _, p := range ps { if p.Confidence >= minConfidence { issues = append(issues, result.Issue{ - File: p.Position.Filename, - LineNumber: p.Position.Line, - Text: p.Text, + Pos: p.Position, + Text: p.Text, }) // TODO: use p.Link and p.Category } diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index 3ea140d4..431a071f 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -22,8 +22,7 @@ func (g Govet) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, error var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: i.Message, FromLinter: g.Name(), }) diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go index 1ccb092b..be5ca72e 100644 --- a/pkg/golinters/ineffassign.go +++ b/pkg/golinters/ineffassign.go @@ -20,8 +20,7 @@ func (lint Ineffassign) Run(ctx context.Context, lintCtx *Context) ([]result.Iss var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: fmt.Sprintf("ineffectual assignment to %s", formatCode(i.IdentName, lintCtx.RunCfg())), FromLinter: lint.Name(), }) diff --git a/pkg/golinters/interfacer.go b/pkg/golinters/interfacer.go index 6c3936a8..c0512c41 100644 --- a/pkg/golinters/interfacer.go +++ b/pkg/golinters/interfacer.go @@ -28,8 +28,7 @@ func (lint Interfacer) Run(ctx context.Context, lintCtx *Context) ([]result.Issu for _, i := range issues { pos := lintCtx.SSAProgram.Fset.Position(i.Pos()) res = append(res, result.Issue{ - File: pos.Filename, - LineNumber: pos.Line, + Pos: pos, Text: i.Message(), FromLinter: lint.Name(), }) diff --git a/pkg/golinters/maligned.go b/pkg/golinters/maligned.go index 52f8d3db..5a773191 100644 --- a/pkg/golinters/maligned.go +++ b/pkg/golinters/maligned.go @@ -24,8 +24,7 @@ func (m Maligned) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, er text += fmt.Sprintf(":\n%s", formatCodeBlock(i.NewStructDef, lintCtx.RunCfg())) } res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: text, FromLinter: m.Name(), }) diff --git a/pkg/golinters/megacheck.go b/pkg/golinters/megacheck.go index 81b27cf3..279aa0a8 100644 --- a/pkg/golinters/megacheck.go +++ b/pkg/golinters/megacheck.go @@ -20,8 +20,7 @@ func (m Megacheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, e var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Position.Filename, - LineNumber: i.Position.Line, + Pos: i.Position, Text: i.Text, FromLinter: m.Name(), }) diff --git a/pkg/golinters/structcheck.go b/pkg/golinters/structcheck.go index 9e332647..0966636e 100644 --- a/pkg/golinters/structcheck.go +++ b/pkg/golinters/structcheck.go @@ -20,8 +20,7 @@ func (s Structcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: fmt.Sprintf("%s is unused", formatCode(i.FieldName, lintCtx.RunCfg())), FromLinter: s.Name(), }) diff --git a/pkg/golinters/unconvert.go b/pkg/golinters/unconvert.go index 5cf2462d..9c2df987 100644 --- a/pkg/golinters/unconvert.go +++ b/pkg/golinters/unconvert.go @@ -18,8 +18,7 @@ func (lint Unconvert) Run(ctx context.Context, lintCtx *Context) ([]result.Issue var res []result.Issue for _, pos := range positions { res = append(res, result.Issue{ - File: pos.Filename, - LineNumber: pos.Line, + Pos: pos, Text: "unnecessary conversion", FromLinter: lint.Name(), }) diff --git a/pkg/golinters/varcheck.go b/pkg/golinters/varcheck.go index 0d2a19e4..16b43b94 100644 --- a/pkg/golinters/varcheck.go +++ b/pkg/golinters/varcheck.go @@ -20,8 +20,7 @@ func (v Varcheck) Run(ctx context.Context, lintCtx *Context) ([]result.Issue, er var res []result.Issue for _, i := range issues { res = append(res, result.Issue{ - File: i.Pos.Filename, - LineNumber: i.Pos.Line, + Pos: i.Pos, Text: fmt.Sprintf("%s is unused", formatCode(i.VarName, lintCtx.RunCfg())), FromLinter: v.Name(), }) diff --git a/pkg/printers/json.go b/pkg/printers/json.go new file mode 100644 index 00000000..2e356730 --- /dev/null +++ b/pkg/printers/json.go @@ -0,0 +1,27 @@ +package printers + +import ( + "encoding/json" + "fmt" + + "github.com/golangci/golangci-lint/pkg/result" +) + +type JSON struct{} + +func NewJSON() *JSON { + return &JSON{} +} + +func (JSON) Print(issues chan result.Issue) (bool, error) { + var allIssues []result.Issue + for i := range issues { + allIssues = append(allIssues, i) + } + outputJSON, err := json.Marshal(allIssues) + if err != nil { + return false, err + } + fmt.Fprint(getOutWriter(), string(outputJSON)) + return len(allIssues) != 0, nil +} diff --git a/pkg/printers/printer.go b/pkg/printers/printer.go new file mode 100644 index 00000000..867ba453 --- /dev/null +++ b/pkg/printers/printer.go @@ -0,0 +1,7 @@ +package printers + +import "github.com/golangci/golangci-lint/pkg/result" + +type Printer interface { + Print(issues chan result.Issue) (bool, error) +} diff --git a/pkg/printers/text.go b/pkg/printers/text.go new file mode 100644 index 00000000..167d85df --- /dev/null +++ b/pkg/printers/text.go @@ -0,0 +1,89 @@ +package printers + +import ( + "bytes" + "fmt" + "io/ioutil" + "time" + + "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/result" + "github.com/sirupsen/logrus" +) + +type Text struct { + printIssuedLine bool + useColors bool +} + +func NewText(printIssuedLine bool, useColors bool) *Text { + return &Text{ + printIssuedLine: printIssuedLine, + useColors: useColors, + } +} + +type linesCache [][]byte +type filesCache map[string]linesCache + +func (p Text) SprintfColored(ca color.Attribute, format string, args ...interface{}) string { + if !p.useColors { + return fmt.Sprintf(format, args...) + } + + c := color.New(ca) + return c.Sprintf(format, args...) +} + +func (p Text) Print(issues chan result.Issue) (bool, error) { + var issuedLineExtractingDuration time.Duration + defer func() { + logrus.Infof("Extracting issued lines took %s", issuedLineExtractingDuration) + }() + + gotAnyIssue := false + cache := filesCache{} + out := getOutWriter() + for i := range issues { + gotAnyIssue = true + text := p.SprintfColored(color.FgRed, "%s", i.Text) + pos := p.SprintfColored(color.Bold, "%s:%d", i.FilePath(), i.Line()) + fmt.Fprintf(out, "%s: %s\n", pos, text) + + if !p.printIssuedLine { + continue + } + + fc := cache[i.FilePath()] + if fc == nil { + startedAt := time.Now() + // TODO: make more optimal algorithm: don't load all files into memory + fileBytes, err := ioutil.ReadFile(i.FilePath()) + if err != nil { + return false, fmt.Errorf("can't read file %s for printing issued line: %s", i.FilePath(), err) + } + lines := bytes.Split(fileBytes, []byte("\n")) // TODO: what about \r\n? + fc = lines + cache[i.FilePath()] = fc + issuedLineExtractingDuration += time.Since(startedAt) + } + + lineRange := i.GetLineRange() + for line := lineRange.From; line <= lineRange.To; line++ { + zeroIndexedLine := line - 1 + if zeroIndexedLine >= len(fc) { + logrus.Warnf("No line %d in file %s", line, i.FilePath()) + break + } + + fmt.Fprintln(out, string(bytes.Trim(fc[zeroIndexedLine], "\r"))) + } + } + + if !gotAnyIssue { + outStr := p.SprintfColored(color.FgGreen, "Congrats! No issues were found.") + fmt.Fprintln(out, outStr) + } + + return gotAnyIssue, nil +} diff --git a/pkg/printers/utils.go b/pkg/printers/utils.go new file mode 100644 index 00000000..28b575c7 --- /dev/null +++ b/pkg/printers/utils.go @@ -0,0 +1,11 @@ +package printers + +import ( + "io" + "os" + "syscall" +) + +func getOutWriter() io.Writer { + return os.NewFile(uintptr(syscall.Stdout), "/dev/stdout") // was set to /dev/null +} diff --git a/pkg/result/issue.go b/pkg/result/issue.go index 40b69ba0..6fce3c13 100644 --- a/pkg/result/issue.go +++ b/pkg/result/issue.go @@ -1,27 +1,35 @@ package result +import "go/token" + +type Range struct { + From, To int +} + type Issue struct { FromLinter string Text string - File string - LineNumber int - HunkPos int + + Pos token.Position + LineRange Range + HunkPos int } func (i Issue) FilePath() string { - return i.File + return i.Pos.Filename } func (i Issue) Line() int { - return i.LineNumber + return i.Pos.Line } -func NewIssue(fromLinter, text, file string, lineNumber, hunkPos int) Issue { - return Issue{ - FromLinter: fromLinter, - Text: text, - File: file, - LineNumber: lineNumber, - HunkPos: hunkPos, +func (i Issue) GetLineRange() Range { + if i.LineRange.From == 0 { + return Range{ + From: i.Line(), + To: i.Line(), + } } + + return i.LineRange } diff --git a/pkg/result/processors/max_per_file_from_linter.go b/pkg/result/processors/max_per_file_from_linter.go index 91c413d4..cea0cee6 100644 --- a/pkg/result/processors/max_per_file_from_linter.go +++ b/pkg/result/processors/max_per_file_from_linter.go @@ -36,16 +36,16 @@ func (p *MaxPerFileFromLinter) Process(issues []result.Issue) ([]result.Issue, e return true } - lm := p.flc[i.File] + lm := p.flc[i.FilePath()] if lm == nil { - p.flc[i.File] = linterToCountMap{} + p.flc[i.FilePath()] = linterToCountMap{} } - count := p.flc[i.File][i.FromLinter] + count := p.flc[i.FilePath()][i.FromLinter] if count >= limit { return false } - p.flc[i.File][i.FromLinter]++ + p.flc[i.FilePath()][i.FromLinter]++ return true }), nil } diff --git a/pkg/result/processors/nolint.go b/pkg/result/processors/nolint.go index f4004767..a14b9d12 100644 --- a/pkg/result/processors/nolint.go +++ b/pkg/result/processors/nolint.go @@ -39,19 +39,19 @@ func (p *Nolint) Process(issues []result.Issue) ([]result.Issue, error) { } func (p *Nolint) shouldPassIssue(i *result.Issue) (bool, error) { - comments := p.cache[i.File] + comments := p.cache[i.FilePath()] if comments == nil { - file, err := parser.ParseFile(p.fset, i.File, nil, parser.ParseComments) + file, err := parser.ParseFile(p.fset, i.FilePath(), nil, parser.ParseComments) if err != nil { return true, err } comments = extractFileComments(p.fset, file.Comments...) - p.cache[i.File] = comments + p.cache[i.FilePath()] = comments } for _, comment := range comments { - if comment.line != i.LineNumber { + if comment.line != i.Line() { continue } diff --git a/pkg/result/processors/nolint_test.go b/pkg/result/processors/nolint_test.go index 061f9524..775ed6c8 100644 --- a/pkg/result/processors/nolint_test.go +++ b/pkg/result/processors/nolint_test.go @@ -10,8 +10,10 @@ import ( func newNolintFileIssue(line int, fromLinter string) result.Issue { return result.Issue{ - File: filepath.Join("testdata", "nolint.go"), - LineNumber: line, + Pos: token.Position{ + Filename: filepath.Join("testdata", "nolint.go"), + Line: line, + }, FromLinter: fromLinter, } } diff --git a/pkg/result/processors/path_prettifier.go b/pkg/result/processors/path_prettifier.go index 043b8973..b68bc59c 100644 --- a/pkg/result/processors/path_prettifier.go +++ b/pkg/result/processors/path_prettifier.go @@ -30,17 +30,17 @@ func (p PathPrettifier) Name() string { func (p PathPrettifier) Process(issues []result.Issue) ([]result.Issue, error) { return transformIssues(issues, func(i *result.Issue) *result.Issue { - if !filepath.IsAbs(i.File) { + if !filepath.IsAbs(i.FilePath()) { return i } - rel, err := filepath.Rel(p.root, i.File) + rel, err := filepath.Rel(p.root, i.FilePath()) if err != nil { return i } newI := i - newI.File = rel + newI.Pos.Filename = rel return newI }), nil } diff --git a/pkg/result/processors/uniq_by_line.go b/pkg/result/processors/uniq_by_line.go index baae38a2..402ed5e2 100644 --- a/pkg/result/processors/uniq_by_line.go +++ b/pkg/result/processors/uniq_by_line.go @@ -25,19 +25,19 @@ func (p UniqByLine) Name() string { func (p *UniqByLine) Process(issues []result.Issue) ([]result.Issue, error) { return filterIssues(issues, func(i *result.Issue) bool { - lc := p.flc[i.File] + lc := p.flc[i.FilePath()] if lc == nil { lc = lineToCount{} - p.flc[i.File] = lc + p.flc[i.FilePath()] = lc } const limit = 1 - count := lc[i.LineNumber] + count := lc[i.Line()] if count == limit { return false } - lc[i.LineNumber]++ + lc[i.Line()]++ return true }), nil } diff --git a/pkg/result/processors/uniq_by_line_test.go b/pkg/result/processors/uniq_by_line_test.go index 0d0339bc..fc13a8a5 100644 --- a/pkg/result/processors/uniq_by_line_test.go +++ b/pkg/result/processors/uniq_by_line_test.go @@ -1,6 +1,7 @@ package processors import ( + "go/token" "testing" "github.com/golangci/golangci-lint/pkg/result" @@ -8,8 +9,10 @@ import ( func newFLIssue(file string, line int) result.Issue { return result.Issue{ - File: file, - LineNumber: line, + Pos: token.Position{ + Filename: file, + Line: line, + }, } }