unused: don't hide methods of unused types

[dominikh]

Currently, U1000 coalesces unused objects. If an entire struct is unused, we don't flag each individual field. Similarly, if a type is unused, we don't flag each individual method. Maybe we should flag each method. I've witnessed the following interaction:

1. staticcheck warns that a type is unused
2. user 1 adds an ignore directive, stating that the type is in fact being used
3. user 2 points out that the type is indeed unused
4. user 1 disagrees, points to the fact that removing the type breaks compilation (because the type has methods that refer to the type)

User 1 wasn't an inexperienced programmer, and very quickly realized their mistake after the methods got pointed out to them. They simply focused on exactly what staticcheck told them, and nothing else.

For fields, this problem won't occur – they are syntactically nested beneath the type. Removing the type removes the fields. Methods, however, stand on their own, possibly not even in the same files as the types they are defined on.

Related: #241

[dominikh]

An actual example of the change:

Input:

package pkg

type t struct{}

func (t *t) Len() int               { return 0 }
func (t *t) Less(i int, j int) bool { return false }
func (t *t) Swap(i int, j int)      {}

Current output:

prj/src/sandbox/foo/foo.go:3:6: type t is unused (U1000)

Proposed output:

prj/src/sandbox/foo/foo.go:3:6: type t is unused (U1000)
prj/src/sandbox/foo/foo.go:5:13: func (*t).Len is unused (U1000)
prj/src/sandbox/foo/foo.go:6:13: func (*t).Less is unused (U1000)
prj/src/sandbox/foo/foo.go:7:13: func (*t).Swap is unused (U1000)

[mvdan]

This sounds good to me; I can see how this could lead to confusion for some users.

Another advantage is that if you warn for every field/method declaration too, the number of warnings would be a bit closer to the amount of unused code there is. So it would be easier to spot that an unused type with twenty methods really is a lot of useless code.

[dominikh]

A complication I didn't consider: //lint:ignore directives are a post-processing step that filters problems occurring on annotated lines. If we flag unused methods on an unused type, then the user would need to //lint:ignore each individual method.

This brings us back to #675 and the question if unused should be aware of these directives. It might arguably be more intuitive that way. This particular case could not be solved merely by assigning to _ either, as using a type will not automatically use its methods.