From e58c27e463d33525199e355e3ee16783daee2c14 Mon Sep 17 00:00:00 2001 From: Denis Isaev Date: Sat, 18 Aug 2018 15:41:17 +0300 Subject: [PATCH] move source code lines extraction to processor and store source lines in output json --- pkg/commands/run.go | 30 ++++++++--- pkg/lint/runner.go | 1 + pkg/printers/checkstyle.go | 6 +-- pkg/printers/json.go | 6 +-- pkg/printers/printer.go | 2 +- pkg/printers/tab.go | 10 +--- pkg/printers/text.go | 80 ++++----------------------- pkg/result/issue.go | 2 + pkg/result/processors/source_code.go | 81 ++++++++++++++++++++++++++++ 9 files changed, 127 insertions(+), 91 deletions(-) create mode 100644 pkg/result/processors/source_code.go diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 04776a3e..ab9e2136 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -273,6 +273,26 @@ func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { return } +func (e *Executor) setExitCodeIfIssuesFound(issues <-chan result.Issue) <-chan result.Issue { + resCh := make(chan result.Issue, 1024) + + go func() { + issuesFound := false + for i := range issues { + issuesFound = true + resCh <- i + } + + if issuesFound { + e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound + } + + close(resCh) + }() + + return resCh +} + func (e *Executor) runAndPrint(ctx context.Context, args []string) error { if !logutils.HaveDebugTag("linters_output") { // Don't allow linters and loader to print anything @@ -293,14 +313,10 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error { return err } - gotAnyIssues, err := p.Print(ctx, issues) - if err != nil { - return fmt.Errorf("can't print %d issues: %s", len(issues), err) - } + issues = e.setExitCodeIfIssuesFound(issues) - if gotAnyIssues { - e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound - return nil + if err = p.Print(ctx, issues); err != nil { + return fmt.Errorf("can't print %d issues: %s", len(issues), err) } return nil diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 50c7b070..0a32cbbd 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -55,6 +55,7 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log) ( processors.NewMaxPerFileFromLinter(), processors.NewMaxSameIssues(icfg.MaxSameIssues, log.Child("max_same_issues")), processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter")), + processors.NewSourceCode(log.Child("source_code")), }, Log: log, }, nil diff --git a/pkg/printers/checkstyle.go b/pkg/printers/checkstyle.go index 8ba01a53..8853b6ee 100644 --- a/pkg/printers/checkstyle.go +++ b/pkg/printers/checkstyle.go @@ -36,7 +36,7 @@ func NewCheckstyle() *Checkstyle { return &Checkstyle{} } -func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { +func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) error { out := checkstyleOutput{ Version: "5.0", } @@ -71,9 +71,9 @@ func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) (bool, data, err := xml.Marshal(&out) if err != nil { - return false, err + return err } fmt.Fprintf(logutils.StdOut, "%s%s\n", xml.Header, data) - return len(files) > 0, nil + return nil } diff --git a/pkg/printers/json.go b/pkg/printers/json.go index adb665fd..c3778fd3 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -25,7 +25,7 @@ type JSONResult struct { Report *report.Data } -func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { +func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) error { allIssues := []result.Issue{} for i := range issues { allIssues = append(allIssues, i) @@ -38,9 +38,9 @@ func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) (bool, erro outputJSON, err := json.Marshal(res) if err != nil { - return false, err + return err } fmt.Fprint(logutils.StdOut, string(outputJSON)) - return len(allIssues) != 0, nil + return nil } diff --git a/pkg/printers/printer.go b/pkg/printers/printer.go index 667e6408..a063bd91 100644 --- a/pkg/printers/printer.go +++ b/pkg/printers/printer.go @@ -7,5 +7,5 @@ import ( ) type Printer interface { - Print(ctx context.Context, issues <-chan result.Issue) (bool, error) + Print(ctx context.Context, issues <-chan result.Issue) error } diff --git a/pkg/printers/tab.go b/pkg/printers/tab.go index fe0373c9..56170bbd 100644 --- a/pkg/printers/tab.go +++ b/pkg/printers/tab.go @@ -28,24 +28,18 @@ func (p Tab) SprintfColored(ca color.Attribute, format string, args ...interface return c.Sprintf(format, args...) } -func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { +func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) error { w := tabwriter.NewWriter(logutils.StdOut, 0, 0, 2, ' ', 0) - issuesN := 0 for i := range issues { - issuesN++ p.printIssue(&i, w) } - if issuesN != 0 { - p.log.Infof("Found %d issues", issuesN) - } - if err := w.Flush(); err != nil { p.log.Warnf("Can't flush tab writer: %s", err) } - return issuesN != 0, nil + return nil } func (p Tab) printIssue(i *result.Issue, w io.Writer) { diff --git a/pkg/printers/text.go b/pkg/printers/text.go index 2133f085..08023382 100644 --- a/pkg/printers/text.go +++ b/pkg/printers/text.go @@ -1,27 +1,20 @@ package printers import ( - "bytes" "context" "fmt" - "io/ioutil" - "time" "github.com/fatih/color" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) -type linesCache [][]byte -type filesCache map[string]linesCache - type Text struct { printIssuedLine bool useColors bool printLinterName bool - cache filesCache - log logutils.Log + log logutils.Log } func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log) *Text { @@ -29,7 +22,6 @@ func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log) printIssuedLine: printIssuedLine, useColors: useColors, printLinterName: printLinterName, - cache: filesCache{}, log: log, } } @@ -43,56 +35,19 @@ func (p Text) SprintfColored(ca color.Attribute, format string, args ...interfac return c.Sprintf(format, args...) } -func (p *Text) getFileLinesForIssue(i *result.Issue) (linesCache, error) { - fc := p.cache[i.FilePath()] - if fc != nil { - return fc, nil - } - - // TODO: make more optimal algorithm: don't load all files into memory - fileBytes, err := ioutil.ReadFile(i.FilePath()) - if err != nil { - return nil, 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 - p.cache[i.FilePath()] = fc - return fc, nil -} - -func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) { - var issuedLineExtractingDuration time.Duration - defer func() { - p.log.Infof("Extracting issued lines took %s", issuedLineExtractingDuration) - }() - - issuesN := 0 +func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) error { for i := range issues { - issuesN++ p.printIssue(&i) if !p.printIssuedLine { continue } - startedAt := time.Now() - lines, err := p.getFileLinesForIssue(&i) - if err != nil { - return false, err - } - issuedLineExtractingDuration += time.Since(startedAt) - - p.printIssuedLines(&i, lines) - if i.Line()-1 < len(lines) { - p.printUnderLinePointer(&i, string(lines[i.Line()-1])) - } + p.printSourceCode(&i) + p.printUnderLinePointer(&i) } - if issuesN != 0 { - p.log.Infof("Found %d issues", issuesN) - } - - return issuesN != 0, nil + return nil } func (p Text) printIssue(i *result.Issue) { @@ -107,32 +62,19 @@ func (p Text) printIssue(i *result.Issue) { fmt.Fprintf(logutils.StdOut, "%s: %s\n", pos, text) } -func (p Text) printIssuedLines(i *result.Issue, lines linesCache) { - lineRange := i.GetLineRange() - var lineStr string - for line := lineRange.From; line <= lineRange.To; line++ { - if line == 0 { // some linters, e.g. gas can do it: it really means first line - line = 1 - } - - zeroIndexedLine := line - 1 - if zeroIndexedLine >= len(lines) { - p.log.Warnf("No line %d in file %s", line, i.FilePath()) - break - } - - lineStr = string(bytes.Trim(lines[zeroIndexedLine], "\r")) - fmt.Fprintln(logutils.StdOut, lineStr) +func (p Text) printSourceCode(i *result.Issue) { + for _, line := range i.SourceLines { + fmt.Fprintln(logutils.StdOut, line) } } -func (p Text) printUnderLinePointer(i *result.Issue, line string) { - lineRange := i.GetLineRange() - if lineRange.From != lineRange.To || i.Pos.Column == 0 { +func (p Text) printUnderLinePointer(i *result.Issue) { + if len(i.SourceLines) != 1 || i.Pos.Column == 0 { return } col0 := i.Pos.Column - 1 + line := i.SourceLines[0] prefixRunes := make([]rune, 0, len(line)) for j := 0; j < len(line) && j < col0; j++ { if line[j] == '\t' { diff --git a/pkg/result/issue.go b/pkg/result/issue.go index 62c95147..39281e04 100644 --- a/pkg/result/issue.go +++ b/pkg/result/issue.go @@ -13,6 +13,8 @@ type Issue struct { Pos token.Position LineRange *Range `json:",omitempty"` HunkPos int `json:",omitempty"` + + SourceLines []string } func (i Issue) FilePath() string { diff --git a/pkg/result/processors/source_code.go b/pkg/result/processors/source_code.go new file mode 100644 index 00000000..eaa8036e --- /dev/null +++ b/pkg/result/processors/source_code.go @@ -0,0 +1,81 @@ +package processors + +import ( + "bytes" + "fmt" + "io/ioutil" + + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result" +) + +type linesCache [][]byte +type filesLineCache map[string]linesCache + +type SourceCode struct { + cache filesLineCache + log logutils.Log +} + +var _ Processor = SourceCode{} + +func NewSourceCode(log logutils.Log) *SourceCode { + return &SourceCode{ + cache: filesLineCache{}, + log: log, + } +} + +func (p SourceCode) Name() string { + return "source_code" +} + +func (p SourceCode) Process(issues []result.Issue) ([]result.Issue, error) { + return transformIssues(issues, func(i *result.Issue) *result.Issue { + lines, err := p.getFileLinesForIssue(i) + if err != nil { + p.log.Warnf("Failed to get lines for file %s: %s", i.FilePath(), err) + return i + } + + newI := *i + + lineRange := i.GetLineRange() + var lineStr string + for line := lineRange.From; line <= lineRange.To; line++ { + if line == 0 { // some linters, e.g. gas can do it: it really means first line + line = 1 + } + + zeroIndexedLine := line - 1 + if zeroIndexedLine >= len(lines) { + p.log.Warnf("No line %d in file %s", line, i.FilePath()) + break + } + + lineStr = string(bytes.Trim(lines[zeroIndexedLine], "\r")) + newI.SourceLines = append(newI.SourceLines, lineStr) + } + + return &newI + }), nil +} + +func (p *SourceCode) getFileLinesForIssue(i *result.Issue) (linesCache, error) { + fc := p.cache[i.FilePath()] + if fc != nil { + return fc, nil + } + + // TODO: make more optimal algorithm: don't load all files into memory + fileBytes, err := ioutil.ReadFile(i.FilePath()) + if err != nil { + return nil, 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 + p.cache[i.FilePath()] = fc + return fc, nil +} + +func (p SourceCode) Finish() {}