[skip-changelog] Use golangci-lint (#2338)

Author: alessio-perugini

.github/workflows/check-go-task.yml

@@ -49,39 +49,6 @@ jobs:
 
           echo "result=$RESULT" >> $GITHUB_OUTPUT
 
-  check-errors:
-    name: check-errors (${{ matrix.module.path }})
-    needs: run-determination
-    if: needs.run-determination.outputs.result == 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      fail-fast: false
-
-      matrix:
-        module:
-          - path: ./
-
-    steps:
-      - name: Checkout repository
-        uses: actions/checkout@v4
-
-      - name: Install Go
-        uses: actions/setup-go@v4
-        with:
-          go-version: ${{ env.GO_VERSION }}
-
-      - name: Install Task
-        uses: arduino/setup-task@v1
-        with:
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
-          version: 3.x
-
-      - name: Check for errors
-        env:
-          GO_MODULE_PATH: ${{ matrix.module.path }}
-        run: task go:vet
-
   check-outdated:
     name: check-outdated (${{ matrix.module.path }})
     needs: run-determination
@@ -146,8 +113,10 @@ jobs:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
           version: 3.x
 
-      - name: Install golint
-        run: go install golang.org/x/lint/golint@latest
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: v1.54
 
       - name: Check style
         env:

.golangci.yml

@@ -0,0 +1,137 @@
+run:
+  timeout: 10m
+  go: "1.21"
+  tests: true
+
+linters:
+  # Disable all linters.
+  disable-all: true
+  # Enable specific linter
+  enable:
+    # Nice to have
+    #- depguard
+    #- errcheck
+    #- gocritic
+    #- thelper
+    - errorlint
+    - dupword
+    - exportloopref
+    - forbidigo
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - revive
+    - staticcheck
+    - tenv
+    - typecheck
+    - unconvert
+
+linters-settings:
+  forbidigo:
+    forbid:
+      - p: ^(fmt\.Print(|f|ln)|print|println)$
+        msg: in cli package use `feedback.*` instead
+      - p: (os\.(Stdout|Stderr|Stdin))(# )?
+        msg: in cli package use `feedback.*` instead
+    analyze-types: true
+
+  revive:
+    confidence: 0.8
+    rules:
+      #- name: error-return
+      #- name: unused-parameter
+      #- name: var-naming
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: empty-block
+      - name: error-naming
+      - name: error-strings
+      - name: errorf
+      - name: exported
+      - name: increment-decrement
+      - name: indent-error-flow
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: superfluous-else
+      - name: time-naming
+      - name: unexported-return
+      - name: unreachable-code
+      - name: var-declaration
+      - name: defer
+      - name: atomic
+      - name: waitgroup-by-value
+
+  errorlint:
+    # Check for plain error comparisons.
+    comparison: true
+
+    # We might evalute to allow the asserts and errofs in the future
+    # Do not check for plain type assertions and type switches.
+    asserts: false
+    # Do not check whether fmt.Errorf uses the %w verb for formatting errors.
+    errorf: false
+
+issues:
+  # Fix found issues (if it's supported by the linter).
+  fix: true
+  # List of regexps of issue texts to exclude.
+  #
+  # But independently of this option we use default exclude patterns,
+  # it can be disabled by `exclude-use-default: false`.
+  # To list all excluded by default patterns execute `golangci-lint run --help`
+  #
+  # Default: https://golangci-lint.run/usage/false-positives/#default-exclusions
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters: [gosec, errcheck]
+    # G401: Use of weak cryptographic primitive
+    - linters: [gosec]
+      text: "G401"
+    # G501: Blocklisted import crypto/md5: weak cryptographic primitive
+    - linters: [gosec]
+      text: "G501"
+    # G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server
+    - linters: [gosec]
+      path: internal/integrationtest/
+      text: "G112"
+    # G204: Subprocess launched with a potential tainted input or cmd arguments
+    - linters: [gosec]
+      path: executils/process.go
+      text: "G204"
+    # SA1019: req.GetQuery is deprecated: Marked as deprecated in cc/arduino/cli/commands/v1/lib.proto.
+    - linters: [staticcheck]
+      path: commands/lib/search.go
+      text: "SA1019"
+
+    # Ignore revive emptyblock
+    - linters: [revive]
+      path: arduino/libraries/loader.go
+      text: "empty-block"
+    - linters: [revive]
+      path: arduino/serialutils/serialutils.go
+      text: "empty-block"
+    - linters: [revive]
+      path: arduino/resources/download.go
+      text: "empty-block"
+    - linters: [revive]
+      path: arduino/builder/internal/progress/progress_test.go
+      text: "empty-block"
+    - linters: [revive]
+      path: internal/algorithms/channels.go
+      text: "empty-block"
+
+    # Run linters only on specific path
+    - path-except: internal/cli/
+      linters:
+        - forbidigo
+    - path: internal/cli/feedback/
+      linters: [forbidigo]

Taskfile.yml

@@ -80,14 +80,12 @@ tasks:
     dir: '{{default "./" .GO_MODULE_PATH}}'
     cmds:
       - |
-        if ! which golint &>/dev/null; then
-          echo "golint not installed or not in PATH. Please install: https://github.com/golang/lint#installation"
+        if ! which golangci-lint &>/dev/null; then
+          echo "golangci-lint not installed or not in PATH. Please install: https://golangci-lint.run/usage/install/"
           exit 1
         fi
       - |
-        golint \
-          {{default "-min_confidence 0.8 -set_exit_status" .GO_LINT_FLAGS}} \
-          {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}}
+        golangci-lint run
 
   # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/test-go-task/Taskfile.yml
   go:test:
@@ -128,13 +126,6 @@ tasks:
           {{.TEST_LDFLAGS}}
         go tool covdata textfmt -i=coverage_data -o coverage_integration.txt
 
-  # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-go-task/Taskfile.yml
-  go:vet:
-    desc: Check for errors in Go code
-    dir: '{{default "./" .GO_MODULE_PATH}}'
-    cmds:
-      - go vet {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}}
-
   go:easyjson-generate:
     desc: Run easyjson code generation
     cmds:
