pretty printing issues

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)
-}

pkg/config/config.go

@@ -27,7 +27,9 @@ type Run struct {
 
 	BuildTags []string
 
-	OutFormat             string
+	OutFormat       string
+	PrintIssuedLine bool
+
 	ExitCodeIfIssuesFound int
 
 	Errcheck struct {

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(),
 		})

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(),
 		})

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,
 		})
 	}
 

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(),
 		})

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(),
 		})

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(),

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)
 		}

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
 		}

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(),
 		})

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(),
 		})

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(),
 		})

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(),
 		})

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(),
 		})

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(),
 		})

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(),
 		})

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(),
 		})

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
+}

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)
+}

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
+}

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
+}

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
 }

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
 }

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
 		}
 

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,
 	}
 }

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
 }

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
 }

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,
+		},
 	}
 }