-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
unused: remove exported-is-used option #4890
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
pkg/golinters/unused/unused.go
Outdated
@@ -90,7 +90,7 @@ func getUnusedResults(pass *analysis.Pass, settings *config.UnusedSettings) unus | |||
opts := unused.Options{ | |||
FieldWritesAreUses: settings.FieldWritesAreUses, | |||
PostStatementsAreReads: settings.PostStatementsAreReads, | |||
ExportedIsUsed: settings.ExportedIsUsed, | |||
ExportedIsUsed: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you said befor that you want golangci-lint
to have its own default but since this stems from a bug where we're deviating from upstream, are you considering using the value from the DefaultConfig in unused
?
I know it probably doesn't make any difference now but it serves some kind of documentation purpose and will follow any changes made upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this context, we are forced to override the value to be sure that this value is always true
.
I will not use unused.DefaultOptions.ExportedIsUsed
because it can be mutated by unused
itself (because it's a global variable), or the default value can be changed by unused
(this is a problem for us).
I added a comment to reference the issue.
Fixes #4218