Investigate `unused` linter report with `field-writes-are-uses` setting disabled #1100

New issue

Closed

opened 2024-04-18 08:16:07 +00:00 by fyrchik · 2 comments

fyrchik commented

2024-04-18 08:16:07 +00:00

Owner

unused linter has a field-writes-are-uses setting, which is true by default.
The default differs from structcheck behaviour which catches struct fields that are only written to.

There are some false-positives here (e.g. in cmd/frostfs-node/policy-engine.go where the struct is used as a map key without any field access).

So I suggest:

Fix all the issues we have.
If the number of false-positives is <=1 , disable this setting in linter-settings and add an exception in code.
Otherwise, leave the linter disabled.

Note that not all of the fields need to be removed, we can have some bugs here (e.g. config parameter being unused). The bugs should be fixed in separate commits.

pkg/services/session/storage/temporary/storage.go:12:2              unused  field `tokenID` is unused
pkg/services/session/storage/temporary/storage.go:13:2              unused  field `ownerID` is unused
cmd/frostfs-cli/modules/container/policy_playground.go:23:2         unused  field `args` is unused
pkg/ape/chainbase/option.go:21:2                                    unused  field `log` is unused
cmd/frostfs-cli/internal/client/client.go:615:2                     unused  field `mainOnly` is unused
cmd/frostfs-node/config.go:605:2                                    unused  field `startEpoch` is unused
cmd/frostfs-node/policy_engine.go:28:2                              unused  field `name` is unused
cmd/frostfs-node/policy_engine.go:29:2                              unused  field `target` is unused
pkg/local_object_storage/engine/control.go:254:2                    unused  field `errorsThreshold` is unused
pkg/local_object_storage/engine/control.go:255:2                    unused  field `shardPoolSize` is unused
pkg/services/policer/option.go:69:2                                 unused  field `rebalanceFreq` is unused
pkg/local_object_storage/pilorama/heap.go:32:2                      unused  field `max` is unused
pkg/local_object_storage/blobstor/memstore/option.go:10:2           unused  field `log` is unused
pkg/local_object_storage/blobstor/memstore/option.go:14:2           unused  field `reportError` is unused
pkg/util/locode/table/csv/calls.go:59:2                             unused  field `countryCode` is unused
pkg/util/locode/table/csv/calls.go:60:2                             unused  field `subDivCode` is unused
pkg/services/object/sign.go:13:2                                    unused  field `key` is unused
pkg/services/object/ape/service.go:24:2                             unused  field `log` is unused
pkg/innerring/processors/governance/processor.go:82:3               unused  field `netmapClient` is unused
pkg/local_object_storage/metabase/delete.go:80:2                    unused  field `addr` is unused
pkg/innerring/innerring.go:106:3                                    unused  field `cmode` is unused
cmd/frostfs-adm/internal/modules/morph/helper/local_client.go:47:2  unused  field `maxGasInvoke` is unused

`unused` linter has a `field-writes-are-uses` setting, which is true by default. The default differs from `structcheck` behaviour which catches struct fields that are only written to. There are some false-positives here (e.g. in `cmd/frostfs-node/policy-engine.go` where the struct is used as a map key without any field access). So I suggest: 1. Fix all the issues we have. 2. If the number of false-positives is <=1 , disable this setting in `linter-settings` and add an exception in code. 3. Otherwise, leave the linter disabled. Note that not all of the fields need to be removed, we can have some bugs here (e.g. config parameter being unused). The bugs should be fixed in separate commits. ``` pkg/services/session/storage/temporary/storage.go:12:2 unused field `tokenID` is unused pkg/services/session/storage/temporary/storage.go:13:2 unused field `ownerID` is unused cmd/frostfs-cli/modules/container/policy_playground.go:23:2 unused field `args` is unused pkg/ape/chainbase/option.go:21:2 unused field `log` is unused cmd/frostfs-cli/internal/client/client.go:615:2 unused field `mainOnly` is unused cmd/frostfs-node/config.go:605:2 unused field `startEpoch` is unused cmd/frostfs-node/policy_engine.go:28:2 unused field `name` is unused cmd/frostfs-node/policy_engine.go:29:2 unused field `target` is unused pkg/local_object_storage/engine/control.go:254:2 unused field `errorsThreshold` is unused pkg/local_object_storage/engine/control.go:255:2 unused field `shardPoolSize` is unused pkg/services/policer/option.go:69:2 unused field `rebalanceFreq` is unused pkg/local_object_storage/pilorama/heap.go:32:2 unused field `max` is unused pkg/local_object_storage/blobstor/memstore/option.go:10:2 unused field `log` is unused pkg/local_object_storage/blobstor/memstore/option.go:14:2 unused field `reportError` is unused pkg/util/locode/table/csv/calls.go:59:2 unused field `countryCode` is unused pkg/util/locode/table/csv/calls.go:60:2 unused field `subDivCode` is unused pkg/services/object/sign.go:13:2 unused field `key` is unused pkg/services/object/ape/service.go:24:2 unused field `log` is unused pkg/innerring/processors/governance/processor.go:82:3 unused field `netmapClient` is unused pkg/local_object_storage/metabase/delete.go:80:2 unused field `addr` is unused pkg/innerring/innerring.go:106:3 unused field `cmode` is unused cmd/frostfs-adm/internal/modules/morph/helper/local_client.go:47:2 unused field `maxGasInvoke` is unused ```

fyrchik added the

triage

label 2024-04-18 08:16:07 +00:00

elebedeva was assigned by fyrchik

2024-04-18 08:16:12 +00:00

fyrchik added this to the v0.39.0 milestone 2024-04-18 08:16:17 +00:00

fyrchik removed the

triage

label 2024-04-18 08:16:20 +00:00

fyrchik changed title from ~~Investigate unused linter report with field-writes-are-uses setting disabled.~~ to Investigate unused linter report with field-writes-are-uses setting disabled

2024-04-18 08:23:36 +00:00

fyrchik commented

2024-04-18 08:25:47 +00:00

Author

Owner

There are other problems as well https://github.com/dominikh/go-tools/issues/288, so leaving the setting in default is a good idea.

The false positive from OP is known about https://github.com/dominikh/go-tools/issues/288#issuecomment-1371631522

There are other problems as well https://github.com/dominikh/go-tools/issues/288, so leaving the setting in default is a good idea. The false positive from OP is known about https://github.com/dominikh/go-tools/issues/288#issuecomment-1371631522

fyrchik referenced this issue from TrueCloudLab/frostfs-s3-gw

2024-04-18 08:36:06 +00:00

Investigate unused linter warnings #373

fyrchik added the

internal

label 2024-04-22 07:38:55 +00:00

elebedeva referenced this issue from a pull request that will close it,

2024-04-27 13:01:33 +00:00

Investigate unused linter report with field-writes-are-uses setting disabled #1111

fyrchik closed this issue

2024-05-06 08:28:33 +00:00

fyrchik referenced this issue from a commit