kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.49k stars 1.29k forks source link

Lint unused funcs #7599

Open sbueringer opened 1 year ago

sbueringer commented 1 year ago

It would be great if we had a linter which detects unused funcs.

I recently opened a PR to drop some unused code in KCP: https://github.com/kubernetes-sigs/cluster-api/pull/7598

I found the following cases:

  1. funcs which weren't used by anyone: e.g. ControlPlane.MachineInfrastructureTemplateRef
  2. funcs which were only used test code: e.g. ControlPlane.NewMachine

I think if there is a linter 1. should be relatively easy to detect.

I'm not sure about 2., it could be that there are a lot of false positives as we probably have non-test code which is used as test utils in tests.

But even if we only find a linter for 1. it should be a good improvement

/kind feature

sbueringer commented 1 year ago

/triage accepted /help

k8s-ci-robot commented 1 year ago

@sbueringer: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/7599): >/triage accepted >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
Anddd7 commented 1 year ago

Hi team, i just analyzed the code with goland.inspection, report:

image

I can help fix some warnings if needed (starting from minor changes, e.g. comments, unused variables & parameters)

sbueringer commented 1 year ago

It would be really good to instead research specific linters. Discuss if we want to enable them and then open a PR to add the linter and fix the findings.

Anddd7 commented 1 year ago

It would be really good to instead research specific linters. Discuss if we want to enable them and then open a PR to add the linter and fix the findings.

The ci configured golangci linters already, do you mean to add more linters or modify the settings of existing linters?

sbueringer commented 1 year ago

The Idea was to see if we can enable additional linters/ rules via golangcilint.

Note this might be tricky as I expect we have a lot of exported types/funcs that are only used by external consumers

fabriziopandini commented 4 months ago

/priority backlog