--new-from-rev gets confused by merge commits

[mering]

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

Adding a reproduction example to demonstrate breakage of #4047.

Version of golangci-lint

$ ./golangci-lint --version
# 1.54.2

Configuration

No configuration file

--new-from-rev <sha of 1 commit before HEAD>

Go environment

Using binary

Verbose output of running

Running with --new-from-rev HEAD~1 doesn't show anything even though an unused variable was introduced in the last commit when an error already existed before a merge commit. When adding changes in unrelated files it shows all problems even though some of them have been introduced before.

A minimal reproducible example or link to a public repository

#!/bin/sh
set -x

REPO="rev-binary"
git init "$REPO" && pushd $_
echo golangci-lint > .gitignore
GOLANGCI_LINT_VERSION="1.54.2"
curl -Lf https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/golangci-lint-${GOLANGCI_LINT_VERSION}-linux-amd64.tar.gz | tar xz -C . --strip-components=1 && chmod +x ./golangci-lint
git add .
git commit -m "chore: initial empty commit."

go mod init github.com/gcl/example 
git add .
git commit -m "chore: setup project"

cat <<EOF > foo.go
package pkg

func foo() {
    unusedVariableInFoo := 0
}
EOF
git add .
git commit -m "introduced unused var"

git diff HEAD~1
./golangci-lint run
./golangci-lint run --new-from-rev HEAD~1

git checkout -b b1
git commit -m "empty b1" --allow-empty
git switch -
git merge b1 --no-ff --no-edit

cat <<EOF >> foo.go

func foo2() {
    unusedVariableInFoo2 := 0
}
EOF
git add .
git commit -m "another prob"

git diff HEAD~1
./golangci-lint run
# NOTE the following command doesn't print anything although it should print the problem within bar()
./golangci-lint run --new-from-rev HEAD~1

cat <<EOF > bar.go
package pkg

func bar() {
    unusedVariableInBar := 0
}
EOF
git add .
git commit -m "another prob"

git diff HEAD~1
./golangci-lint run
# NOTE the following command also prints the problems in foo.go although this file didn't change in the last commit
./golangci-lint run --new-from-rev HEAD~1

echo "Cleanup"
popd
rm -rf "$REPO"

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,

[x] Yes, I'm using a binary release within 2 latest major releases.

Have you tried with previous versions?

is there a difference?

[mering]

Which version do you want me to try? I am not aware of any previous working version. I tried 1.50.0 for example with the same result.
You can easily change the version string in the reproduction script and just run it yourself.

[udaya2899]

Hi @ldez, friendly reminder. Can you help us with this please?

[ldez]

This is how git works, so I have no magic solution.

[mering]

This is how git works, so I have no magic solution.

Can you elaborate on which part of Git workings you are referring to? To me the git diff looks as expected but golangci-lint doesn't interpret the diff correctly.

[faximan]

@ldez ping!

[janwytze]

I think I'm currently running in the same problem when using golangci-lint in Gitlab merge result pipielines. This is my CI stage:

lint:
  image: golangci/golangci-lint:latest-alpine
  script:
    - git fetch origin $CI_MERGE_REQUEST_TARGET_BRANCH_NAME && git branch $CI_MERGE_REQUEST_TARGET_BRANCH_NAME -f remotes/origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME
    - git diff HEAD $CI_MERGE_REQUEST_TARGET_BRANCH_NAME # Works, and gives back files and lines that do contain linting errors.
    - golangci-lint run --new-from-rev $CI_MERGE_REQUEST_TARGET_BRANCH_NAME # No result, this same command works locally.

Locally it works fine, but within merge result pipelines it does not work :(

[mering]

@janwytze we discovered this in the GitHub CI which by default also creates a merge commit against the base branch. So golangci-lint is pretty much unusable for GitHub CI at the moment as well.

[janwytze]

@mering Hmm, but git diff does detect the difference, so how can it than be unusable?

golangci-lint should check the difference between the current HEAD, and the provided rev. In my case that is the target branch, which is set to the origin.

I've also added these commands:

- go install github.com/golangci/revgrep/cmd/revgrep@latest
- revgrep -d $CI_MERGE_REQUEST_TARGET_BRANCH_NAME HEAD

golangci-lint internally uses revgrep. This works well, also in the Gitlab CI merge result pipeline.

So both git diff and revgrep do detect the correct diff, also in the merge result pipeline. I just don't understand how golangci-lint run --new-from-rev never sees the changes.