Skip to content

Commit e58c27e

Browse files
jirfagjirfag
committed
move source code lines extraction to processor and store source lines in output json
1 parent f81283f commit e58c27e

File tree

9 files changed

+127
-91
lines changed

9 files changed

+127
-91
lines changed

‎pkg/commands/run.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,26 @@ func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) {
273273
return
274274
}
275275

276+
func (e *Executor) setExitCodeIfIssuesFound(issues <-chan result.Issue) <-chan result.Issue {
277+
resCh := make(chan result.Issue, 1024)
278+
279+
go func() {
280+
issuesFound := false
281+
for i := range issues {
282+
issuesFound = true
283+
resCh <- i
284+
}
285+
286+
if issuesFound {
287+
e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound
288+
}
289+
290+
close(resCh)
291+
}()
292+
293+
return resCh
294+
}
295+
276296
func (e *Executor) runAndPrint(ctx context.Context, args []string) error {
277297
if !logutils.HaveDebugTag("linters_output") {
278298
// Don't allow linters and loader to print anything
@@ -293,14 +313,10 @@ func (e *Executor) runAndPrint(ctx context.Context, args []string) error {
293313
return err
294314
}
295315

296-
gotAnyIssues, err := p.Print(ctx, issues)
297-
if err != nil {
298-
return fmt.Errorf("can't print %d issues: %s", len(issues), err)
299-
}
316+
issues = e.setExitCodeIfIssuesFound(issues)
300317

301-
if gotAnyIssues {
302-
e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound
303-
return nil
318+
if err = p.Print(ctx, issues); err != nil {
319+
return fmt.Errorf("can't print %d issues: %s", len(issues), err)
304320
}
305321

306322
return nil

‎pkg/lint/runner.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func NewRunner(astCache *astcache.Cache, cfg *config.Config, log logutils.Log) (
5555
processors.NewMaxPerFileFromLinter(),
5656
processors.NewMaxSameIssues(icfg.MaxSameIssues, log.Child("max_same_issues")),
5757
processors.NewMaxFromLinter(icfg.MaxIssuesPerLinter, log.Child("max_from_linter")),
58+
processors.NewSourceCode(log.Child("source_code")),
5859
},
5960
Log: log,
6061
}, nil

‎pkg/printers/checkstyle.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func NewCheckstyle() *Checkstyle {
3636
return &Checkstyle{}
3737
}
3838

39-
func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) {
39+
func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) error {
4040
out := checkstyleOutput{
4141
Version: "5.0",
4242
}
@@ -71,9 +71,9 @@ func (Checkstyle) Print(ctx context.Context, issues <-chan result.Issue) (bool,
7171

7272
data, err := xml.Marshal(&out)
7373
if err != nil {
74-
return false, err
74+
return err
7575
}
7676

7777
fmt.Fprintf(logutils.StdOut, "%s%s\n", xml.Header, data)
78-
return len(files) > 0, nil
78+
return nil
7979
}

‎pkg/printers/json.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type JSONResult struct {
2525
Report *report.Data
2626
}
2727

28-
func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) {
28+
func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) error {
2929
allIssues := []result.Issue{}
3030
for i := range issues {
3131
allIssues = append(allIssues, i)
@@ -38,9 +38,9 @@ func (p JSON) Print(ctx context.Context, issues <-chan result.Issue) (bool, erro
3838

3939
outputJSON, err := json.Marshal(res)
4040
if err != nil {
41-
return false, err
41+
return err
4242
}
4343

4444
fmt.Fprint(logutils.StdOut, string(outputJSON))
45-
return len(allIssues) != 0, nil
45+
return nil
4646
}

‎pkg/printers/printer.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ import (
77
)
88

99
type Printer interface {
10-
Print(ctx context.Context, issues <-chan result.Issue) (bool, error)
10+
Print(ctx context.Context, issues <-chan result.Issue) error
1111
}

‎pkg/printers/tab.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,18 @@ func (p Tab) SprintfColored(ca color.Attribute, format string, args ...interface
2828
return c.Sprintf(format, args...)
2929
}
3030

31-
func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) {
31+
func (p *Tab) Print(ctx context.Context, issues <-chan result.Issue) error {
3232
w := tabwriter.NewWriter(logutils.StdOut, 0, 0, 2, ' ', 0)
3333

34-
issuesN := 0
3534
for i := range issues {
36-
issuesN++
3735
p.printIssue(&i, w)
3836
}
3937

40-
if issuesN != 0 {
41-
p.log.Infof("Found %d issues", issuesN)
42-
}
43-
4438
if err := w.Flush(); err != nil {
4539
p.log.Warnf("Can't flush tab writer: %s", err)
4640
}
4741

48-
return issuesN != 0, nil
42+
return nil
4943
}
5044

5145
func (p Tab) printIssue(i *result.Issue, w io.Writer) {

‎pkg/printers/text.go‎

Lines changed: 11 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,27 @@
11
package printers
22

33
import (
4-
"bytes"
54
"context"
65
"fmt"
7-
"io/ioutil"
8-
"time"
96

107
"github.com/fatih/color"
118
"github.com/golangci/golangci-lint/pkg/logutils"
129
"github.com/golangci/golangci-lint/pkg/result"
1310
)
1411

15-
type linesCache [][]byte
16-
type filesCache map[string]linesCache
17-
1812
type Text struct {
1913
printIssuedLine bool
2014
useColors bool
2115
printLinterName bool
2216

23-
cache filesCache
24-
log logutils.Log
17+
log logutils.Log
2518
}
2619

2720
func NewText(printIssuedLine, useColors, printLinterName bool, log logutils.Log) *Text {
2821
return &Text{
2922
printIssuedLine: printIssuedLine,
3023
useColors: useColors,
3124
printLinterName: printLinterName,
32-
cache: filesCache{},
3325
log: log,
3426
}
3527
}
@@ -43,56 +35,19 @@ func (p Text) SprintfColored(ca color.Attribute, format string, args ...interfac
4335
return c.Sprintf(format, args...)
4436
}
4537

46-
func (p *Text) getFileLinesForIssue(i *result.Issue) (linesCache, error) {
47-
fc := p.cache[i.FilePath()]
48-
if fc != nil {
49-
return fc, nil
50-
}
51-
52-
// TODO: make more optimal algorithm: don't load all files into memory
53-
fileBytes, err := ioutil.ReadFile(i.FilePath())
54-
if err != nil {
55-
return nil, fmt.Errorf("can't read file %s for printing issued line: %s", i.FilePath(), err)
56-
}
57-
lines := bytes.Split(fileBytes, []byte("\n")) // TODO: what about \r\n?
58-
fc = lines
59-
p.cache[i.FilePath()] = fc
60-
return fc, nil
61-
}
62-
63-
func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) (bool, error) {
64-
var issuedLineExtractingDuration time.Duration
65-
defer func() {
66-
p.log.Infof("Extracting issued lines took %s", issuedLineExtractingDuration)
67-
}()
68-
69-
issuesN := 0
38+
func (p *Text) Print(ctx context.Context, issues <-chan result.Issue) error {
7039
for i := range issues {
71-
issuesN++
7240
p.printIssue(&i)
7341

7442
if !p.printIssuedLine {
7543
continue
7644
}
7745

78-
startedAt := time.Now()
79-
lines, err := p.getFileLinesForIssue(&i)
80-
if err != nil {
81-
return false, err
82-
}
83-
issuedLineExtractingDuration += time.Since(startedAt)
84-
85-
p.printIssuedLines(&i, lines)
86-
if i.Line()-1 < len(lines) {
87-
p.printUnderLinePointer(&i, string(lines[i.Line()-1]))
88-
}
89-
}
90-
91-
if issuesN != 0 {
92-
p.log.Infof("Found %d issues", issuesN)
46+
p.printSourceCode(&i)
47+
p.printUnderLinePointer(&i)
9348
}
9449

95-
return issuesN != 0, nil
50+
return nil
9651
}
9752

9853
func (p Text) printIssue(i *result.Issue) {
@@ -107,32 +62,19 @@ func (p Text) printIssue(i *result.Issue) {
10762
fmt.Fprintf(logutils.StdOut, "%s: %s\n", pos, text)
10863
}
10964

110-
func (p Text) printIssuedLines(i *result.Issue, lines linesCache) {
111-
lineRange := i.GetLineRange()
112-
var lineStr string
113-
for line := lineRange.From; line <= lineRange.To; line++ {
114-
if line == 0 { // some linters, e.g. gas can do it: it really means first line
115-
line = 1
116-
}
117-
118-
zeroIndexedLine := line - 1
119-
if zeroIndexedLine >= len(lines) {
120-
p.log.Warnf("No line %d in file %s", line, i.FilePath())
121-
break
122-
}
123-
124-
lineStr = string(bytes.Trim(lines[zeroIndexedLine], "\r"))
125-
fmt.Fprintln(logutils.StdOut, lineStr)
65+
func (p Text) printSourceCode(i *result.Issue) {
66+
for _, line := range i.SourceLines {
67+
fmt.Fprintln(logutils.StdOut, line)
12668
}
12769
}
12870

129-
func (p Text) printUnderLinePointer(i *result.Issue, line string) {
130-
lineRange := i.GetLineRange()
131-
if lineRange.From != lineRange.To || i.Pos.Column == 0 {
71+
func (p Text) printUnderLinePointer(i *result.Issue) {
72+
if len(i.SourceLines) != 1 || i.Pos.Column == 0 {
13273
return
13374
}
13475

13576
col0 := i.Pos.Column - 1
77+
line := i.SourceLines[0]
13678
prefixRunes := make([]rune, 0, len(line))
13779
for j := 0; j < len(line) && j < col0; j++ {
13880
if line[j] == '\t' {

‎pkg/result/issue.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ type Issue struct {
1313
Pos token.Position
1414
LineRange *Range `json:",omitempty"`
1515
HunkPos int `json:",omitempty"`
16+
17+
SourceLines []string
1618
}
1719

1820
func (i Issue) FilePath() string {

‎pkg/result/processors/source_code.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package processors
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"io/ioutil"
7+
8+
"github.com/golangci/golangci-lint/pkg/logutils"
9+
"github.com/golangci/golangci-lint/pkg/result"
10+
)
11+
12+
type linesCache [][]byte
13+
type filesLineCache map[string]linesCache
14+
15+
type SourceCode struct {
16+
cache filesLineCache
17+
log logutils.Log
18+
}
19+
20+
var _ Processor = SourceCode{}
21+
22+
func NewSourceCode(log logutils.Log) *SourceCode {
23+
return &SourceCode{
24+
cache: filesLineCache{},
25+
log: log,
26+
}
27+
}
28+
29+
func (p SourceCode) Name() string {
30+
return "source_code"
31+
}
32+
33+
func (p SourceCode) Process(issues []result.Issue) ([]result.Issue, error) {
34+
return transformIssues(issues, func(i *result.Issue) *result.Issue {
35+
lines, err := p.getFileLinesForIssue(i)
36+
if err != nil {
37+
p.log.Warnf("Failed to get lines for file %s: %s", i.FilePath(), err)
38+
return i
39+
}
40+
41+
newI := *i
42+
43+
lineRange := i.GetLineRange()
44+
var lineStr string
45+
for line := lineRange.From; line <= lineRange.To; line++ {
46+
if line == 0 { // some linters, e.g. gas can do it: it really means first line
47+
line = 1
48+
}
49+
50+
zeroIndexedLine := line - 1
51+
if zeroIndexedLine >= len(lines) {
52+
p.log.Warnf("No line %d in file %s", line, i.FilePath())
53+
break
54+
}
55+
56+
lineStr = string(bytes.Trim(lines[zeroIndexedLine], "\r"))
57+
newI.SourceLines = append(newI.SourceLines, lineStr)
58+
}
59+
60+
return &newI
61+
}), nil
62+
}
63+
64+
func (p *SourceCode) getFileLinesForIssue(i *result.Issue) (linesCache, error) {
65+
fc := p.cache[i.FilePath()]
66+
if fc != nil {
67+
return fc, nil
68+
}
69+
70+
// TODO: make more optimal algorithm: don't load all files into memory
71+
fileBytes, err := ioutil.ReadFile(i.FilePath())
72+
if err != nil {
73+
return nil, fmt.Errorf("can't read file %s for printing issued line: %s", i.FilePath(), err)
74+
}
75+
lines := bytes.Split(fileBytes, []byte("\n")) // TODO: what about \r\n?
76+
fc = lines
77+
p.cache[i.FilePath()] = fc
78+
return fc, nil
79+
}
80+
81+
func (p SourceCode) Finish() {}

0 commit comments

Comments
 (0)