nishanths/exhaustive

Support type aliasing

Closed this issue · 5 comments

Hello!

Can you look at example in golangci/golangci-lint#1940 (comment)

Thank u.

Thank you for the example. I will take a look.

I have not been able to take a look. If anyone else would like to, please go ahead. Please comment on the issue before beginning if possible. Thanks.

Hi, sorry for the delay. I tried the nonolint.zip example shared in the related golangci-lint issue. However I wasn't able to reproduce. Based on the output below, the issue likely originally occurred during your tries because of a golangci-lint cache issue. The issue does not occur if golangci-lint cache clean is executed before each golangci-lint run.

If you see the same behavior I've shown below, please create an issue in golangci-lint to fix how the cache works (or if you don't see the same behavior reopen this issue). It appears that the cache isn't getting invalidated aggressively enough when source files have changed.

My output was obtained using:

nonolint % golangci-lint --version
golangci-lint has version 1.40.1 built from 625445b on 2021-05-14T16:27:59Z

installed using brew install golangci-lint.

Output

First, I tested using the original code. The code does not have the //nolint directive. This produces the diagnostic from exhaustive as expected.

nonolint % cat adapt.go | head -n 10 # does not have //nolint directive
package nonolint

import "example/tigers"

func adaptTigerToInt(t tigers.Cat) int {
	switch t {
	case tigers.Tiger1:
		return 1
	}
	return 0
nonolint % golangci-lint cache clean
nonolint % golangci-lint run .
adapt.go:6:2: missing cases in switch of type cats.Cat: Cat1|Cat2 (exhaustive)
	switch t {
	^

Next I added the //nolint directive to the switch statement.

nonolint|1 % cat adapt.go | head -n 10
package nonolint

import "example/tigers"

func adaptTigerToInt(t tigers.Cat) int {
	switch t { //nolint:exhaustive
	case tigers.Tiger1:
		return 1
	}
	return 0
nonolint % golangci-lint cache clean
nonolint % golangci-lint run .
# ... no output ...

This produces no output and a zero exit code.

However, if I omit the golangci-lint cache clean before the second golangci-lint run command, then I see the spurious diagnostic that you saw, i.e.

adapt.go:6:13: directive `//nolint:exhaustive` is unused for linter "exhaustive" (nolintlint)
	switch t { //nolint:exhaustive

So the underlying issue seems to be the incorrect caching invalidation in golangci-lint.

All that said, exhaustive can be improved regarding how it handles type aliases (see issue #13). But I think that is separate from the original reason behind this issue and golangci/golangci-lint#1940.

Based on the findings in #7 (comment), I'm closing this issue. Please feel free to reopen 🙂

@nishanths thank u for feedback