alecthomas / gometalinter

DEPRECATED: Use https://github.com/golangci/golangci-lint
MIT License
3.51k stars 267 forks source link

Exit code for warnings. #205

Closed plod closed 7 years ago

plod commented 7 years ago

As far as I understand it unless a linter returns a severity error or is configured to do so :

Severity: map[string]string{
            "gotype":  "error",
            "test":    "error",
            "testify": "error",
            "vet":     "error",
        },

However when there is just warnings the process exits with 1

./gometalinter --errors
echo $?

0

./gometalinter
main.go:27:1:warning: Warning is unused (deadcode)
main.go:723::warning: cyclomatic complexity 18 of function processOutput() is high (> 10) (gocyclo)
main.go:446::warning: cyclomatic complexity 16 of function expandPaths() is high (> 10) (gocyclo)
main.go:87::warning: cyclomatic complexity 14 of function (*sortedIssues).Less() is high (> 10) (gocyclo)
main.go:309::warning: cyclomatic complexity 13 of function processConfig() is high (> 10) (gocyclo)
main.go:515::warning: Subprocess launching with variable.,HIGH,HIGH (gas)
main.go:526::warning: Subprocess launching with variable.,HIGH,HIGH (gas)
main.go:657::warning: Subprocess launching with variable.,HIGH,HIGH (gas)
main.go:24:6:warning: exported type Severity should have comment or be unexported (golint)
main.go:40:6:warning: exported type Linter should have comment or be unexported (golint)
main.go:52:1:warning: exported method Linter.MarshalJSON should have comment or be unexported (golint)
main.go:60:1:warning: exported function LinterFromName should have comment or be unexported (golint)
main.go:193:6:warning: exported type Issue should have comment or be unexported (golint)
main.go:240:6:warning: exported type Vars should have comment or be unexported (golint)
main.go:242:1:warning: exported method Vars.Copy should have comment or be unexported (golint)
main.go:250:1:warning: exported method Vars.Replace should have comment or be unexported (golint)
main.go:163:15:warning: error return value not checked (defer r.Close()) (errcheck)
config.go:11:6:warning: struct Config could have size 240 (currently 264) (aligncheck)

echo $?

1

I was expecting the warnings to cause the program to exit with 0, have I not understood something or is this a bug?

alecthomas commented 7 years ago

Exit status is the first FAQ entry.

alecthomas commented 7 years ago

(also you can fence blocks with ``` to correctly quote them as code)

plod commented 7 years ago

I read FAQ entry 1 but I was expecting warnings not to turn on the first bit (unless the linter had' been configured with severity "error") or am I still misunderstanding?

alecthomas commented 7 years ago

You are misunderstanding: warnings and errors are both treated as "issues". That said, I'd be fine with an extra exit status bit for errors.

plod commented 7 years ago

this is the behaviour I thought I was getting, but any exit code of > 0 is causing CI to fail on other projects, trying to think of a way to solve this, I guess extra status bit for errors we could script for that.

AlekSi commented 7 years ago

The idea is to exit with 0 if only warnings were generated, but still print them. For example, CI build should stop on vet errors, but continue for golint warnings. And we want to achieve this without bash and grepping. In Makefile terms the following is roughly equivalent:

check:
    gometalinter --errors
    - gometalinter

Here - ignores command's exit code.

Since this is likely a breaking change, it can be enabled by (yet another) flag. What do you think?

alecthomas commented 7 years ago

A separate exit status bit seems more generally useful to me, though it too could break backwards compatibility somewhat.

AlekSi commented 7 years ago

But this will require an additional check with shell scripting: non-zero exit status stops make targets, bash command sequences and almost everything else.

How about

      --errors              Ignore linter warnings in exit status and hide them.
      --ignore-warnings     Ignore linter warnings in exit status, but still show them.
alecthomas commented 7 years ago

I do not want to add flags for tweaking minor, niche behaviours. Given that this is the first time anybody has ever wanted this, I don't think it is very common to want to display warnings but not act on them.

alecthomas commented 7 years ago

Feel free to send a PR to implement the extra status bit.

displague commented 6 years ago

I also expected --severity=thing:warn to return 0 and not 1. The README/FAQ doesn't appear to cover what the possible severity states are. (Is it actually warning?)

I would assume that if warn also returns 1, that a --severity=thing:info would exist which would result in a return code of 0.

I don't think it is very common to want to display warnings but not act on them.

You want to introduce gometalinter into your project, but you don't have the time to act on all of the warnings immediately.

A work around would be to run gometalinter with the specific linters disabled, and then run gometalinter again enabling only those specific linters, or'ing the command to || true.