Configure linter for unused methods

[ahmetb]

Every now and then, we do major refactoring that leaves unused methods behind.

It would be great to have a linter that can notify us about unused methods,
variables, constants. I suspect this is probably easier to configure with "unexported" methods,
variables etc. However, if there's a way to do this with "exported" methods,
variables etc, we should do it because now anything in this repository (sigs.k8s.io/krew/...)
is expected to be used in this repository. So if something is not referenced,
ideally it should be cleaned up.

/label good-first-issue
/priority P3
/kind cleanup

[corneliusweig]

Hmm.. I just tried configuring the unused linter for golangci-lint with

diff --git a/.golangci.yml b/.golangci.yml
index ff0bbe9..3a53d8c 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -5,6 +5,9 @@ linters-settings:
     # it's a comma-separated list of prefixes
     local-prefixes: sigs.k8s.io/krew
 
+  unused:
+    check-exported: false
+
 # options for analysis running
 run:
   # which dirs to skip: they won't be analyzed;

That setting reports a lot of false positives, which makes it impossible to use in the CI. Btw, the unused linter is active by default. However, it does not consider functions as unused, if they are used in tests. So if all tests are deleted, the unused function from #290 is correctly reported. So it seems that our current meta-linter cannot handle this as desired.

[ahmetb]

So if all tests are deleted,

haha that's a bummer. :D They should totally have a flag to ignore usage in tests.

[ferhatelmas]

For a quick win, how about enabling deadcode and varcheck with tests flag?

[ahmetb]

Please try out. I'm not sure if/how much it helps.

[ferhatelmas]

Not very useful at the moment but there might be beneficial cases.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[ahmetb]

/close

[k8s-ci-robot]

@ahmetb: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ferhatelmas]

#291 would be useful because packages under internal can only be used in the same repo. Otherwise, they are exported, linter won't complain to prevent false positive.