diff --git a/Gopkg.lock b/Gopkg.lock index 07347c04..be4be608 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -11,12 +11,6 @@ revision = "a9de4d6c1589158e002cc336c495bf11fbf3ea06" source = "github.com/golangci/gas" -[[projects]] - name = "github.com/bradleyfalzon/revgrep" - packages = ["."] - revision = "c04006dc3307c8768bda7a33c7c15d1c6f664e14" - version = "v0.3" - [[projects]] name = "github.com/davecgh/go-spew" packages = ["spew"] @@ -121,6 +115,12 @@ packages = ["."] revision = "b1d89398deca2fd3f8578e5a9551e819bd01ca5f" +[[projects]] + branch = "master" + name = "github.com/golangci/revgrep" + packages = ["."] + revision = "dfd919b445ba350862d7a6e7fe81989000b84020" + [[projects]] branch = "master" name = "github.com/golangci/unconvert" @@ -296,6 +296,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "f77d7467814715b13829f36bfcd792b9d8e18c1194f44e41ba9967c2c4397374" + inputs-digest = "76708ccce918c7352a0a1ac338ffaae892409a10ef8c7cf23724bf56e7877caf" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 67f47f1f..d1fd90f6 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -26,8 +26,8 @@ [[constraint]] - name = "github.com/bradleyfalzon/revgrep" - version = "0.3.0" + name = "github.com/golangci/revgrep" + branch = "master" [[constraint]] name = "github.com/stretchr/testify" diff --git a/internal/commands/run.go b/internal/commands/run.go index b2ad91ef..a4a94ef5 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -83,6 +83,10 @@ func (e *Executor) initRun() { 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().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 { @@ -196,10 +200,11 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) (chan result. runner := pkg.SimpleRunner{ Processors: []processors.Processor{ - processors.NewMaxPerFileFromLinter(), processors.NewExclude(fmt.Sprintf("(%s)", strings.Join(e.cfg.Run.ExcludePatterns, "|"))), processors.NewNolint(lintCtx.Program.Fset), 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.NewPathPrettifier(), }, diff --git a/pkg/config/config.go b/pkg/config/config.go index 92998925..1b1209fc 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -79,6 +79,10 @@ type Run struct { Deadline time.Duration MaxIssuesPerLinter int + + DiffFromRevision string + DiffPatchFilePath string + Diff bool } type Config struct { diff --git a/pkg/result/issue.go b/pkg/result/issue.go index 56d73d27..40b69ba0 100644 --- a/pkg/result/issue.go +++ b/pkg/result/issue.go @@ -8,6 +8,14 @@ type Issue struct { 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 { return Issue{ FromLinter: fromLinter, diff --git a/pkg/result/processors/diff.go b/pkg/result/processors/diff.go index ebfb548c..67c1d6ea 100644 --- a/pkg/result/processors/diff.go +++ b/pkg/result/processors/diff.go @@ -1,92 +1,66 @@ package processors import ( + "bytes" "fmt" + "io" "io/ioutil" - "strings" - "github.com/bradleyfalzon/revgrep" "github.com/golangci/golangci-lint/pkg/result" + "github.com/golangci/revgrep" ) -type DiffProcessor struct { - patch string +type Diff struct { + onlyNew bool + fromRev string + patchFilePath string } -func NewDiffProcessor(patch string) *DiffProcessor { - return &DiffProcessor{ - patch: patch, +var _ Processor = Diff{} + +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" } -func (p DiffProcessor) processResult(res result.Result) (*result.Result, error) { - // Make mapping to restore original issues metadata later - fli := makeFilesToLinesToIssuesMap([]result.Result{res}) - - rIssues, err := p.runRevgrepOnIssues(res.Issues) - if err != nil { - return nil, err +func (p Diff) Process(issues []result.Issue) ([]result.Issue, error) { + if !p.onlyNew && p.fromRev == "" && p.patchFilePath == "" { // no need to work + return issues, nil } - newIssues := []result.Issue{} - for _, ri := range rIssues { - if fli[ri.File] == nil { - return nil, fmt.Errorf("can't get original issue file for %v", ri) - } - - oi := fli[ri.File][ri.LineNo] - if len(oi) != 1 { - return nil, fmt.Errorf("can't get original issue for %v: %v", ri, oi) - } - - i := result.Issue{ - 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) + var patchReader io.Reader + if p.patchFilePath != "" { + patch, err := ioutil.ReadFile(p.patchFilePath) if err != nil { - return nil, fmt.Errorf("can't filter only new issues for result %+v: %s", res, err) + return nil, fmt.Errorf("can't read from pathc file %s: %s", p.patchFilePath, err) } - retResults = append(retResults, *newRes) + 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) } - return retResults, nil + return transformIssues(issues, func(i *result.Issue) *result.Issue { + hunkPos, isNew := c.IsNewIssue(i) + if !isNew { + return nil + } + + newI := *i + newI.HunkPos = hunkPos + return &newI + }), 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 -} +func (Diff) Finish() {} diff --git a/pkg/result/processors/utils.go b/pkg/result/processors/utils.go index 592e3be1..29b442cd 100644 --- a/pkg/result/processors/utils.go +++ b/pkg/result/processors/utils.go @@ -2,23 +2,6 @@ package processors 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 { retIssues := make([]result.Issue, 0, len(issues)) for _, i := range issues { diff --git a/vendor/github.com/bradleyfalzon/revgrep/.gitignore b/vendor/github.com/golangci/revgrep/.gitignore similarity index 100% rename from vendor/github.com/bradleyfalzon/revgrep/.gitignore rename to vendor/github.com/golangci/revgrep/.gitignore diff --git a/vendor/github.com/bradleyfalzon/revgrep/.travis.yml b/vendor/github.com/golangci/revgrep/.travis.yml similarity index 100% rename from vendor/github.com/bradleyfalzon/revgrep/.travis.yml rename to vendor/github.com/golangci/revgrep/.travis.yml diff --git a/vendor/github.com/bradleyfalzon/revgrep/LICENSE b/vendor/github.com/golangci/revgrep/LICENSE similarity index 100% rename from vendor/github.com/bradleyfalzon/revgrep/LICENSE rename to vendor/github.com/golangci/revgrep/LICENSE diff --git a/vendor/github.com/bradleyfalzon/revgrep/README.md b/vendor/github.com/golangci/revgrep/README.md similarity index 100% rename from vendor/github.com/bradleyfalzon/revgrep/README.md rename to vendor/github.com/golangci/revgrep/README.md diff --git a/vendor/github.com/bradleyfalzon/revgrep/revgrep.go b/vendor/github.com/golangci/revgrep/revgrep.go similarity index 84% rename from vendor/github.com/bradleyfalzon/revgrep/revgrep.go rename to vendor/github.com/golangci/revgrep/revgrep.go index 9e4c2708..b8636dd2 100644 --- a/vendor/github.com/bradleyfalzon/revgrep/revgrep.go +++ b/vendor/github.com/golangci/revgrep/revgrep.go @@ -40,6 +40,9 @@ type Checker struct { // relative in order to match patch file. If not set, current working // directory is used. AbsPath string + + // Calculated changes for next calls to IsNewIssue + changes map[string][]pos } // Issue contains metadata about an issue found. @@ -61,6 +64,78 @@ type Issue struct { 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 // Checker.Patch. // @@ -72,22 +147,8 @@ type Issue struct { // File paths in reader must be relative to current working directory or // absolute. func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err error) { - // Check if patch is supplied, if not, retrieve from VCS - var ( - 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") - } - } + returnErr := c.Prepare() + writeAll := returnErr != nil // file.go:lineNo:colNo: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 // checking for recent changes - linesChanged := c.linesChanged() - c.debugf("lines changed: %+v", linesChanged) + c.debugf("lines changed: %+v", c.changes) absPath := c.AbsPath if absPath == "" { @@ -154,38 +214,23 @@ func (c Checker) Check(reader io.Reader, writer io.Writer) (issues []Issue, err msg := string(line[4]) c.debugf("path: %q, lineNo: %v, colNo: %v, msg: %q", path, lno, cno, msg) - - var ( - fpos pos - 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 - } - } - if changed || fchanges == nil { - // either file changed or it's a new file - issue := Issue{ - File: path, - LineNo: fpos.lineNo, - ColNo: int(cno), - HunkPos: fpos.lineNo, - Issue: scanner.Text(), - Message: msg, - } - if changed { - // existing file changed - issue.HunkPos = fpos.hunkPos - } - issues = append(issues, issue) - fmt.Fprintln(writer, scanner.Text()) - } + i := simpleInputIssue{ + filePath: path, + lineNumber: int(lno), } - if !changed { + hunkPos, changed := c.IsNewIssue(i) + if changed { + issue := Issue{ + File: path, + LineNo: int(lno), + ColNo: int(cno), + HunkPos: hunkPos, + Issue: scanner.Text(), + Message: msg, + } + issues = append(issues, issue) + fmt.Fprintln(writer, scanner.Text()) + } else { c.debugf("unchanged: %s", scanner.Text()) } }