After removing some codes in the legacy file, new-from-rev is not correctly working

[wangmir]

Welcome

• 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 read the typecheck section of the FAQ (https://golangci-lint.run/usage/faq/#why-do-you-have-typecheck-errors).
• Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Description of the problem

I'm using new-from-rev to avoid lint from legacy code, and it worked well previously.

But, when I delete some of the codes in the middle of the legacy file, and it causes the old code to be included in the lint.

I did similar behavior before, and it was fine for that situation, but for this case, all the codes in the legacy file under the deleted code are included into lint.

It's a bit hard to reproduce with minimal example because I tried, but all cases was fine. So, I think it seems extremely corner case. But, when I check the git diff, it is not a problem of git diff, it detects only changed part.

Are there any known issue for this?

I'll try to find a way with some of the public project which has a lot of long code, but for now, in order to check is known issue or, are there any solution. So, I raised issue first.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.57.2 built with go1.22.1 from 77a8601a on 2024-03-28T19:01:11Z

I tested with 1.55.2 too.

Configuration

issues:
  exclude:
    - SA1019
  new-from-rev: 6ce5c895

Go environment

$ go version && go env
go version go1.20.4 darwin/arm64
GO111MODULE=""
GOARCH="arm64"
GOBIN="/Users/wangmir/go/bin"
GOCACHE="/Users/wangmir/Library/Caches/go-build"
GOENV="/Users/wangmir/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/wangmir/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/wangmir/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.4"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pl/hz15bk5d0cg7dnfqc59gw0k40000gn/T/go-build1595526823=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
# paste output here

A minimal reproducible example or link to a public repository

Actually, it's hard to reproducable, I tried to reproduce with some codes, but I failed. In normal case, deletion of the code in the old code will not affect to the new-from-rev.

But, only happening with large complex code case.

I'll further try to find a way to reproduce this with public large project.

Validation

• Yes, I've included all information above (version, config, etc.).

[boring-cyborg]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[ldez]

Hello,

without a reproducible context, it's not possible to diagnose the issue.

new-from-rev is based on git calls, maybe this is related to the size of your diff.
But without something to reproduce the context, I cannot help.

[wangmir]

@ldez So, you mean, there are size limitation to detect the diff?

Yes, I'm trying to find a way, but from the multiple test with minimal code, it was fine.. If it's related to the size of diff, then it will be much harder to detect actually.

[wangmir]

@ldez I realized that, git diff in the revgrep needs to support histogram option.

https://stackoverflow.com/questions/40949745/git-diff-incorrectly-interprets-my-changes

In case of default option git diff, because of the greedy algorithm limitation, when I check the git diff from the commit a long time ago, the diff is mixed with multiple functions. But, in case of histogram option, it shows proper git diff.

When I add histogram option in the revgrep's gitDiff function, it works well.

func gitDiff(extraArgs ...string) *exec.Cmd {
	cmd := exec.Command("git", "diff", "--histogram", "--color=never", "--no-ext-diff")

	if isSupportedByGit(2, 41, 0) {
		cmd.Args = append(cmd.Args, "--default-prefix")
	}

	cmd.Args = append(cmd.Args, "--relative")
	cmd.Args = append(cmd.Args, extraArgs...)

	return cmd
}

Check histogram option from git: https://git-scm.com/docs/diff-options/2.6.7

I think it can resolve problem on #4907 too.

[wangmir]

If it's ok, then I'll raise a PR to revgrep repo.

[ldez]

I think the "verbose" option --diff-algorithm=histogram can be used.

I think it can resolve problem on #4907 too.

this issue doesn't exist, can you provide the right issue number?

[wangmir]

I think the "verbose" option --diff-algorithm=histogram can be used.

@ldez Can u explain more on this? what do u mean about "verbose" option? From the golangci-lint side? How can we used this from the config file then?

I think it can resolve problem on #4907 too.

this issue doesn't exist, can you provide the right issue number?

#4097 this one, sorry I put wrong number.

[ldez]

You are suggesting --histogram, it's an alias for --diff-algorithm=histogram.
--diff-algorithm=histogram is a more "verbose"/explicit option.

[wangmir]

@ldez oh, ok. so you just want to use --diff-algorithm=histogram but still agree on the fix on the revgrep side. right?

[ldez]

yes

[ldez]

Do you still plan to create a PR?

[ldez]

Seems related to #4349