golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.36k stars 1.37k forks source link

Add --staged flag to only check changes in staged code #4328

Open abemedia opened 8 months ago

abemedia commented 8 months ago

Your feature request related to a problem? Please describe.

I use golangci-lint in pre-commit hooks and without adding a custom script to first stash unstaged changes this results in the pre-commit hook failing if there are issues in unstaged code.

Describe the solution you'd like.

A new flag --staged which ignores all issues that are not in staged changes.

Describe alternatives you've considered.

A custom pre-commit script which first stashes the changes e.g.

git stash --keep-index --include-untracked --quiet
exitCode=0
golangci-lint run || exitCode=$?
git stash pop --quiet
exit $exitCode

The problem with this solution is that it can result in the loss of what is currently staged and what isn't.

Additional context.

No response

boring-cyborg[bot] commented 8 months ago

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

titolins commented 7 months ago

hey @abemedia , are there any current proposals on how to tackle that? I'm looking for this exact solution, would be glad to work on PR later if you haven't started yet 👍

abemedia commented 7 months ago

Well there's already logic to just use a patch file. One could implement this by generating a patch file from the currently staged changes.

ldez commented 7 months ago

Hello,

FYI, the current behavior is based on git ls-files --others --exclude-standard.

ls-files doesn't have an option to list only staged files (--stage just displays files with staged contents' object information, but it can be a staged file or just modified existing files) ls-files just shows tracked/untracked files. https://git-scm.com/docs/git-ls-files

Maybe git diff --name-only --cached can be an approach. https://git-scm.com/docs/git-diff

titolins commented 7 months ago

ah, I wasn't aware of the new-from-patch flag. That's easier than I thought then 🙌 yeah, I'd suggest using the git diff ... approach also. I'll see if I can work on that later today/tomorrow then 👍 Thanks!

ldez commented 7 months ago

My comment was not really a suggestion but more a technical thinking.

I'm not sure of the value of an option, only for a niche, that can be done easily with the command line. This is why I flagged this issue with no-decision.

git diff --cached > /tmp/stage.patch; golangci-lint run --new-from-patch=/tmp/stage.patch; rm /tmp/stage.patch
git diff --cached > /tmp/stage.patch
golangci-lint run --new-from-patch=/tmp/stage.patch
rm /tmp/stage.patch

Maybe this can be a new hook inside our pre-commit hook file. :thinking:

Also, note that running golangci-lint only on a few files can lead to unexpected behavior with linters (the majority) that require scanning all files.

titolins commented 7 months ago

I'm not sure of the value of an option, only for a niche, that can be done easily with the command line.

Yeah, to be honest I've had the same thought. After reading your comments, I re-wrote my pre commit hook to do pretty much the same as you've suggested above.

I'm just sad since this was an easy first contribution 😅

Also, note that running golangci-lint only on a few files can lead to unexpected behavior with linters (the majority) that require scanning all files.

I wasn't aware of this though - thanks for raising it 👍

NBS-LLC commented 3 months ago

I think a --staged flag would still be useful. For example, in a monorepo where I have a number of go services that are sub-projects. I'd like to use golangci-lint in a husky pre-commit hook that runs against each sub-project only if there are staged changes.

Ideally I should be able to use this in conjunction with --whole-files.