ci: upgrade golangci-lint and enable misspell linter (#4540)

* ci: upgrade golangci-lint

* refactor: apply `golangci-lint run --fix`

* refactor: manually address some lint issues

* fix: fix misspelled words

I'm not the only one :)

Author: kzys

.github/workflows/checks.yml

@@ -41,6 +41,6 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version-file: "go.mod"
-      - uses: golangci/golangci-lint-action@v6
+      - uses: golangci/golangci-lint-action@v8
         with:
-          version: v1.64
+          version: v2.4.0

.golangci.yml

@@ -1,96 +1,71 @@
-issues:
-  # List of regexps of issue texts to exclude, empty list by default.
-  # But independently from 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`
-
-  exclude-rules:
-    # Exclude gosimple bool check
-    - linters:
-        - gosimple
-      text: "S(1002|1008|1021)"
-    # Exclude failing staticchecks for now
-    - linters:
-        - staticcheck
-      text: "SA(1006|1019|4006|4010|4017|5007|6005|9004):"
-    # Exclude lll issues for long lines with go:generate
-    - linters:
-        - lll
-      source: "^//go:generate "
-
-  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
-  max-issues-per-linter: 0
-
-  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
-  max-same-issues: 0
-
+version: "2"
+run:
+  concurrency: 4
+  go: "1.21"
+  issues-exit-code: 1
+  tests: true
 linters:
-  disable-all: true
+  default: none
   enable:
-    - gofmt
-    - gosimple
     - govet
     - ineffassign
     - staticcheck
     - unconvert
     - unused
-
-# options for analysis running
-run:
-  go: "1.21"
-
-  # default concurrency is a available CPU number
-  concurrency: 4
-
-  # timeout for analysis, e.g. 30s, 5m, default is 1m
-  timeout: 10m
-
-  # exit code when at least one issue was found, default is 1
-  issues-exit-code: 1
-
-  # include test files or not, default is true
-  tests: true
-
-  # list of build tags, all linters use it. Default is empty list.
-  #build-tags:
-  #  - mytag
-  #  - lib/bad.go
-
-  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
-  # If invoked with -mod=readonly, the go command is disallowed from the implicit
-  # automatic updating of go.mod described above. Instead, it fails when any changes
-  # to go.mod are needed. This setting is most useful to check that go.mod does
-  # not need updates, such as in a continuous integration and testing system.
-  # If invoked with -mod=vendor, the go command assumes that the vendor
-  # directory holds the correct copies of dependencies and ignores
-  # the dependency descriptions in go.mod.
-  # modules-download-mode: vendor
-
-# output configuration options
-output:
-  # print lines of code with issue, default is true
-  print-issued-lines: true
-
-  # print linter name in the end of issue text, default is true
-  print-linter-name: true
-
-
-  sort-results: true
-
-# all available settings of specific linters
-linters-settings:
-  errcheck:
-    exclude-functions:
-      - fmt.*
-      - io.Close
-  govet:
-    settings:
-      printf:
-        funcs:
-          - github.com/superfly/flyctl/terminal.Debugf
-          - github.com/superfly/flyctl/terminal.Infof
-          - github.com/superfly/flyctl/terminal.Warnf
-          - github.com/superfly/flyctl/terminal.Errorf
-          - github.com/superfly/flyctl/render.Printf
-          - github.com/superfly/flyctl/render.Detailf
-          - github.com/superfly/flyctl/render.Donef
+    - misspell
+  settings:
+    errcheck:
+      exclude-functions:
+        - fmt.*
+        - io.Close
+    govet:
+      settings:
+        printf:
+          funcs:
+            - github.com/superfly/flyctl/terminal.Debugf
+            - github.com/superfly/flyctl/terminal.Infof
+            - github.com/superfly/flyctl/terminal.Warnf
+            - github.com/superfly/flyctl/terminal.Errorf
+            - github.com/superfly/flyctl/render.Printf
+            - github.com/superfly/flyctl/render.Detailf
+            - github.com/superfly/flyctl/render.Donef
+    staticcheck:
+      checks:
+        - all
+        - -SA1019 # ... is deprecated
+        - -ST1003 # struct field ... should be ... (mostly acronyms such as Http -> HTTP)
+        - -ST1005 # error strings should not be capitalized
+        - -ST1008 # error should be returned as the last argument
+        - -ST1012 # error var ... should have name of the form Err...
+        - -QF1001 # could apply De Morgan's law
+        - -QF1002 # could use tagged switch
+        - -QF1003 # could use tagged switch on app
+        - -QF1008 # could remove embedded field ... from selector
+        - -QF1012 # Use fmt.Fprintf(...) instead of WriteString(fmt.Sprintf(...))
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - lll
+        source: '^//go:generate '
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+formatters:
+  enable:
+    - gofmt
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

.pre-commit-config.yaml

@@ -18,7 +18,7 @@ repos:
 # because goalngci-lint github action is much more useful than the pre-commit action.
 # The trick is to run github action only for "manual" hook stage
 -   repo: https://github.com/golangci/golangci-lint
-    rev: v1.54.2
+    rev: v1.24.0
     hooks:
     -   id: golangci-lint
         stages: [pre-commit]

agent/client.go

@@ -59,7 +59,7 @@ func Establish(ctx context.Context, apiClient wireguard.WebClient) (*Client, err
 		return c, nil
 	}
 
-	// TOOD: log this instead
+	// TODO: log this instead
 	msg := fmt.Sprintf("The running flyctl agent (v%s) is older than the current flyctl (v%s).", res.Version, buildinfo.Version())
 
 	logger := logger.MaybeFromContext(ctx)

doc/main.go

@@ -110,7 +110,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string)
 			}
 			cname := name + " " + child.Name()
 			link := cname + ".md"
