search

influxdata / telegraf

Agent for collecting, processing, aggregating, and writing metrics, logs, and other arbitrary data.
https://influxdata.com/telegraf
MIT License
14.67k stars 5.59k forks source link

New linter: `inamedparam`. Should we enable it? #15504

Closed zak-pawel closed 4 months ago

zak-pawel commented 4 months ago

Description

This issue starts a discussion about enabling:

Example

Before:

type Runner interface {
    Run(string, []string, time.Duration) ([]byte, []byte, error)
}

After:

type Runner interface {
    Run(command string, environments []string, timeout time.Duration,) ([]byte, []byte, error)
}

Expected output

Decision about enabling or not enabling this linter.

Findings

For this linter, the following findings were found in the current codebase:

accumulator.go:42:12                                                inamedparam  interface method AddMetric must have named param for type Metric
cmd/telegraf/main.go:30:26                                          inamedparam  interface method CollectDeprecationInfos must have all named params
cmd/telegraf/main.go:30:36                                          inamedparam  interface method CollectDeprecationInfos must have all named params
cmd/telegraf/main.go:30:46                                          inamedparam  interface method CollectDeprecationInfos must have all named params
cmd/telegraf/main.go:30:56                                          inamedparam  interface method CollectDeprecationInfos must have all named params
cmd/telegraf/main.go:31:23                                          inamedparam  interface method PrintDeprecationList must have all named params
cmd/telegraf/pprof.go:12:8                                          inamedparam  interface method Start must have named param for type string
cmd/telegraf/telegraf.go:64:7                                       inamedparam  interface method Init must have all named params
cmd/telegraf/telegraf.go:64:21                                      inamedparam  interface method Init must have named param for type Filters
cmd/telegraf/telegraf.go:64:30                                      inamedparam  interface method Init must have named param for type GlobalFlags
cmd/telegraf/telegraf.go:64:43                                      inamedparam  interface method Init must have named param for type WindowFlags
cmd/telegraf/telegraf.go:69:17                                      inamedparam  interface method GetSecretStore must have named param for type string
filter/filter.go:10:8                                               inamedparam  interface method Match must have named param for type string
input.go:8:9                                                        inamedparam  interface method Gather must have named param for type Accumulator
input.go:16:8                                                       inamedparam  interface method Start must have named param for type Accumulator
internal/content_coding.go:166:9                                    inamedparam  interface method Encode must have all named params
internal/content_coding.go:338:14                                   inamedparam  interface method SetEncoding must have named param for type string
internal/content_coding.go:339:9                                    inamedparam  interface method Decode must have all named params
internal/snmp/wrapper.go:19:7                                       inamedparam  interface method Walk must have named param for type string
internal/snmp/wrapper.go:19:15                                      inamedparam  interface method Walk must have named param for type gosnmp.WalkFunc
logger.go:39:24                                                     inamedparam  interface method RegisterErrorCallback must have all named params
plugins/common/parallel/parallel.go:6:10                            inamedparam  interface method Enqueue must have named param for type telegraf.Metric
plugins/common/socket/socket.go:24:13                               inamedparam  interface method listenData must have named param for type CallbackData
plugins/common/socket/socket.go:24:27                               inamedparam  interface method listenData must have named param for type CallbackError
plugins/common/socket/socket.go:25:19                               inamedparam  interface method listenConnection must have named param for type CallbackConnection
plugins/common/socket/socket.go:25:39                               inamedparam  interface method listenConnection must have named param for type CallbackError
plugins/inputs/cloudwatch/cloudwatch.go:89:14                       inamedparam  interface method ListMetrics must have named param for type context.Context
plugins/inputs/cloudwatch/cloudwatch.go:89:31                       inamedparam  interface method ListMetrics must have all named params
plugins/inputs/cloudwatch/cloudwatch.go:89:59                       inamedparam  interface method ListMetrics must have all named params
plugins/inputs/cloudwatch/cloudwatch.go:90:16                       inamedparam  interface method GetMetricData must have named param for type context.Context
plugins/inputs/cloudwatch/cloudwatch.go:90:33                       inamedparam  interface method GetMetricData must have all named params
plugins/inputs/cloudwatch/cloudwatch.go:90:63                       inamedparam  interface method GetMetricData must have all named params
plugins/inputs/exec/exec.go:58:6                                    inamedparam  interface method Run must have named param for type string
plugins/inputs/exec/exec.go:58:14                                   inamedparam  interface method Run must have all named params
plugins/inputs/exec/exec.go:58:24                                   inamedparam  interface method Run must have named param for type time.Duration
plugins/inputs/intel_pmu/activators.go:45:16                        inamedparam  interface method activateEvent must have named param for type ia.Activator
plugins/inputs/intel_pmu/activators.go:45:30                        inamedparam  interface method activateEvent must have named param for type ia.PlacementProvider
plugins/inputs/intel_pmu/activators.go:45:52                        inamedparam  interface method activateEvent must have named param for type ia.Options
plugins/inputs/intel_pmu/activators.go:46:16                        inamedparam  interface method activateGroup must have named param for type ia.PlacementProvider
plugins/inputs/intel_pmu/activators.go:46:38                        inamedparam  interface method activateGroup must have all named params
plugins/inputs/intel_pmu/activators.go:47:16                        inamedparam  interface method activateMulti must have named param for type ia.MultiActivator
plugins/inputs/intel_pmu/activators.go:47:35                        inamedparam  interface method activateMulti must have all named params
plugins/inputs/intel_pmu/activators.go:47:59                        inamedparam  interface method activateMulti must have named param for type ia.Options
plugins/inputs/intel_pmu/intel_pmu.go:30:11                         inamedparam  interface method readFile must have named param for type string
plugins/inputs/intel_pmu/intel_pmu.go:31:8                          inamedparam  interface method lstat must have named param for type string
plugins/inputs/intel_pmu/reader.go:51:15                            inamedparam  interface method readEntities must have all named params
plugins/inputs/intel_pmu/reader.go:51:35                            inamedparam  interface method readEntities must have all named params
plugins/inputs/netflow/netflow.go:24:9                              inamedparam  interface method Decode must have named param for type net.IP
plugins/inputs/netflow/netflow.go:24:17                             inamedparam  interface method Decode must have all named params
plugins/inputs/opensearch_query/aggregation.go:9:17                 inamedparam  interface method AddAggregation must have named param for type string
plugins/inputs/opensearch_query/aggregation.go:9:25                 inamedparam  interface method AddAggregation must have named param for type string
plugins/inputs/opensearch_query/aggregation.go:9:33                 inamedparam  interface method AddAggregation must have named param for type string
plugins/inputs/opensearch_query/aggregation.go:13:9                 inamedparam  interface method Nested must have named param for type string
plugins/inputs/opensearch_query/aggregation.go:13:17                inamedparam  interface method Nested must have named param for type AggregationRequest
plugins/inputs/opensearch_query/aggregation.go:14:10                inamedparam  interface method Missing must have named param for type string
plugins/inputs/opensearch_query/aggregation.go:15:7                 inamedparam  interface method Size must have named param for type int
plugins/inputs/procstat/process.go:18:9                             inamedparam  interface method SetTag must have named param for type string
plugins/inputs/procstat/process.go:18:17                            inamedparam  interface method SetTag must have named param for type string
plugins/inputs/procstat/process.go:19:13                            inamedparam  interface method MemoryMaps must have named param for type bool
plugins/inputs/procstat/process.go:20:9                             inamedparam  interface method Metric must have named param for type string
plugins/inputs/procstat/process.go:20:17                            inamedparam  interface method Metric must have all named params
plugins/inputs/zipkin/zipkin.go:47:9                                inamedparam  interface method Record must have named param for type trace.Trace
plugins/inputs/zipkin/zipkin.go:48:8                                inamedparam  interface method Error must have named param for type error
plugins/outputs/application_insights/application_insights.go:23:8   inamedparam  interface method Track must have named param for type appinsights.Telemetry
plugins/outputs/application_insights/application_insights.go:28:12  inamedparam  interface method Subscribe must have named param for type appinsights.DiagnosticsMessageHandler
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:39:3             inamedparam  interface method DescribeLogGroups must have named param for type context.Context
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:40:3             inamedparam  interface method DescribeLogGroups must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:41:3             inamedparam  interface method DescribeLogGroups must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:44:3             inamedparam  interface method DescribeLogStreams must have named param for type context.Context
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:45:3             inamedparam  interface method DescribeLogStreams must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:46:3             inamedparam  interface method DescribeLogStreams must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:49:3             inamedparam  interface method CreateLogStream must have named param for type context.Context
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:50:3             inamedparam  interface method CreateLogStream must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:51:3             inamedparam  interface method CreateLogStream must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:53:15            inamedparam  interface method PutLogEvents must have named param for type context.Context
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:53:32            inamedparam  interface method PutLogEvents must have all named params
plugins/outputs/cloudwatch_logs/cloudwatch_logs.go:53:67            inamedparam  interface method PutLogEvents must have all named params
plugins/outputs/exec/exec.go:97:6                                   inamedparam  interface method Run must have named param for type time.Duration
plugins/outputs/exec/exec.go:97:21                                  inamedparam  interface method Run must have all named params
plugins/outputs/exec/exec.go:97:31                                  inamedparam  interface method Run must have all named params
plugins/outputs/exec/exec.go:97:41                                  inamedparam  interface method Run must have named param for type io.Reader
plugins/outputs/influxdb/influxdb.go:33:8                           inamedparam  interface method Write must have named param for type context.Context
plugins/outputs/influxdb/influxdb.go:33:25                          inamedparam  interface method Write must have all named params
plugins/outputs/influxdb_v2/influxdb_v2.go:33:8                     inamedparam  interface method Write must have named param for type context.Context
plugins/outputs/influxdb_v2/influxdb_v2.go:33:25                    inamedparam  interface method Write must have all named params
plugins/outputs/kinesis/kinesis.go:49:13                            inamedparam  interface method PutRecords must have named param for type context.Context
plugins/outputs/kinesis/kinesis.go:49:30                            inamedparam  interface method PutRecords must have all named params
plugins/outputs/kinesis/kinesis.go:49:56                            inamedparam  interface method PutRecords must have all named params
plugins/outputs/postgresql/datatype_uint8.go:220:22                 inamedparam  interface method Set must have all named params
plugins/outputs/timestream/timestream.go:54:15                      inamedparam  interface method CreateTable must have named param for type context.Context
plugins/outputs/timestream/timestream.go:54:32                      inamedparam  interface method CreateTable must have all named params
plugins/outputs/timestream/timestream.go:54:67                      inamedparam  interface method CreateTable must have all named params
plugins/outputs/timestream/timestream.go:55:16                      inamedparam  interface method WriteRecords must have named param for type context.Context
plugins/outputs/timestream/timestream.go:55:33                      inamedparam  interface method WriteRecords must have all named params
plugins/outputs/timestream/timestream.go:55:69                      inamedparam  interface method WriteRecords must have all named params
plugins/outputs/timestream/timestream.go:57:4                       inamedparam  interface method DescribeDatabase must have named param for type context.Context
plugins/outputs/timestream/timestream.go:58:4                       inamedparam  interface method DescribeDatabase must have all named params
plugins/outputs/timestream/timestream.go:59:4                       inamedparam  interface method DescribeDatabase must have all named params

Additional configuration

For this linter, additional configuration can be provided:

  inamedparam:
    # Skips check for interface methods with only a single parameter.
    # Default: false
    skip-single-param: true
powersj commented 4 months ago

I don't think it is necessary for interfaces to always have named parameters, nor see the value it adds. I'm -1 unless someone can say otherwise.

powersj commented 4 months ago

After discussion we will decline this one as well. We can leave it up to PR reviews to determine the best course of action for each scenario.