Synchronize lint rules across repositories (#60)

* Disable low value linters.

See related discussion in elastic/beats#31683

* Fix new lint errors.

Author: cmacknz

.golangci.yml

@@ -19,34 +19,21 @@ issues:
 output:
   sort-results: true
 
-# Uncomment and add a path if needed to exclude
-# skip-dirs:
-#   - some/path
-# skip-files:
-#   - ".*\\.my\\.go$"
-#   - lib/bad.go
-
 # Find the whole list here https://golangci-lint.run/usage/linters/
 linters:
   disable-all: true
   enable:
-    - deadcode # finds unused code
     - errcheck # checking for unchecked errors in go programs
     - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
-    - goconst # finds repeated strings that could be replaced by a constant
-    - dupl # tool for code clone detection
     - forbidigo # forbids identifiers	matched by reg exps
     - gomoddirectives # manage the use of 'replace', 'retract', and 'excludes' directives in go.mod.
     - gosimple # linter for Go source code that specializes in simplifying a code
     - misspell # finds commonly misspelled English words in comments
     - nakedret # finds naked returns in functions greater than a specified function length
-    - prealloc # finds slice declarations that could potentially be preallocated
     - nolintlint # reports ill-formed or insufficient nolint directives
     - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks
     - stylecheck # a replacement for golint
-    - unparam # reports unused function parameters
     - unused # checks Go code for unused constants, variables, functions and types
-
     - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
     - ineffassign # detects when assignments to existing variables are not used
     - structcheck # finds unused struct fields
@@ -63,15 +50,20 @@ linters:
     - noctx # noctx finds sending http request without context.Context
     - unconvert # Remove unnecessary type conversions
     - wastedassign # wastedassign finds wasted assignment statements.
-    - godox # tool for detection of FIXME, TODO and other comment keywords
     - gomodguard # check for blocked dependencies
 
 # all available settings of specific linters
 linters-settings:
   errcheck:
     # report about not checking of errors in type assertions: `a := b.(MyStruct)`;
-    # default is false: such cases aren't reported by default.
     check-type-assertions: true
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`.
+    check-blank: false
+    # List of functions to exclude from checking, where each entry is a single function to exclude.
+    # See https://github.com/kisielk/errcheck#excluding-functions for details.
+    exclude-functions:
+      - (mapstr.M).Delete # Only returns ErrKeyNotFound, can safely be ignored.
+      - (mapstr.M).Put # Can only fail on type conversions, usually safe to ignore.
 
   errorlint:
     # Check whether fmt.Errorf uses the %w verb for formatting errors. See the readme for caveats
@@ -81,16 +73,6 @@ linters-settings:
     # Check for plain error comparisons
     comparison: true
 
-  goconst:
-    # minimal length of string constant, 3 by default
-    min-len: 3
-    # minimal occurrences count to trigger, 3 by default
-    min-occurrences: 2
-
-  dupl:
-    # tokens count to trigger issue, 150 by default
-    threshold: 100
-
   forbidigo:
     # Forbid the following identifiers
     forbid:
@@ -106,30 +88,15 @@ linters-settings:
     # Select the Go version to target. The default is '1.13'.
     go: "1.17.11"
 
-  misspell:
-    # Correct spellings using locale preferences for US or UK.
-    # Default is to use a neutral variety of English.
-    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
-    # locale: US
-    # ignore-words:
-    #   - IdP
-
   nakedret:
     # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
     max-func-lines: 0
 
-  prealloc:
-    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
-    # True by default.
-    simple: true
-    range-loops: true # Report preallocation suggestions on range loops, true by default
-    for-loops: false # Report preallocation suggestions on for loops, false by default
-
   nolintlint:
     # Enable to ensure that nolint directives are all used. Default is true.
     allow-unused: false
     # Disable to ensure that nolint directives don't have a leading space. Default is true.
-    allow-leading-space: true
+    allow-leading-space: false
     # Exclude following linters from requiring an explanation.  Default is [].
     allow-no-explanation: []
     # Enable to require an explanation of nonzero length after each nolint directive. Default is false.
@@ -147,7 +114,7 @@ linters-settings:
             recommendations:
               - errors
               - fmt
-            reason: "This package is deprecated"
+            reason: "This package is deprecated, use fmt.Errorf with %%w instead"
         - github.com/elastic/beats/v7:
             reason: "There must be no Beats dependency"
 
@@ -163,13 +130,14 @@ linters-settings:
     # https://staticcheck.io/docs/options#checks
     checks: ["all"]
 
-  unparam:
-    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
-    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-
   unused:
     # Select the Go version to target. The default is '1.13'.
     go: "1.17.11"
+
+  gosec:
+    excludes:
+    - G306 # Expect WriteFile permissions to be 0600 or less
+    - G404 # Use of weak random number generator
+    - G401 # Detect the usage of DES, RC4, MD5 or SHA1: Used in non-crypto contexts.
+    - G501 # Import blocklist: crypto/md5: Used in non-crypto contexts.
+    - G505 # Import blocklist: crypto/sha1: Used in non-crypto contexts.

dev-tools/mage/linter.go

@@ -124,7 +124,7 @@ func (Linter) Install() error {
 	defer installScript.Close()
 
 	log.Println("Downloading the linter installation script...")
-	// nolint: noctx // valid use since there is no context
+	//nolint:noctx // valid use since there is no context
 	resp, err := http.Get(linterInstallURL)
 	if err != nil {
 		return fmt.Errorf("cannot download the linter installation script from %q: %w", linterInstallURL, err)

dev-tools/templates/.golangci.yml

@@ -16,34 +16,21 @@ issues:
 output:
   sort-results: true
 
-# Uncomment and add a path if needed to exclude
-# skip-dirs:
-#   - some/path
-# skip-files:
-#   - ".*\\.my\\.go$"
-#   - lib/bad.go
-
 # Find the whole list here https://golangci-lint.run/usage/linters/
 linters:
   disable-all: true
   enable:
-    - deadcode # finds unused code
     - errcheck # checking for unchecked errors in go programs
     - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
-    - goconst # finds repeated strings that could be replaced by a constant
-    - dupl # tool for code clone detection
     - forbidigo # forbids identifiers	matched by reg exps
     - gomoddirectives # manage the use of 'replace', 'retract', and 'excludes' directives in go.mod.
     - gosimple # linter for Go source code that specializes in simplifying a code
     - misspell # finds commonly misspelled English words in comments
     - nakedret # finds naked returns in functions greater than a specified function length
-    - prealloc # finds slice declarations that could potentially be preallocated
     - nolintlint # reports ill-formed or insufficient nolint directives
     - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks
     - stylecheck # a replacement for golint
-    - unparam # reports unused function parameters
     - unused # checks Go code for unused constants, variables, functions and types
-
     - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
     - ineffassign # detects when assignments to existing variables are not used
     - structcheck # finds unused struct fields
@@ -60,15 +47,20 @@ linters:
     - noctx # noctx finds sending http request without context.Context
     - unconvert # Remove unnecessary type conversions
     - wastedassign # wastedassign finds wasted assignment statements.
-    - godox # tool for detection of FIXME, TODO and other comment keywords
     - gomodguard # check for blocked dependencies
 
 # all available settings of specific linters
 linters-settings:
   errcheck:
     # report about not checking of errors in type assertions: `a := b.(MyStruct)`;
-    # default is false: such cases aren't reported by default.
     check-type-assertions: true
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`.
+    check-blank: false
+    # List of functions to exclude from checking, where each entry is a single function to exclude.
+    # See https://github.com/kisielk/errcheck#excluding-functions for details.
+    exclude-functions:
+      - (mapstr.M).Delete # Only returns ErrKeyNotFound, can safely be ignored.
+      - (mapstr.M).Put # Can only fail on type conversions, usually safe to ignore.
 
   errorlint:
     # Check whether fmt.Errorf uses the %w verb for formatting errors. See the readme for caveats
@@ -78,16 +70,6 @@ linters-settings:
     # Check for plain error comparisons
     comparison: true
 
-  goconst:
-    # minimal length of string constant, 3 by default
-    min-len: 3
-    # minimal occurrences count to trigger, 3 by default
-    min-occurrences: 2
-
-  dupl:
-    # tokens count to trigger issue, 150 by default
-    threshold: 100
-
   forbidigo:
     # Forbid the following identifiers
     forbid:
@@ -103,30 +85,15 @@ linters-settings:
     # Select the Go version to target. The default is '1.13'.
     go: "{{.GoVersion}}"
 
-  misspell:
-    # Correct spellings using locale preferences for US or UK.
-    # Default is to use a neutral variety of English.
-    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
-    # locale: US
-    # ignore-words:
-    #   - IdP
-
   nakedret:
     # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
     max-func-lines: 0
 
-  prealloc:
-    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
-    # True by default.
-    simple: true
-    range-loops: true # Report preallocation suggestions on range loops, true by default
-    for-loops: false # Report preallocation suggestions on for loops, false by default
-
   nolintlint:
     # Enable to ensure that nolint directives are all used. Default is true.
     allow-unused: false
     # Disable to ensure that nolint directives don't have a leading space. Default is true.
-    allow-leading-space: true
+    allow-leading-space: false
     # Exclude following linters from requiring an explanation.  Default is [].
     allow-no-explanation: []
     # Enable to require an explanation of nonzero length after each nolint directive. Default is false.
@@ -144,7 +111,7 @@ linters-settings:
             recommendations:
               - errors
               - fmt
-            reason: "This package is deprecated"
+            reason: "This package is deprecated, use fmt.Errorf with %%w instead"
         - github.com/elastic/beats/v7:
             reason: "There must be no Beats dependency"
 
@@ -160,13 +127,14 @@ linters-settings:
     # https://staticcheck.io/docs/options#checks
     checks: ["all"]
 
-  unparam:
-    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
-    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-
   unused:
     # Select the Go version to target. The default is '1.13'.
     go: "{{.GoVersion}}"
+
+  gosec:
+    excludes:
+    - G306 # Expect WriteFile permissions to be 0600 or less
+    - G404 # Use of weak random number generator
+    - G401 # Detect the usage of DES, RC4, MD5 or SHA1: Used in non-crypto contexts.
+    - G501 # Import blocklist: crypto/md5: Used in non-crypto contexts.
+    - G505 # Import blocklist: crypto/sha1: Used in non-crypto contexts.

file/helper_other.go

@@ -43,7 +43,7 @@ func SyncParent(path string) error {
 	parent := filepath.Dir(path)
 	f, err := os.Open(parent)
 
-	// nolint: nilerr // ignore error
+	//nolint:nilerr // ignore error
 	if err != nil {
 		return nil
 	}

file/helper_test.go

@@ -29,7 +29,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// nolint: gosec // file permissions are valid for test purposes
 func TestSafeFileRotateExistingFile(t *testing.T) {
 	tempdir, err := ioutil.TempDir("", "")
 	assert.NoError(t, err)

kibana/client.go

@@ -110,7 +110,7 @@ func extractMessage(result []byte) error {
 		}
 	}
 	if err := json.Unmarshal(result, &kibanaResult); err != nil {
-		return nil // nolint: nilerr // we suppress some malformed errors on purpose
+		return nil //nolint:nilerr // we suppress some malformed errors on purpose
 	}
 
 	if !kibanaResult.Success {

match/matcher_test.go

@@ -15,7 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-// nolint: dupl // test cases are similar
 package match
 
 import (

service/service.go

@@ -24,7 +24,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
-	_ "net/http/pprof" // nolint: gosec // we want to expose it
+	_ "net/http/pprof" //nolint:gosec // we want to expose it
 	"os"
 	"os/signal"
 	"runtime"

transport/client.go

@@ -51,8 +51,8 @@ func NewClient(c Config, network, host string, defaultPort int) (*Client, error)
 	// do some sanity checks regarding network and Config matching +
 	// address being parseable
 	switch network {
-	case "tcp", "tcp4", "tcp6": // nolint: goconst // we do not win anything by making tcp a const
-	case "udp", "udp4", "udp6": // nolint: goconst // we do not win anything by making udp a const
+	case "tcp", "tcp4", "tcp6":
+	case "udp", "udp4", "udp6":
 		if c.TLS == nil && c.Proxy == nil {
 			break
 		}

transport/httpcommon/httpcommon.go

@@ -253,8 +253,8 @@ func (settings *HTTPTransportSettings) httpRoundTripper(
 	t := http.DefaultTransport.(*http.Transport).Clone()
 	t.DialContext = nil
 	t.DialTLSContext = nil
-	t.Dial = dialer.Dial       // nolint: staticcheck // use deprecated function to preserve functionality
-	t.DialTLS = tlsDialer.Dial // nolint: staticcheck // use deprecated function to preserve functionality
+	t.Dial = dialer.Dial       //nolint:staticcheck // use deprecated function to preserve functionality
+	t.DialTLS = tlsDialer.Dial //nolint:staticcheck // use deprecated function to preserve functionality
 	t.TLSClientConfig = tls.ToConfig()
 	t.ForceAttemptHTTP2 = false
 	t.Proxy = settings.Proxy.ProxyFunc()