golangci-lint icon indicating copy to clipboard operation
golangci-lint copied to clipboard

`--new-from-rev` should be fatal if the rev can't be located

Open andrewbaxter opened this issue 3 years ago • 0 comments

Welcome

  • [X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [X] Yes, I've searched similar issues on GitHub and didn't find any.
  • [X] Yes, I've included all information below (version, config, etc).
  • [X] Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

If a user specifies --new-from-rev but there's some issue generating the diff, this is logged then continues as normal but showing all lint errors.

This means that

  1. The program doesn't do what the user wants
  2. The actual error is buried in a flood of lint messages from stuff that didn't change (only people with lots of unsolved linter warnings would use this issue)

I believe this should be a fatal error, or otherwise at a minimum this behavior should be documented by the --new-from-rev option (and perhaps other options it affects).

Version of golangci-lint

1.50.0

Configuration file

x

Go environment

x

Verbose output of running

$ golangci-lint run --new-from-rev=123     
WARN [runner] Can't process result by diff processor: can't prepare diff by revgrep: could not read git repo: error executing "git diff --color=never --no-ext-diff --default-prefix --relative 123 --": exit status 128: fatal: bad revision '123' 
main.go:4:5: Error return value is not checked (errcheck)
        foo()
           ^

Code example or link to a public repository

https://github.com/golangci/golangci-lint/blob/v1.50.0/pkg/lint/runner.go#L228 appears to be where the non-fatal logging happens, however this behavior applies to all processors and I'm not sure if an early exit is appropriate for all of them.

andrewbaxter avatar Oct 24 '22 10:10 andrewbaxter