Skip to content

Commit 36388e5

Browse files
stefanhallerstefanhaller
authored
Update linter (#4671)
- Update the golangci-lint configuration to use the latest version (2.2.1) - Enable a few more linter checks - Improve the VS Code configuration so that we don't get spurious linter warnings that wouldn't also show up on CI - Provide a way to run the exact same golangci-lint version locally that we use on CI. This is useful in case the globally installed version is a newer one which emits more warnings.
2 parents 4439531 + ca05a2c commit 36388e5

File tree

82 files changed

+464
-382
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+464
-382
lines changed

‎.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ jobs:
169169
with:
170170
go-version: 1.24.x
171171
- name: Lint
172-
uses: golangci/golangci-lint-action@v6.5.0
172+
uses: golangci/golangci-lint-action@v8
173173
with:
174-
version: v1.64.6
174+
version: v2.2.1
175175
- name: errors
176176
run: golangci-lint run
177177
if: ${{ failure() }}

‎.golangci.yml

Lines changed: 108 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,114 @@
1+
version: "2"
2+
run:
3+
go: "1.24"
14
linters:
25
enable:
3-
- gofumpt
4-
- thelper
5-
- goimports
6-
- tparallel
7-
- wastedassign
8-
- unparam
9-
- prealloc
10-
- unconvert
6+
- copyloopvar
7+
- errorlint
118
- exhaustive
9+
- intrange
1210
- makezero
1311
- nakedret
14-
- copyloopvar
15-
fast: false
16-
17-
linters-settings:
18-
copyloopvar:
19-
# Check all assigning the loop variable to another variable.
20-
# Default: false
21-
# If true, an assignment like `a := x` will be detected as an error.
22-
check-alias: true
23-
exhaustive:
24-
default-signifies-exhaustive: true
25-
staticcheck:
26-
# SA1019 is for checking that we're not using fields marked as deprecated
27-
# in a comment. It decides this in a loose way so I'm silencing it. Also because
28-
# it's tripping on our own structs.
29-
checks: ["all", "-SA1019"]
30-
nakedret:
31-
# the gods will judge me but I just don't like naked returns at all
32-
max-func-lines: 0
12+
- nolintlint
13+
- prealloc
14+
- revive
15+
- thelper
16+
- tparallel
17+
- unconvert
18+
- unparam
19+
- wastedassign
20+
settings:
21+
copyloopvar:
22+
check-alias: true
23+
exhaustive:
24+
default-signifies-exhaustive: true
25+
nakedret:
26+
# the gods will judge me but I just don't like naked returns at all
27+
max-func-lines: 0
28+
staticcheck:
29+
checks:
30+
- all
3331

34-
run:
35-
go: "1.24"
36-
timeout: 10m
32+
# SA1019 is for checking that we're not using fields marked as
33+
# deprecated in a comment. It decides this in a loose way so I'm
34+
# silencing it. Also because it's tripping on our own structs.
35+
- -SA1019
36+
37+
# ST1003 complains about names like remoteUrl or itemId (should be
38+
# remoteURL and itemID). While I like these suggestions, it also
39+
# complains about enum constants that are all caps, and we use these and
40+
# I like them, and also about camelCase identifiers that contain an
41+
# underscore, which we also use in a few places. Since it can't be
42+
# configured to ignore specific cases, and I don't want to use nolint
43+
# comments in the code, we have to disable it altogether.
44+
- -ST1003 # Poorly chosen identifier
45+
46+
# Probably a good idea, but we first have to review our error reporting
47+
# strategy to be able to use it everywhere.
48+
- -ST1005 # Error strings should not be capitalized
49+
50+
# Many of our classes use self as a receiver name, and we think that's fine.
51+
- -ST1006 # Use of self or this as receiver name
52+
53+
# De Morgan's law suggests to replace `!(a && b)` with `!a || !b`; but
54+
# sometimes I find one more readable than the other, so I want to decide
55+
# that myself.
56+
- -QF1001 # De Morgan's law
57+
58+
# QF1003 is about using a tagged switch instead of an if-else chain. In
59+
# many cases this is a useful suggestion; however, sometimes the change
60+
# is only possible by adding a default case to the switch (when there
61+
# was no `else` block in the original code), in which case I don't find
62+
# it to be an improvement.
63+
- -QF1003 # Could replace with tagged switch
64+
65+
# We need to review our use of embedded fields. I suspect that in some
66+
# cases the fix is not to remove the selector for the embedded field,
67+
# but to turn the embedded field into a named field.
68+
- -QF1008 # Could remove embedded field from selector
69+
70+
# The following checks are all disabled by default in golangci-lint, but
71+
# we disable them again explicitly here to make it easier to keep this
72+
# list in sync with the gopls config in .vscode/settings.json.
73+
- -ST1000, # At least one file in a package should have a package comment
74+
- -ST1020, # The documentation of an exported function should start with the function's name
75+
- -ST1021, # The documentation of an exported type should start with type's name
76+
- -ST1022, # The documentation of an exported variable or constant should start with variable's name
77+
78+
dot-import-whitelist:
79+
- github.com/jesseduffield/lazygit/pkg/integration/components
80+
revive:
81+
severity: warning
82+
rules:
83+
- name: atomic
84+
- name: context-as-argument
85+
- name: context-keys-type
86+
- name: error-naming
87+
- name: var-declaration
88+
- name: package-comments
89+
- name: range
90+
- name: time-naming
91+
- name: indent-error-flow
92+
- name: errorf
93+
- name: superfluous-else
94+
exclusions:
95+
generated: lax
96+
presets:
97+
- comments
98+
- common-false-positives
99+
- legacy
100+
- std-error-handling
101+
paths:
102+
- third_party$
103+
- builtin$
104+
- examples$
105+
formatters:
106+
enable:
107+
- gofumpt
108+
- goimports
109+
exclusions:
110+
generated: lax
111+
paths:
112+
- third_party$
113+
- builtin$
114+
- examples$

‎.vscode/settings.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,29 @@
11
{
22
"gopls": {
33
"formatting.gofumpt": true,
4+
"ui.diagnostic.staticcheck": true,
5+
"ui.diagnostic.analyses": {
6+
// This list must match the one in .golangci.yml
7+
"SA1019": false,
8+
"ST1003": false,
9+
"ST1005": false,
10+
"ST1006": false,
11+
"QF1001": false,
12+
"QF1003": false,
13+
"QF1008": false,
14+
"ST1000": false,
15+
"ST1020": false,
16+
"ST1021": false,
17+
"ST1022": false,
18+
// Dot imports; this warning is enabled in .golangci.yml, but with an
19+
// extra dot-import-whitelist config. Because I couldn't figure out how to
20+
// specify that extra config for gopls, I'm disabling the check altogether
21+
// here.
22+
"ST1001": false,
23+
},
424
},
25+
"go.alternateTools": {
26+
"golangci-lint-v2": "${workspaceFolder}/.bin/golangci-lint",
27+
},
28+
"go.lintTool": "golangci-lint-v2",
529
}

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ format:
4040

4141
.PHONY: lint
4242
lint:
43-
golangci-lint run
43+
./scripts/lint.sh
4444

4545
# For more details about integration test, see https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md.
4646
.PHONY: integration-test-tui

‎pkg/app/app.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ func newLogger(cfg config.AppConfigurer) *logrus.Entry {
8686
log.Fatal(err)
8787
}
8888
return logs.NewDevelopmentLogger(logPath)
89-
} else {
90-
return logs.NewProductionLogger()
9189
}
90+
91+
return logs.NewProductionLogger()
9292
}
9393