@@ -303,7 +294,6 @@ tasks:
   check:
     desc: Check fmt and lint
     cmds:
-      - task: go:vet
       - task: go:lint
       - task: protoc:check
 

arduino/builder/internal/preprocessor/internal/ctags/ctags_has_issues.go

@@ -206,78 +206,76 @@ func removeComments(text string, multilinecomment bool) (string, bool) {
 // FindCLinkageLines scans the source files searching for "extern C" context
 // It save the line numbers in a map filename -> {lines...}
 func (p *Parser) FindCLinkageLines(tags []*Tag) map[string][]int {
-
 	lines := make(map[string][]int)
 
 	for _, tag := range tags {
-
 		if lines[tag.Filename] != nil {
 			break
 		}
 
 		file, err := os.Open(tag.Filename)
-		if err == nil {
-			defer file.Close()
+		if err != nil {
+			continue
+		}
+		lines[tag.Filename] = append(lines[tag.Filename], -1)
 
-			lines[tag.Filename] = append(lines[tag.Filename], -1)
+		scanner := bufio.NewScanner(file)
 
-			scanner := bufio.NewScanner(file)
+		// we can't remove the comments otherwise the line number will be wrong
+		// there are three cases:
+		// 1 - extern "C" void foo()
+		// 2 - extern "C" {
+		//		void foo();
+		//		void bar();
+		//	}
+		// 3 - extern "C"
+		//	{
+		//		void foo();
+		//		void bar();
+		//	}
+		// case 1 and 2 can be simply recognized with string matching and indent level count
+		// case 3 needs specia attention: if the line ONLY contains `extern "C"` string, don't bail out on indent level = 0
+
+		inScope := false
+		enteringScope := false
+		indentLevels := 0
+		line := 0
 
-			// we can't remove the comments otherwise the line number will be wrong
-			// there are three cases:
-			// 1 - extern "C" void foo()
-			// 2 - extern "C" {
-			//		void foo();
-			//		void bar();
-			//	}
-			// 3 - extern "C"
-			//	{
-			//		void foo();
-			//		void bar();
-			//	}
-			// case 1 and 2 can be simply recognized with string matching and indent level count
-			// case 3 needs specia attention: if the line ONLY contains `extern "C"` string, don't bail out on indent level = 0
-
-			inScope := false
-			enteringScope := false
-			indentLevels := 0
-			line := 0
-
-			externCDecl := removeSpacesAndTabs(keywordExternC)
-
-			for scanner.Scan() {
-				line++
-				str := removeSpacesAndTabs(scanner.Text())
+		externCDecl := removeSpacesAndTabs(keywordExternC)
 
-				if len(str) == 0 {
-					continue
-				}
+		for scanner.Scan() {
+			line++
+			str := removeSpacesAndTabs(scanner.Text())
 
-				// check if we are on the first non empty line after externCDecl in case 3
-				if enteringScope {
-					enteringScope = false
-				}
+			if len(str) == 0 {
+				continue
+			}
 
-				// check if the line contains externCDecl
-				if strings.Contains(str, externCDecl) {
-					inScope = true
-					if len(str) == len(externCDecl) {
-						// case 3
-						enteringScope = true
-					}
-				}
-				if inScope {
-					lines[tag.Filename] = append(lines[tag.Filename], line)
-				}
-				indentLevels += strings.Count(str, "{") - strings.Count(str, "}")
+			// check if we are on the first non empty line after externCDecl in case 3
+			if enteringScope {
+				enteringScope = false
+			}
 
-				// Bail out if indentLevel is zero and we are not in case 3
-				if indentLevels == 0 && !enteringScope {
-					inScope = false
+			// check if the line contains externCDecl
+			if strings.Contains(str, externCDecl) {
+				inScope = true
+				if len(str) == len(externCDecl) {
+					// case 3
+					enteringScope = true
 				}
 			}
+			if inScope {
+				lines[tag.Filename] = append(lines[tag.Filename], line)
+			}
+			indentLevels += strings.Count(str, "{") - strings.Count(str, "}")
+
+			// Bail out if indentLevel is zero and we are not in case 3
+			if indentLevels == 0 && !enteringScope {
+				inScope = false
+			}
 		}
 
+		file.Close()
 	}
 	return lines
 }

arduino/builder/internal/utils/utils.go

@@ -277,7 +277,7 @@ func ExecCommand(
 	return verboseInfoBuf.Bytes(), stdoutBuffer.Bytes(), stderrBuffer.Bytes(), errors.WithStack(err)
 }
 
-// DirContentIsOlderThan DirContentIsOlderThan returns true if the content of the given directory is
+// DirContentIsOlderThan returns true if the content of the given directory is
 // older than target file. If extensions are given, only the files with these
 // extensions are tested.
 func DirContentIsOlderThan(dir *paths.Path, target *paths.Path, extensions ...string) (bool, error) {

arduino/builder/linker.go

@@ -50,7 +50,7 @@ func (b *Builder) link() error {
 	// and use that archives to complete the build.
 	if len(objectFileList) > 30000 {
 
-		// We must create an object file for each visited directory: this is required becuase gcc-ar checks
+		// We must create an object file for each visited directory: this is required because gcc-ar checks
 		// if an object file is already in the archive by looking ONLY at the filename WITHOUT the path, so
 		// it may happen that a subdir/spi.o inside the archive may be overwritten by a anotherdir/spi.o
 		// because thery are both named spi.o.

arduino/builder/sketch.go

@@ -140,7 +140,7 @@ func (b *Builder) sketchCopyAdditionalFiles(buildPath *paths.Path, overrides map
 			sourceBytes = s
 		}
 
-		// tag each addtional file with the filename of the source it was copied from
+		// tag each additional file with the filename of the source it was copied from
 		sourceBytes = append([]byte("#line 1 "+cpp.QuoteString(file.String())+"\n"), sourceBytes...)
 
 		err = writeIfDifferent(sourceBytes, targetPath)

arduino/cores/packagemanager/package_manager.go

@@ -319,7 +319,7 @@ func (pme *Explorer) ResolveFQBN(fqbn *cores.FQBN) (
 	}
 
 	// Create the build properties map by overlaying the properties of the
-	// referenced platform propeties, the board platform properties and the
+	// referenced platform properties, the board platform properties and the
 	// board specific properties.
 	buildProperties := properties.NewMap()
 	buildProperties.Merge(variantPlatformRelease.Properties)