-			link = strings.Replace(link, " ", "_", -1)
+			link = strings.ReplaceAll(link, " ", "_")
 			buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", child.Name(), linkHandler(link), child.Short))
 		}
 		buf.WriteString("\n")
@@ -130,7 +130,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string)
 			parent := cmd.Parent()
 			pname := parent.CommandPath()
 			link := pname + ".md"
-			link = strings.Replace(link, " ", "_", -1)
+			link = strings.ReplaceAll(link, " ", "_")
 			buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", pname, linkHandler(link), parent.Short))
 			cmd.VisitParents(func(c *cobra.Command) {
 				if c.DisableAutoGenTag {
@@ -172,7 +172,7 @@ func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHa
 		}
 	}
 
-	basename := strings.Replace(cmd.CommandPath(), " ", "_", -1) + ".md"
+	basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + ".md"
 	filename := filepath.Join(dir, basename)
 	f, err := os.Create(filename)
 	if err != nil {

internal/appconfig/config.go

@@ -298,48 +298,48 @@ func (c *Config) InternalPort() int {
 	return 0
 }
 
-func (cfg *Config) BuildStrategies() []string {
+func (c *Config) BuildStrategies() []string {
 	strategies := []string{}
 
-	if cfg == nil || cfg.Build == nil {
+	if c == nil || c.Build == nil {
 		return strategies
 	}
 
-	if cfg.Build.Image != "" {
-		strategies = append(strategies, fmt.Sprintf("the \"%s\" docker image", cfg.Build.Image))
+	if c.Build.Image != "" {
+		strategies = append(strategies, fmt.Sprintf("the \"%s\" docker image", c.Build.Image))
 	}
-	if cfg.Build.Builder != "" || len(cfg.Build.Buildpacks) > 0 {
+	if c.Build.Builder != "" || len(c.Build.Buildpacks) > 0 {
 		strategies = append(strategies, "a buildpack")
 	}
-	if cfg.Build.Dockerfile != "" || cfg.Build.DockerBuildTarget != "" {
-		if cfg.Build.Dockerfile != "" {
-			strategies = append(strategies, fmt.Sprintf("the \"%s\" dockerfile", cfg.Build.Dockerfile))
+	if c.Build.Dockerfile != "" || c.Build.DockerBuildTarget != "" {
+		if c.Build.Dockerfile != "" {
+			strategies = append(strategies, fmt.Sprintf("the \"%s\" dockerfile", c.Build.Dockerfile))
 		} else {
 			strategies = append(strategies, "a dockerfile")
 		}
 	}
-	if cfg.Build.Builtin != "" {
-		strategies = append(strategies, fmt.Sprintf("the \"%s\" builtin image", cfg.Build.Builtin))
+	if c.Build.Builtin != "" {
+		strategies = append(strategies, fmt.Sprintf("the \"%s\" builtin image", c.Build.Builtin))
 	}
 
 	return strategies
 }
 
-func (cfg *Config) URL() *url.URL {
+func (c *Config) URL() *url.URL {
 	u := &url.URL{
 		Scheme: "https",
-		Host:   cfg.AppName + ".fly.dev",
+		Host:   c.AppName + ".fly.dev",
 		Path:   "/",
 	}
 
 	// HTTPService always listen on https, even if ForceHTTPS is false
-	if cfg.HTTPService != nil && cfg.HTTPService.InternalPort > 0 {
+	if c.HTTPService != nil && c.HTTPService.InternalPort > 0 {
 		return u
 	}
 
 	var httpPorts []int
 	var httpsPorts []int
-	for _, service := range cfg.Services {
+	for _, service := range c.Services {
 		for _, port := range service.Ports {
 			if port.Port == nil || !slices.Contains(port.Handlers, "http") {
 				continue
@@ -374,10 +374,10 @@ func (cfg *Config) URL() *url.URL {
 
 // MergeFiles merges the provided files with the files in the config wherein the provided files
 // take precedence.
-func (cfg *Config) MergeFiles(files []*fly.File) error {
+func (c *Config) MergeFiles(files []*fly.File) error {
 	// First convert the Config files to Machine files.
-	cfgFiles := make([]*fly.File, 0, len(cfg.Files))
-	for _, f := range cfg.Files {
+	cfgFiles := make([]*fly.File, 0, len(c.Files))
+	for _, f := range c.Files {
 		machineFile, err := f.toMachineFile()
 		if err != nil {
 			return err
@@ -392,29 +392,29 @@ func (cfg *Config) MergeFiles(files []*fly.File) error {
 	fly.MergeFiles(mConfig, files)
 
 	// Persist the merged files back to the config to be used later for deploying.
-	cfg.MergedFiles = mConfig.Files
+	c.MergedFiles = mConfig.Files
 
 	return nil
 }
 
-func (cfg *Config) DeployStrategy() string {
-	if cfg.Deploy == nil {
+func (c *Config) DeployStrategy() string {
+	if c.Deploy == nil {
 		return ""
 	}
-	return cfg.Deploy.Strategy
+	return c.Deploy.Strategy
 }
 
 // DetectComposeFile returns Build.Compose.File if set, otherwise looks for
 // well-known compose filenames in the directory containing the config file.
 // Returns the first found filename or empty string.
-func (cfg *Config) DetectComposeFile() string {
+func (c *Config) DetectComposeFile() string {
 	// If compose file is explicitly set, return it
-	if cfg.Build != nil && cfg.Build.Compose != nil && cfg.Build.Compose.File != "" {
-		return cfg.Build.Compose.File
+	if c.Build != nil && c.Build.Compose != nil && c.Build.Compose.File != "" {
+		return c.Build.Compose.File
 	}
 
 	// Otherwise, detect well-known filenames
-	configDir := filepath.Dir(cfg.configFilePath)
+	configDir := filepath.Dir(c.configFilePath)
 
 	for _, filename := range WellKnownComposeFilenames {
 		path := filepath.Join(configDir, filename)

internal/appconfig/machines.go

@@ -239,7 +239,7 @@ func (c *Config) ToConsoleMachineConfig() (*fly.MachineConfig, error) {
 
 // updateMachineConfig applies configuration options from the optional MachineConfig passed in, then the base config, into a new MachineConfig
 func (c *Config) updateMachineConfig(src *fly.MachineConfig) (*fly.MachineConfig, error) {
-	// For flattened app configs there is only one proces name and it is the group it was flattened for
+	// For flattened app configs there is only one process name and it is the group it was flattened for
 	processGroup := c.DefaultProcessName()
 
 	mConfig := &fly.MachineConfig{}