Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect struct fields that should be local variables #1485

Open
Jacalz opened this issue Dec 22, 2023 · 0 comments
Open

Detect struct fields that should be local variables #1485

Jacalz opened this issue Dec 22, 2023 · 0 comments
Labels
enhancement needs-decision We have to decide if this check is feasible and desirable

Comments

@Jacalz
Copy link

Jacalz commented Dec 22, 2023

I have seen quite a few times that struct fields that previously were used in multiple places end up being used like local variables in methods after some kind of refactor. It is even easier to run into this problem when you have a large code base (or a struct with many fields or many methods) so you can't see all places that the variable is used in.

Let's say in the code example below that we used to have to keep track of our position inside the struct but we clean up the code and grab out position from somewhere else (think like position of window from OS) so we no longer need to keep it inside the struct. Our old pos() method (not changed since the other code was modified) still set the x and y fields even though they are never accessed anywhere else. It seems wrong to me.

I believe that staticcheck ought to flag this kind of problem. The x and y fields should be removed. Introducing this kind of check is not something that personally don't see introducing any more false positives than the already existing check for unused code.

type A struct {
	x int
	y int
}

func (a *A) pos() (int, int) {
	a.x = 20
	a.y = a.x * 10

	return a.x, a.y
}

func main() {
	a := A{}
	fmt.Println(a.pos())
}
@Jacalz Jacalz added the needs-triage Newly filed issue that needs triage label Dec 22, 2023
@dominikh dominikh added enhancement needs-decision We have to decide if this check is feasible and desirable and removed needs-triage Newly filed issue that needs triage labels Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-decision We have to decide if this check is feasible and desirable
Projects
None yet
Development

No branches or pull requests

2 participants