Merge branch 'master' into fix/skip-recent-repositories

Author: gforien

.github/workflows/ci.yml

@@ -169,9 +169,9 @@ jobs:
         with:
           go-version: 1.24.x
       - name: Lint
-        uses: golangci/golangci-lint-action@v6.5.0
+        uses: golangci/golangci-lint-action@v8
         with:
-          version: v1.64.6
+          version: v2.2.1
       - name: errors
         run: golangci-lint run
         if: ${{ failure() }}

.golangci.yml

@@ -1,36 +1,114 @@
+version: "2"
+run:
+  go: "1.24"
 linters:
   enable:
-    - gofumpt
-    - thelper
-    - goimports
-    - tparallel
-    - wastedassign
-    - unparam
-    - prealloc
-    - unconvert
+    - copyloopvar
+    - errorlint
     - exhaustive
+    - intrange
     - makezero
     - nakedret
-    - copyloopvar
-  fast: false
-
-linters-settings:
-  copyloopvar:
-    # Check all assigning the loop variable to another variable.
-    # Default: false
-    # If true, an assignment like `a := x` will be detected as an error.
-    check-alias: true
-  exhaustive:
-    default-signifies-exhaustive: true
-  staticcheck:
-    # SA1019 is for checking that we're not using fields marked as deprecated
-    # in a comment. It decides this in a loose way so I'm silencing it. Also because
-    # it's tripping on our own structs.
-    checks: ["all", "-SA1019"]
-  nakedret:
-    # the gods will judge me but I just don't like naked returns at all
-    max-func-lines: 0
+    - nolintlint
+    - prealloc
+    - revive
+    - thelper
+    - tparallel
+    - unconvert
+    - unparam
+    - wastedassign
+  settings:
+    copyloopvar:
+      check-alias: true
+    exhaustive:
+      default-signifies-exhaustive: true
+    nakedret:
+      # the gods will judge me but I just don't like naked returns at all
+      max-func-lines: 0
+    staticcheck:
+      checks:
+        - all
 
-run:
-  go: "1.24"
-  timeout: 10m
+        # SA1019 is for checking that we're not using fields marked as
+        # deprecated in a comment. It decides this in a loose way so I'm
+        # silencing it. Also because it's tripping on our own structs.
+        - -SA1019
+
+        # ST1003 complains about names like remoteUrl or itemId (should be
+        # remoteURL and itemID). While I like these suggestions, it also
+        # complains about enum constants that are all caps, and we use these and
+        # I like them, and also about camelCase identifiers that contain an
+        # underscore, which we also use in a few places. Since it can't be
+        # configured to ignore specific cases, and I don't want to use nolint
+        # comments in the code, we have to disable it altogether.
+        - -ST1003 # Poorly chosen identifier
+
+        # Probably a good idea, but we first have to review our error reporting
+        # strategy to be able to use it everywhere.
+        - -ST1005 # Error strings should not be capitalized
+
+        # Many of our classes use self as a receiver name, and we think that's fine.
+        - -ST1006 # Use of self or this as receiver name
+
+        # De Morgan's law suggests to replace `!(a && b)` with `!a || !b`; but
+        # sometimes I find one more readable than the other, so I want to decide
+        # that myself.
+        - -QF1001 # De Morgan's law
+
+        # QF1003 is about using a tagged switch instead of an if-else chain. In
+        # many cases this is a useful suggestion; however, sometimes the change
+        # is only possible by adding a default case to the switch (when there
+        # was no `else` block in the original code), in which case I don't find
+        # it to be an improvement.
+        - -QF1003 # Could replace with tagged switch
+
+        # We need to review our use of embedded fields. I suspect that in some
+        # cases the fix is not to remove the selector for the embedded field,
+        # but to turn the embedded field into a named field.
+        - -QF1008 # Could remove embedded field from selector
+
+        # The following checks are all disabled by default in golangci-lint, but
+        # we disable them again explicitly here to make it easier to keep this
+        # list in sync with the gopls config in .vscode/settings.json.
+        - -ST1000, # At least one file in a package should have a package comment
+        - -ST1020, # The documentation of an exported function should start with the function's name
+        - -ST1021, # The documentation of an exported type should start with type's name
+        - -ST1022, # The documentation of an exported variable or constant should start with variable's name
+
+      dot-import-whitelist:
+        - github.com/jesseduffield/lazygit/pkg/integration/components
+    revive:
+      severity: warning
+      rules:
+        - name: atomic
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: error-naming
+        - name: var-declaration
+        - name: package-comments
+        - name: range
+        - name: time-naming
+        - name: indent-error-flow
+        - name: errorf
+        - name: superfluous-else
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofumpt
+    - goimports
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

.vscode/settings.json

