support diff options

This commit is contained in:
golangci 2018-05-08 09:55:38 +03:00
parent 393733fa6a
commit 073ad51ed9
12 changed files with 162 additions and 143 deletions

14
Gopkg.lock generated
View File

@ -11,12 +11,6 @@
revision = "a9de4d6c1589158e002cc336c495bf11fbf3ea06" revision = "a9de4d6c1589158e002cc336c495bf11fbf3ea06"
source = "github.com/golangci/gas" source = "github.com/golangci/gas"
[[projects]]
name = "github.com/bradleyfalzon/revgrep"
packages = ["."]
revision = "c04006dc3307c8768bda7a33c7c15d1c6f664e14"
version = "v0.3"
[[projects]] [[projects]]
name = "github.com/davecgh/go-spew" name = "github.com/davecgh/go-spew"
packages = ["spew"] packages = ["spew"]
@ -121,6 +115,12 @@
packages = ["."] packages = ["."]
revision = "b1d89398deca2fd3f8578e5a9551e819bd01ca5f" revision = "b1d89398deca2fd3f8578e5a9551e819bd01ca5f"
[[projects]]
branch = "master"
name = "github.com/golangci/revgrep"
packages = ["."]
revision = "dfd919b445ba350862d7a6e7fe81989000b84020"
[[projects]] [[projects]]
branch = "master" branch = "master"
name = "github.com/golangci/unconvert" name = "github.com/golangci/unconvert"
@ -296,6 +296,6 @@
[solve-meta] [solve-meta]
analyzer-name = "dep" analyzer-name = "dep"
analyzer-version = 1 analyzer-version = 1
inputs-digest = "f77d7467814715b13829f36bfcd792b9d8e18c1194f44e41ba9967c2c4397374" inputs-digest = "76708ccce918c7352a0a1ac338ffaae892409a10ef8c7cf23724bf56e7877caf"
solver-name = "gps-cdcl" solver-name = "gps-cdcl"
solver-version = 1 solver-version = 1

View File

@ -26,8 +26,8 @@
[[constraint]] [[constraint]]
name = "github.com/bradleyfalzon/revgrep" name = "github.com/golangci/revgrep"
version = "0.3.0" branch = "master"
[[constraint]] [[constraint]]
name = "github.com/stretchr/testify" name = "github.com/stretchr/testify"

View File