9494
// NewApp bootstrap a new application

‎pkg/app/daemon/rebase.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ type TodoLine struct {
2121
func (self *TodoLine) ToString() string {
2222
if self.Action == "break" {
2323
return self.Action + "\n"
24-
} else {
25-
return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n"
2624
}
25+
return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n"
2726
}
2827

2928
func TodoLinesToString(todoLines []TodoLine) string {

‎pkg/commands/git_cmd_obj_runner.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error {
2828
func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) {
2929
var output string
3030
var err error
31-
for i := 0; i < RetryCount; i++ {
31+
for range RetryCount {
3232
newCmdObj := cmdObj.Clone()
3333
output, err = self.innerRunner.RunWithOutput(newCmdObj)
3434

@@ -47,7 +47,7 @@ func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, e
4747
func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
4848
var stdout, stderr string
4949
var err error
50-
for i := 0; i < RetryCount; i++ {
50+
for range RetryCount {
5151
newCmdObj := cmdObj.Clone()
5252
stdout, stderr, err = self.innerRunner.RunWithOutputs(newCmdObj)
5353

‎pkg/commands/git_commands/branch_loader.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,8 @@ func parseDifference(track string, regexStr string) string {
356356
match := re.FindStringSubmatch(track)
357357
if len(match) > 1 {
358358
return match[1]
359-
} else {
360-
return "0"
361359
}
360+
return "0"
362361
}
363362

364363
// TODO: only look at the new reflog commits, and otherwise store the recencies in

‎pkg/commands/git_commands/commit.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,8 @@ func (self *CommitCommands) CommitEditorCmdObj() *oscommands.CmdObj {
149149
func (self *CommitCommands) signoffFlag() string {
150150
if self.UserConfig().Git.Commit.SignOff {
151151
return "--signoff"
152-
} else {
153-
return ""
154152
}
153+
return ""
155154
}
156155

157156
func (self *CommitCommands) GetCommitMessage(commitHash string) (string, error) {

‎pkg/commands/git_commands/commit_loader_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ import (
1616
"github.com/stretchr/testify/assert"
1717
)
1818

19-
var commitsOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode
19+
var commitsOutput = strings.ReplaceAll(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode
2020
b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|jessedduffield@gmail.com|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging
2121
e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|jessedduffield@gmail.com|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor
2222
d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|jessedduffield@gmail.com||65f910ebd85283b5cce9|>|WIP
2323
65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|jessedduffield@gmail.com||26c07b1ab33860a1a759|>|WIP
2424
26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|jessedduffield@gmail.com||3d4470a6c072208722e5|>|WIP
2525
3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|jessedduffield@gmail.com||053a66a7be3da43aacdc|>|WIP
26-
053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00", -1)
26+
053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00")
2727

28-
var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00", -1)
28+
var singleCommitOutput = strings.ReplaceAll(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00")
2929

3030
func TestGetCommits(t *testing.T) {
3131
type scenario struct {

0 commit comments

Comments
 (0)