@@ -1,5 +1,29 @@
 {
   "gopls": {
     "formatting.gofumpt": true,
+    "ui.diagnostic.staticcheck": true,
+    "ui.diagnostic.analyses": {
+      // This list must match the one in .golangci.yml
+      "SA1019": false,
+      "ST1003": false,
+      "ST1005": false,
+      "ST1006": false,
+      "QF1001": false,
+      "QF1003": false,
+      "QF1008": false,
+      "ST1000": false,
+      "ST1020": false,
+      "ST1021": false,
+      "ST1022": false,
+      // Dot imports; this warning is enabled in .golangci.yml, but with an
+      // extra dot-import-whitelist config. Because I couldn't figure out how to
+      // specify that extra config for gopls, I'm disabling the check altogether
+      // here.
+      "ST1001": false,
+    },
   },
+  "go.alternateTools": {
+    "golangci-lint-v2": "${workspaceFolder}/.bin/golangci-lint",
+  },
+  "go.lintTool": "golangci-lint-v2",
 }

Makefile

@@ -40,7 +40,7 @@ format:
 
 .PHONY: lint
 lint:
-	golangci-lint run
+	./scripts/lint.sh
 
 # For more details about integration test, see https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md.
 .PHONY: integration-test-tui

go.mod

@@ -47,7 +47,7 @@ require (
 	github.com/ProtonMail/go-crypto v1.1.6 // indirect
 	github.com/bahlo/generic-list-go v0.2.0 // indirect
 	github.com/buger/jsonparser v1.1.1 // indirect
-	github.com/cloudflare/circl v1.6.0 // indirect
+	github.com/cloudflare/circl v1.6.1 // indirect
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect

go.sum

@@ -65,8 +65,8 @@ github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWR
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
-github.com/cloudflare/circl v1.6.0 h1:cr5JKic4HI+LkINy2lg3W2jF8sHCVTBncJr5gIIq7qk=
-github.com/cloudflare/circl v1.6.0/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs=
+github.com/cloudflare/circl v1.6.1 h1:zqIqSPIndyBh1bjLVVDHMPpVKqp8Su/V+6MeDzzQBQ0=
+github.com/cloudflare/circl v1.6.1/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs=
 github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 h1:tuijfIjZyjZaHq9xDUh0tNitwXshJpbLkqMOJv4H3do=
 github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21/go.mod h1:po7NpZ/QiTKzBKyrsEAxwnTamCoh8uDk/egRpQ7siIc=
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=

pkg/app/app.go

@@ -86,9 +86,9 @@ func newLogger(cfg config.AppConfigurer) *logrus.Entry {
 			log.Fatal(err)
 		}
 		return logs.NewDevelopmentLogger(logPath)
-	} else {
-		return logs.NewProductionLogger()
 	}
+
+	return logs.NewProductionLogger()
 }
 
 // NewApp bootstrap a new application

pkg/app/daemon/rebase.go

@@ -21,9 +21,8 @@ type TodoLine struct {
 func (self *TodoLine) ToString() string {
 	if self.Action == "break" {
 		return self.Action + "\n"
-	} else {
-		return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n"
 	}
+	return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n"
 }
 
 func TodoLinesToString(todoLines []TodoLine) string {

pkg/commands/git_cmd_obj_runner.go

@@ -28,7 +28,7 @@ func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error {
 func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) {
 	var output string
 	var err error
-	for i := 0; i < RetryCount; i++ {
+	for range RetryCount {
 		newCmdObj := cmdObj.Clone()
 		output, err = self.innerRunner.RunWithOutput(newCmdObj)
 
@@ -47,7 +47,7 @@ func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, e
 func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
 	var stdout, stderr string
 	var err error
-	for i := 0; i < RetryCount; i++ {
+	for range RetryCount {
 		newCmdObj := cmdObj.Clone()
 		stdout, stderr, err = self.innerRunner.RunWithOutputs(newCmdObj)
 

pkg/commands/git_commands/branch.go

@@ -102,19 +102,18 @@ func (self *BranchCommands) CurrentBranchInfo() (BranchInfo, error) {
 	}, nil
 }
 
-// CurrentBranchName get name of current branch
+// CurrentBranchName get name of current branch. Returns empty string if HEAD is detached.
 func (self *BranchCommands) CurrentBranchName() (string, error) {
-	cmdArgs := NewGitCmd("rev-parse").
-		Arg("--abbrev-ref").
-		Arg("--verify").
-		Arg("HEAD").
+	cmdArgs := NewGitCmd("branch").
+		Arg("--show-current").
 		ToArgv()
 
 	output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
-	if err == nil {
-		return strings.TrimSpace(output), nil
+	if err != nil {
+		return "", err
 	}
-	return "", err
+
+	return strings.TrimSpace(output), nil
 }
 
 // LocalDelete delete branch locally