--new/--new-from-rev flags does not work

[invidian]

It seems that --new or/and --new-from-rev flags are not really working. I've created test repository to demonstrate: https://github.com/invidian/golangci-lint-new-from-rev

There are 2 branches, master with one issue and new with 2 issues. When running following command on new branch:

golangci-lint run --enable-all --max-same-issues=0 --max-issues-per-linter=0 --new-from-rev=master -v

I would expect to just see one warning introduced in commit 0a550855f7a5573e791060cb1f3ab1e6a5eacbee. However, I see none. The same thing happens, when I introduce new code, which should result in warning and I run the linter with --new.

When running without --new and/or --new-from-rev, everything seems fine. My guess is, that https://github.com/golangci/revgrep is just filtering all found issues, instead of just old ones. I've tried testing the following:

golangci-lint run --enable-all --max-same-issues=0 --max-issues-per-linter=0 |& revgrep master

But it gives the same result (no warnings printed). And the debug output:

DEBUG: diff --git main.go main.go
DEBUG: index 0f87f3a..6965c1b 100644
DEBUG: --- main.go
DEBUG: +++ main.go
DEBUG: @@ -8,4 +8,6 @@ func main() {
DEBUG:  	}
DEBUG:  	a := 5
DEBUG:  	fmt.Println(a)
DEBUG: +	b := 5
DEBUG: +	fmt.Println(b)
DEBUG:  }
DEBUG: lines changed: map[in.go:[{lineNo:11 hunkPos:4} {lineNo:12 hunkPos:5}]]
DEBUG: path: "main.go", lineNo: 9, colNo: 2, msg: "assignments should only be cuddled with other assignments (wsl)"
DEBUG: unchanged: main.go:9:2: assignments should only be cuddled with other assignments (wsl)
DEBUG: cannot parse file+line number: 	a := 5
DEBUG: cannot parse file+line number: 	^
DEBUG: path: "main.go", lineNo: 11, colNo: 2, msg: "assignments should only be cuddled with other assignments (wsl)"
DEBUG: unchanged: main.go:11:2: assignments should only be cuddled with other assignments (wsl)
DEBUG: cannot parse file+line number: 	b := 5
DEBUG: cannot parse file+line number: 	^

Thank you for creating the issue!

• Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I've searched similar issues on GitHub and didn't find any.
• Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.23.1 built from 567904e on 2020-01-20T08:00:15Z

Config file

$ cat .golangci.yml
cat: .golangci.yml: No such file or directory

Go environment

