Linter: revive, Rule: `flag-parameter` - Warns on boolean parameters that create a control coupling. Should we enable it?

zak-pawel commented 2 weeks ago

Description

This issue starts a discussion about enabling:

linter: revive - Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. Revive provides a framework for development of custom rules, and lets you define a strict preset for enhancing your development & code review processes.
rule: flag-parameter - If a function controls the flow of another by passing it information on what to do, both functions are said to be control-coupled. Coupling among functions must be minimized for better maintainability of the code. This rule warns on boolean parameters that create a control coupling.

Example configuration:

      - name: flag-parameter
        exclude: ["TEST"]

Expected output

Decision about enabling or not enabling this rule.

Findings

For this rule (with above configuration), the following findings were found in the current codebase:

agent/agent.go:749:18                                                revive  flag-parameter: parameter 'roundInterval' seems to be a control flag, avoid control coupling
cmd/telegraf/printer.go:356:17                                       revive  flag-parameter: parameter 'commented' seems to be a control flag, avoid control coupling
cmd/telegraf/telegraf.go:336:28                                      revive  flag-parameter: parameter 'reloadConfig' seems to be a control flag, avoid control coupling
config/deprecation.go:91:40                                          revive  flag-parameter: parameter 'all' seems to be a control flag, avoid control coupling
config/envvar.go:222:27                                              revive  flag-parameter: parameter 'oldReplacementBehavior' seems to be a control flag, avoid control coupling
internal/snmp/table.go:120:21                                        revive  flag-parameter: parameter 'walk' seems to be a control flag, avoid control coupling
internal/snmp/translator_gosmi.go:56:41                              revive  flag-parameter: parameter 'full' seems to be a control flag, avoid control coupling
logger/handler.go:51:29                                              revive  flag-parameter: parameter 'skipEarlyLogs' seems to be a control flag, avoid control coupling
plugins/common/socket/splitters.go:94:14                             revive  flag-parameter: parameter 'atEOF' seems to be a control flag, avoid control coupling
plugins/common/socket/splitters.go:94:14                             revive  flag-parameter: parameter 'atEOF' seems to be a control flag, avoid control coupling
plugins/inputs/ethtool/ethtool_linux.go:276:36                       revive  flag-parameter: parameter 'includeNamespaces' seems to be a control flag, avoid control coupling
plugins/inputs/gnmi/path.go:250:29                                   revive  flag-parameter: parameter 'withNamespace' seems to be a control flag, avoid control coupling
plugins/inputs/gnmi/path.go:250:29                                   revive  flag-parameter: parameter 'withNamespace' seems to be a control flag, avoid control coupling
plugins/inputs/gnmi/path.go:370:25                                   revive  flag-parameter: parameter 'pathPrefix' seems to be a control flag, avoid control coupling
plugins/inputs/interrupts/interrupts.go:124:19                       revive  flag-parameter: parameter 'cpusAsTags' seems to be a control flag, avoid control coupling
plugins/inputs/ipset/ipset.go:107:13                                 revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/inputs/ldap/openldap.go:66:27                                revive  flag-parameter: parameter 'reverse' seems to be a control flag, avoid control coupling
plugins/inputs/libvirt/libvirt_utils.go:65:38                        revive  flag-parameter: parameter 'shouldGetCurrentPCPU' seems to be a control flag, avoid control coupling
plugins/inputs/logparser/logparser.go:153:39                         revive  flag-parameter: parameter 'fromBeginning' seems to be a control flag, avoid control coupling
plugins/inputs/memcached/memcached.go:107:33                         revive  flag-parameter: parameter 'unix' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/configuration_register.go:134:55               revive  flag-parameter: parameter 'typed' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/configuration_request.go:302:57                revive  flag-parameter: parameter 'typed' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/configuration_request.go:302:57                revive  flag-parameter: parameter 'typed' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/configuration_request.go:302:57                revive  flag-parameter: parameter 'typed' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/type_conversions8.go:7:22                      revive  flag-parameter: parameter 'low' seems to be a control flag, avoid control coupling
plugins/inputs/modbus/type_conversions8.go:7:22                      revive  flag-parameter: parameter 'low' seems to be a control flag, avoid control coupling
plugins/inputs/mongodb/mongodb_server.go:273:28                      revive  flag-parameter: parameter 'gatherTopStat' seems to be a control flag, avoid control coupling
plugins/inputs/mongodb/mongodb_server.go:273:28                      revive  flag-parameter: parameter 'gatherColStats' seems to be a control flag, avoid control coupling
plugins/inputs/mongodb/mongodb_server.go:273:28                      revive  flag-parameter: parameter 'gatherDbStats' seems to be a control flag, avoid control coupling
plugins/inputs/mongodb/mongodb_server.go:273:28                      revive  flag-parameter: parameter 'gatherClusterStatus' seems to be a control flag, avoid control coupling
plugins/inputs/mongodb/mongostat.go:940:17                           revive  flag-parameter: parameter 'all' seems to be a control flag, avoid control coupling
plugins/inputs/nsd/nsd.go:41:15                                      revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/inputs/openntpd/openntpd.go:58:20                            revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/inputs/opensmtpd/opensmtpd.go:39:21                          revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/inputs/processes/processes_notwindows.go:215:12              revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/inputs/rabbitmq/rabbitmq.go:266:15                           revive  flag-parameter: parameter 'b' seems to be a control flag, avoid control coupling
plugins/inputs/smart/smart.go:511:28                                 revive  flag-parameter: parameter 'ignoreExcludes' seems to be a control flag, avoid control coupling
plugins/inputs/snmp/snmp.go:133:27                                   revive  flag-parameter: parameter 'walk' seems to be a control flag, avoid control coupling
plugins/inputs/system/ps.go:48:28                                    revive  flag-parameter: parameter 'perCPU' seems to be a control flag, avoid control coupling
plugins/inputs/system/ps.go:48:28                                    revive  flag-parameter: parameter 'totalCPU' seems to be a control flag, avoid control coupling
plugins/inputs/tail/tail.go:169:28                                   revive  flag-parameter: parameter 'fromBeginning' seems to be a control flag, avoid control coupling
plugins/inputs/varnish/varnish.go:81:19                              revive  flag-parameter: parameter 'useSudo' seems to be a control flag, avoid control coupling
plugins/outputs/application_insights/application_insights.go:121:58  revive  flag-parameter: parameter 'useFieldNameInTelemetryName' seems to be a control flag, avoid control coupling
plugins/outputs/cloudwatch/cloudwatch.go:254:22                      revive  flag-parameter: parameter 'buildStatistic' seems to be a control flag, avoid control coupling
plugins/outputs/cloudwatch/cloudwatch.go:254:22                      revive  flag-parameter: parameter 'highResolutionMetrics' seems to be a control flag, avoid control coupling
plugins/outputs/graylog/graylog_test_linux.go:138:15                 revive  flag-parameter: parameter 'namefieldnoprefix' seems to be a control flag, avoid control coupling
plugins/outputs/influxdb/udp.go:133:15                               revive  flag-parameter: parameter 'atEOF' seems to be a control flag, avoid control coupling
plugins/outputs/opentsdb/opentsdb_http.go:50:28                      revive  flag-parameter: parameter 'debug' seems to be a control flag, avoid control coupling
plugins/outputs/timestream/timestream.go:266:39                      revive  flag-parameter: parameter 'resourceNotFoundRetry' seems to be a control flag, avoid control coupling
plugins/parsers/influx/influx_upstream/parser.go:273:16              revive  flag-parameter: parameter 'allowPartial' seems to be a control flag, avoid control coupling
plugins/parsers/influx/influx_upstream/parser.go:273:16              revive  flag-parameter: parameter 'allowPartial' seems to be a control flag, avoid control coupling
plugins/parsers/influx/influx_upstream/parser.go:273:16              revive  flag-parameter: parameter 'allowPartial' seems to be a control flag, avoid control coupling
plugins/parsers/json/json_flattener.go:24:40                         revive  flag-parameter: parameter 'convertBool' seems to be a control flag, avoid control coupling
plugins/parsers/json/json_flattener.go:24:40                         revive  flag-parameter: parameter 'convertString' seems to be a control flag, avoid control coupling
plugins/parsers/xpath/cbor_document.go:65:35                         revive  flag-parameter: parameter 'withParent' seems to be a control flag, avoid control coupling
plugins/parsers/xpath/json_document.go:65:35                         revive  flag-parameter: parameter 'withParent' seems to be a control flag, avoid control coupling
plugins/parsers/xpath/parser.go:607:36                               revive  flag-parameter: parameter 'expand' seems to be a control flag, avoid control coupling
plugins/parsers/xpath/protocolbuffer_document.go:173:39              revive  flag-parameter: parameter 'withParent' seems to be a control flag, avoid control coupling
plugins/processors/topk/topk.go:65:17                                revive  flag-parameter: parameter 'reverse' seems to be a control flag, avoid control coupling
plugins/serializers/wavefront/replacers.go:26:14                     revive  flag-parameter: parameter 'strict' seems to be a control flag, avoid control coupling

zak-pawel commented 2 weeks ago

This rule had been enabled in the past but we disabled it here and removed completely here.

srebhan commented 2 weeks ago

Yeah I vaguely remember this as "fixing" this lead to a lot of duplication of code where we currently have

func doSomething(allowXYZ bool) {
   ...
   if allowXYZ {
      doSomethingElse()
      setSomeVar()
  }
  ...
}

which then would need to be split into two functions... I don't see much value in this, do you?

zak-pawel commented 2 weeks ago

I don't see much value in this, do you?

@srebhan I also don't see much value in enabling this.

srebhan commented 2 weeks ago

So -1 from my side then. ;-)

influxdata / telegraf