Skip to content

unused: remove exported-is-used option #4890

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 31, 2024
Merged

unused: remove exported-is-used option #4890

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e923c3a
fix: remove exported-is-used option (unused)
ldez Jul 31, 2024
234cf8d
docs: add reference to the root issue
ldez Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions jsonschema/golangci.next.jsonschema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3233,11 +3233,6 @@
"type": "boolean",
"default": false
},
"exported-is-used": {
"description": "",
"type": "boolean",
"default": true
},
"exported-fields-are-used": {
"description": "",
"type": "boolean",
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/linters_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ type UnparamSettings struct {
type UnusedSettings struct {
FieldWritesAreUses bool `mapstructure:"field-writes-are-uses"`
PostStatementsAreReads bool `mapstructure:"post-statements-are-reads"`
ExportedIsUsed bool `mapstructure:"exported-is-used"`
ExportedIsUsed bool `mapstructure:"exported-is-used"` // Deprecated
ExportedFieldsAreUsed bool `mapstructure:"exported-fields-are-used"`
ParametersAreUsed bool `mapstructure:"parameters-are-used"`
LocalVariablesAreUsed bool `mapstructure:"local-variables-are-used"`
Expand Down
5 changes: 5 additions & 0 deletions pkg/config/loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,11 @@ func (l *Loader) handleLinterOptionDeprecations() {
l.log.Warnf("The configuration option `linters.stylecheck.go` is deprecated, please use global `run.go`.")
}

// Deprecated since v1.60.0
if !l.cfg.LintersSettings.Unused.ExportedIsUsed {
l.log.Warnf("The configuration option `linters.unused.exported-is-used` is deprecated.")
}

// Deprecated since v1.58.0
if l.cfg.LintersSettings.SlogLint.ContextOnly {
l.log.Warnf("The configuration option `linters.sloglint.context-only` is deprecated, please use `linters.sloglint.context`.")
Expand Down
2 changes: 1 addition & 1 deletion pkg/golinters/unused/unused.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

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.

Copy link
Member Author

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.

ExportedFieldsAreUsed: settings.ExportedFieldsAreUsed,
ParametersAreUsed: settings.ParametersAreUsed,
LocalVariablesAreUsed: settings.LocalVariablesAreUsed,
Expand Down
Loading