$ go version && go env
go version go1.13.7 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/invidian/.cache/go-build"
GOENV="/home/invidian/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/invidian/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build048887854=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint run --enable-all --max-same-issues=0 --max-issues-per-linter=0 --new-from-rev=master -v
INFO [config_reader] Config search paths: [./ /home/invidian/data/workspaces/golangci-list-rev /home/invidian/data/workspaces /home/invidian/data /home/invidian /home /] 
INFO [lintersdb] Active 41 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen gochecknoglobals gochecknoinits gocognit goconst gocritic gocyclo godox gofmt goimports golint gomnd goprintffuncname gosec gosimple govet ineffassign interfacer lll maligned misspell nakedret prealloc rowserrcheck scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace wsl] 
INFO [lintersdb] Active 41 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen gochecknoglobals gochecknoinits gocognit goconst gocritic gocyclo godox gofmt goimports golint gomnd goprintffuncname gosec gosimple govet ineffassign interfacer lll maligned misspell nakedret prealloc rowserrcheck scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace wsl] 
INFO [loader] Go packages loading at mode 575 (types_sizes|exports_file|files|imports|compiled_files|deps|name) took 63.749191ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 81.682µs 
INFO [runner/goanalysis_metalinter/goanalysis] analyzers took 0s with no stages 
INFO [runner/unused/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 2, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_files: 2/2, exclude-rules: 2/2, nolint: 2/2, cgo: 2/2, path_prettifier: 2/2, exclude: 2/2, autogenerated_exclude: 2/2, uniq_by_line: 2/2, diff: 0/2, filename_unadjuster: 2/2, skip_dirs: 2/2, identifier_marker: 2/2 
INFO [runner] processing took 3.870694ms with stages: diff: 3.663416ms, exclude: 90.573µs, identifier_marker: 43.955µs, nolint: 32.977µs, path_prettifier: 17.167µs, autogenerated_exclude: 10.117µs, skip_dirs: 5.821µs, uniq_by_line: 2.203µs, cgo: 1.578µs, max_same_issues: 826ns, filename_unadjuster: 689ns, path_shortener: 281ns, max_from_linter: 269ns, max_per_file_from_linter: 266ns, exclude-rules: 191ns, source_code: 190ns, skip_files: 175ns 
INFO [runner] linters took 44.934301ms with stages: goanalysis_metalinter: 40.766266ms, unused: 226.976µs 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 3 samples, avg is 69.7MB, max is 69.9MB 
INFO Execution took 114.702165ms

[invidian]

Some additional findings:

• it seems the issue has something to do with matching file names. I have following line in ~/.gitconfig:

  [diff]
    noprefix = true
  

  I doubt it helps.
• I added following patch and with noprefix = false, it seems to work now:

  $ git diff
  diff --git pkg/result/processors/diff.go pkg/result/processors/diff.go
  index fc4aba4..45e8e8d 100644
  --- pkg/result/processors/diff.go
  +++ pkg/result/processors/diff.go
  @@ -54,6 +54,7 @@ func (p Diff) Process(issues []result.Issue) ([]result.Issue, error) {
          c := revgrep.Checker{
                  Patch:        patchReader,
                  RevisionFrom: p.fromRev,
  +               Regexp:       `.\/(.*?\.go):([0-9]+):([0-9]+)?:?\s*(.*)`,
          }
          if err := c.Prepare(); err != nil {
                  return nil, fmt.Errorf("can't prepare diff by revgrep: %s", err)

Perhaps that could be reported to https://github.com/bradleyfalzon/revgrep, but it seems unmaintained and https://github.com/golangci/revgrep does not have issue tracking enabled...

Edit: so it seems there are 2 issues here:

• revgrep does not handle output from go vet correctly (as file paths start with ./)
• revgrep does not work correctly with golangci-lint, when user has diff.noprefix = true set in their .gitconfig.

[invidian]

I guess following patch will ensure, that prefixes are always there:

diff --git a/revgrep.go b/revgrep.go
index 3650d64..d9b9942 100644
--- a/revgrep.go
+++ b/revgrep.go
@@ -356,7 +356,7 @@ func GitPatch(revisionFrom, revisionTo string) (io.Reader, []string, error) {
        }
 
        if revisionFrom != "" {
-               cmd := exec.Command("git", "diff", "--relative", revisionFrom)
+               cmd := exec.Command("git", "diff", "-src-prefix=a/", "--dst-prefix=b/", "--relative", revisionFrom)
                if revisionTo != "" {
                        cmd.Args = append(cmd.Args, revisionTo)
                }
@@ -373,7 +373,7 @@ func GitPatch(revisionFrom, revisionTo string) (io.Reader, []string, error) {
 
        // make a patch for unstaged changes
        // use --no-prefix to remove b/ given: +++ b/main.go
-       cmd := exec.Command("git", "diff", "--relative")
+       cmd := exec.Command("git", "diff", "--src-prefix=a/", "--dst-prefix=b/", "--relative")
        cmd.Stdout = &patch
        if err := cmd.Run(); err != nil {
                return nil, nil, fmt.Errorf("error executing git diff: %s", err)
@@ -388,7 +388,7 @@ func GitPatch(revisionFrom, revisionTo string) (io.Reader, []string, error) {
 
        // check for changes in recent commit
 
-       cmd = exec.Command("git", "diff", "--relative", "HEAD~")
+       cmd = exec.Command("git", "diff", "--src-prefix=a/", "--dst-prefix=b/", "--relative", "HEAD~")
        cmd.Stdout = &patch
        if err := cmd.Run(); err != nil {
                return nil, nil, fmt.Errorf("error executing git diff HEAD~: %s", err)

[tbarbugli]

I am having a different problem with the same flag (--new-from-rev); on TravisCI I get

WARN [runner] Can't process result by diff processor: can't prepare diff by revgrep: could not read git repo: error executing git diff "origin/master" "": exit status 128

At first I thought this had something to do with how Travis clones repos (--depth) but that's not the case.

[jlucktay]

I'm also seeing some issues with the --new-from-rev flag in v1.23.6 with the diff processor basically throwing everything out.
This is from running a pretty basic filter, new issues since branching away from origin/master: golangci-lint run --new-from-rev=origin/master --verbose:

INFO [runner] Processors filtering stat (out/in): cgo: 9160/9160,
path_prettifier: 9160/9160, skip_dirs: 9160/9160, exclude-rules: 7120/7120,
exclude: 7120/9160, uniq_by_line: 5407/7113, autogenerated_exclude: 9160/9160,
identifier_marker: 9160/9160, nolint: 7113/7120, diff: 0/5407,
filename_unadjuster: 9160/9160, skip_files: 9160/9160 

Edit to add: rolling back from v1.23.6 to v.1.23.3 seems to have helped; I'm now seeing issues that were introduced only since the nominated revision come up once again.

[valbuena]

I am having a different problem with the same flag (--new-from-rev); on TravisCI I get

WARN [runner] Can't process result by diff processor: can't prepare diff by revgrep: could not read git repo: error executing git diff "origin/master" "": exit status 128

At first I thought this had something to do with how Travis clones repos (--depth) but that's not the case.

Hi @tbarbugli , Sorry for the approach, did you get to fix it? I have the same problem in Google Cloud Build. Thank you.