@ -83,6 +83,10 @@ func (e *Executor) initRun() {
runCmd.Flags().StringSliceVarP(&rc.ExcludePatterns, "exclude", "e", config.DefaultExcludePatterns, "Exclude issue by regexp") runCmd.Flags().StringSliceVarP(&rc.ExcludePatterns, "exclude", "e", config.DefaultExcludePatterns, "Exclude issue by regexp")
runCmd.Flags().IntVar(&rc.MaxIssuesPerLinter, "max-issues-per-linter", 50, "Maximum issues count per one linter. Set to 0 to disable") runCmd.Flags().IntVar(&rc.MaxIssuesPerLinter, "max-issues-per-linter", 50, "Maximum issues count per one linter. Set to 0 to disable")
runCmd.Flags().BoolVarP(&rc.Diff, "new", "n", false, "Show only new issues: if there are unstaged changes or untracked files, only those changes are shown, else only changes in HEAD~ are shown")
runCmd.Flags().StringVar(&rc.DiffFromRevision, "new-from-rev", "", "Show only new issues created after git revision `REV`")
runCmd.Flags().StringVar(&rc.DiffPatchFilePath, "new-from-patch", "", "Show only new issues created in git patch with file path `PATH`")
} }
func isFullImportNeeded(linters []pkg.Linter) bool { func isFullImportNeeded(linters []pkg.Linter) bool {
@ -196,10 +200,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (chan result.
runner := pkg.SimpleRunner{ runner := pkg.SimpleRunner{
Processors: []processors.Processor{ Processors: []processors.Processor{
processors.NewMaxPerFileFromLinter(),
processors.NewExclude(fmt.Sprintf("(%s)", strings.Join(e.cfg.Run.ExcludePatterns, "|"))), processors.NewExclude(fmt.Sprintf("(%s)", strings.Join(e.cfg.Run.ExcludePatterns, "|"))),
processors.NewNolint(lintCtx.Program.Fset), processors.NewNolint(lintCtx.Program.Fset),
processors.NewUniqByLine(), processors.NewUniqByLine(),
processors.NewDiff(e.cfg.Run.Diff, e.cfg.Run.DiffFromRevision, e.cfg.Run.DiffPatchFilePath),
processors.NewMaxPerFileFromLinter(),
processors.NewMaxFromLinter(e.cfg.Run.MaxIssuesPerLinter), processors.NewMaxFromLinter(e.cfg.Run.MaxIssuesPerLinter),
processors.NewPathPrettifier(), processors.NewPathPrettifier(),
}, },

View File

@ -79,6 +79,10 @@ type Run struct {
Deadline time.Duration Deadline time.Duration
MaxIssuesPerLinter int MaxIssuesPerLinter int
DiffFromRevision string
DiffPatchFilePath string
Diff bool
} }
type Config struct { type Config struct {

View File

@ -8,6 +8,14 @@ type Issue struct {
HunkPos int HunkPos int
} }
func (i Issue) FilePath() string {
return i.File
}
func (i Issue) Line() int {
return i.LineNumber
}
func NewIssue(fromLinter, text, file string, lineNumber, hunkPos int) Issue { func NewIssue(fromLinter, text, file string, lineNumber, hunkPos int) Issue {
return Issue{ return Issue{
FromLinter: fromLinter, FromLinter: fromLinter,

View File

@ -1,92 +1,66 @@
package processors package processors
import ( import (
"bytes"
"fmt" "fmt"
"io"
"io/ioutil" "io/ioutil"
"strings"
"github.com/bradleyfalzon/revgrep"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
"github.com/golangci/revgrep"
) )
type DiffProcessor struct { type Diff struct {
patch string onlyNew bool
fromRev string
patchFilePath string
} }
func NewDiffProcessor(patch string) *DiffProcessor { var _ Processor = Diff{}
return &DiffProcessor{
patch: patch, func NewDiff(onlyNew bool, fromRev, patchFilePath string) *Diff {
return &Diff{
onlyNew: onlyNew,
fromRev: fromRev,
patchFilePath: patchFilePath,
} }
} }
func (p DiffProcessor) Name() string { func (p Diff) Name() string {
return "diff" return "diff"
} }
func (p DiffProcessor) processResult(res result.Result) (*result.Result, error) { func (p Diff) Process(issues []result.Issue) ([]result.Issue, error) {
// Make mapping to restore original issues metadata later if !p.onlyNew && p.fromRev == "" && p.patchFilePath == "" { // no need to work
fli := makeFilesToLinesToIssuesMap([]result.Result{res}) return issues, nil
}
rIssues, err := p.runRevgrepOnIssues(res.Issues) var patchReader io.Reader
if p.patchFilePath != "" {
patch, err := ioutil.ReadFile(p.patchFilePath)
if err != nil { if err != nil {
return nil, err return nil, fmt.Errorf("can't read from pathc file %s: %s", p.patchFilePath, err)
}
patchReader = bytes.NewReader(patch)
}
c := revgrep.Checker{
Patch: patchReader,
RevisionFrom: p.fromRev,
}
if err := c.Prepare(); err != nil {
return nil, fmt.Errorf("can't prepare diff by revgrep: %s", err)
} }
newIssues := []result.Issue{} return transformIssues(issues, func(i *result.Issue) *result.Issue {
for _, ri := range rIssues { hunkPos, isNew := c.IsNewIssue(i)
if fli[ri.File] == nil { if !isNew {
return nil, fmt.Errorf("can't get original issue file for %v", ri) return nil
} }
oi := fli[ri.File][ri.LineNo] newI := *i
if len(oi) != 1 { newI.HunkPos = hunkPos
return nil, fmt.Errorf("can't get original issue for %v: %v", ri, oi) return &newI
}), nil
} }
i := result.Issue{ func (Diff) Finish() {}
File: ri.File,
LineNumber: ri.LineNo,
Text: ri.Message,
HunkPos: ri.HunkPos,
FromLinter: oi[0].FromLinter,
}
newIssues = append(newIssues, i)
}
res.Issues = newIssues
return &res, nil
}
func (p DiffProcessor) Process(results []result.Result) ([]result.Result, error) {
retResults := []result.Result{}
for _, res := range results {
newRes, err := p.processResult(res)
if err != nil {
return nil, fmt.Errorf("can't filter only new issues for result %+v: %s", res, err)
}
retResults = append(retResults, *newRes)
}
return retResults, nil
}
func (p DiffProcessor) runRevgrepOnIssues(issues []result.Issue) ([]revgrep.Issue, error) {
// TODO: change revgrep to accept interface with line number, file name
fakeIssuesLines := []string{}
for _, i := range issues {
line := fmt.Sprintf("%s:%d:%d: %s", i.File, i.LineNumber, 0, i.Text)
fakeIssuesLines = append(fakeIssuesLines, line)
}
fakeIssuesOut := strings.Join(fakeIssuesLines, "\n")
checker := revgrep.Checker{
Patch: strings.NewReader(p.patch),
Regexp: `^([^:]+):(\d+):(\d+)?:?\s*(.*)$`,
}
rIssues, err := checker.Check(strings.NewReader(fakeIssuesOut), ioutil.Discard)
if err != nil {
return nil, fmt.Errorf("can't filter only new issues by revgrep: %s", err)
}
return rIssues, nil
}

View File

@ -2,23 +2,6 @@ package processors
import "github.com/golangci/golangci-lint/pkg/result" import "github.com/golangci/golangci-lint/pkg/result"
type linesToIssuesMap map[int][]result.Issue
type filesToLinesToIssuesMap map[string]linesToIssuesMap
func makeFilesToLinesToIssuesMap(results []result.Result) filesToLinesToIssuesMap {
fli := filesToLinesToIssuesMap{}
for _, res := range results {
for _, i := range res.Issues {
if fli[i.File] == nil {
fli[i.File] = linesToIssuesMap{}
}
li := fli[i.File]
li[i.LineNumber] = append(li[i.LineNumber], i)
}
}
return fli
}
func filterIssues(issues []result.Issue, filter func(i *result.Issue) bool) []result.Issue { func filterIssues(issues []result.Issue, filter func(i *result.Issue) bool) []result.Issue {
retIssues := make([]result.Issue, 0, len(issues)) retIssues := make([]result.Issue, 0, len(issues))
for _, i := range issues { for _, i := range issues {

View File

@ -40,6 +40,9 @@ type Checker struct {
// relative in order to match patch file. If not set, current working // relative in order to match patch file. If not set, current working
// directory is used. // directory is used.
AbsPath string AbsPath string
// Calculated changes for next calls to IsNewIssue
changes map[string][]pos
} }
// Issue contains metadata about an issue found. // Issue contains metadata about an issue found.
@ -61,6 +64,78 @@ type Issue struct {
Message string Message string
} }
func (c *Checker) preparePatch() error {
// Check if patch is supplied, if not, retrieve from VCS
if c.Patch == nil {
var err error
c.Patch, c.NewFiles, err = GitPatch(c.RevisionFrom, c.RevisionTo)
if err != nil {
return fmt.Errorf("could not read git repo: %s", err)
}
if c.Patch == nil {
return errors.New("no version control repository found")
}
}
return nil
}
type InputIssue interface {
FilePath() string
Line() int
}
type simpleInputIssue struct {
filePath string
lineNumber int
}
func (i simpleInputIssue) FilePath() string {
return i.filePath
}
func (i simpleInputIssue) Line() int {
return i.lineNumber
}
func (c *Checker) Prepare() error {
returnErr := c.preparePatch()
c.changes = c.linesChanged()
return returnErr
}
func (c Checker) IsNewIssue(i InputIssue) (hunkPos int, isNew bool) {
fchanges, ok := c.changes[i.FilePath()]
if !ok { // file wasn't changed
return 0, false
}
var (
fpos pos
changed bool
)
// found file, see if lines matched
for _, pos := range fchanges {
if pos.lineNo == int(i.Line()) {
fpos = pos
changed = true
break
}
}
if changed || fchanges == nil {
// either file changed or it's a new file
hunkPos := fpos.lineNo
if changed { // existing file changed
hunkPos = fpos.hunkPos
}
return hunkPos, true
}
return 0, false
}
// Check scans reader and writes any lines to writer that have been added in // Check scans reader and writes any lines to writer that have been added in
// Checker.Patch. // Checker.Patch.
// //
@ -72,22 +147,8 @@ type Issue struct {
// File paths in reader must be relative to current working directory or // File paths in reader must be relative to current working directory or
// absolute. // absolute.
func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err error) { func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err error) {
// Check if patch is supplied, if not, retrieve from VCS returnErr := c.Prepare()
var ( writeAll := returnErr != nil
writeAll bool
returnErr error
)
if c.Patch == nil {
c.Patch, c.NewFiles, err = GitPatch(c.RevisionFrom, c.RevisionTo)
if err != nil {
writeAll = true
returnErr = fmt.Errorf("could not read git repo: %s", err)
}
if c.Patch == nil {
writeAll = true
returnErr = errors.New("no version control repository found")
}
}
// file.go:lineNo:colNo:message // file.go:lineNo:colNo:message
// colNo is optional, strip spaces before message // colNo is optional, strip spaces before message
@ -101,8 +162,7 @@ func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err
// TODO consider lazy loading this, if there's nothing in stdin, no point // TODO consider lazy loading this, if there's nothing in stdin, no point
// checking for recent changes // checking for recent changes
linesChanged := c.linesChanged() c.debugf("lines changed: %+v", c.changes)
c.debugf("lines changed: %+v", linesChanged)
absPath := c.AbsPath absPath := c.AbsPath
if absPath == "" { if absPath == "" {
@ -154,38 +214,23 @@ func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err
msg := string(line[4]) msg := string(line[4])
c.debugf("path: %q, lineNo: %v, colNo: %v, msg: %q", path, lno, cno, msg) c.debugf("path: %q, lineNo: %v, colNo: %v, msg: %q", path, lno, cno, msg)
i := simpleInputIssue{
var ( filePath: path,
fpos pos lineNumber: int(lno),
changed bool
)
if fchanges, ok := linesChanged[path]; ok {
// found file, see if lines matched
for _, pos := range fchanges {
if pos.lineNo == int(lno) {
fpos = pos
changed = true
} }
} hunkPos, changed := c.IsNewIssue(i)
if changed || fchanges == nil { if changed {
// either file changed or it's a new file
issue := Issue{ issue := Issue{
File: path, File: path,
LineNo: fpos.lineNo, LineNo: int(lno),
ColNo: int(cno), ColNo: int(cno),
HunkPos: fpos.lineNo, HunkPos: hunkPos,
Issue: scanner.Text(), Issue: scanner.Text(),
Message: msg, Message: msg,
} }
if changed {
// existing file changed
issue.HunkPos = fpos.hunkPos
}
issues = append(issues, issue) issues = append(issues, issue)
fmt.Fprintln(writer, scanner.Text()) fmt.Fprintln(writer, scanner.Text())
} } else {
}
if !changed {
c.debugf("unchanged: %s", scanner.Text()) c.debugf("unchanged: %s", scanner.Text())
} }
} }