-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
tools: Add a linter to check for struct fields that aren't used anywhere in the project #1408
Comments
handled in #1417 |
This hasn't been handled, #1417 didn't include anything which checks that every field in a struct is used somewhere. |
we really need this 100% - would have already saved time, probably other bugs are currently hiding just waiting to be found by this kind of a tool - changed to prelaunch |
Fixing this probably means forking StructCheck (I haven't found another linter that does this), not sure if thats worth the prelaunch time investment? |
@ValarDragon do we need to fork in order to handle embedded structures? |
Not sure what you mean by embedded structures, I believe it does handle that, it just searches for usage per package, not per repository. |
The readme states thats the only thing it doesn't handle. I guess my real question is, why do we need to fork this? |
looking at the codebase it's pretty clean - could probably fork and add repo wide search easily. https://github.com/opennota/check/blob/master/cmd/structcheck/structcheck.go but also to @alexanderbez 's point though structcheck does not handle embedded structs: |
Why don't we add a new tag for "last things to add during prelaunch phase", I'm cool with this being done then, if we have time. |
can we consolidate with #1417? |
We should probably keep the more detailed issues. I think 1417 should get closed by now, see me comment there. This issue is still very much a problem until we integrate in a linter to solve it |
closed via #4606 |
Struct check does this, but at the module level. We may need to fork this to get it to do what we want, if there is not already another tool for this.
The text was updated successfully, but these errors were encountered: