Skip to content

Commit 593cfd4

Browse files
authored
chore(lint): enable additional golangci-lint rules and fix all findings (#46)
Enable 10 new linters in `.golangci.yml`: `copyloopvar`, `dupword`, `errname`, `errorlint`, `gocritic`, `intrange`, `modernize`, `nilerr`, `perfsprint`, and `unconvert`. Fixes across 19 files: - Use `errors.Is`/`errors.As` instead of `==`/type assertions on errors - Use `errors.New` instead of `fmt.Errorf` for static error strings - Use `any` instead of `interface{}`, `strings.SplitSeq` instead of `strings.Split`, `slices.Contains` instead of manual loop - Convert if-else chains to switch statements - Use integer range loops (`for i := range n`) - Rename `gitPathErr` to `errGitPath` per sentinel error convention - Suppress `nilerr` for intentional patterns (git exit codes as booleans)
1 parent 4b907b1 commit 593cfd4

File tree

21 files changed

+89
-69
lines changed

21 files changed

+89
-69
lines changed

.golangci.yml

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,22 @@ run:
55

66
linters:
77
enable:
8+
- copyloopvar
9+
- dupword
810
- errcheck
11+
- errname
12+
- errorlint
13+
- gocritic
914
- govet
10-
- staticcheck
11-
- unused
1215
- ineffassign
16+
- intrange
1317
- misspell
18+
- modernize
19+
- nilerr
20+
- perfsprint
21+
- staticcheck
22+
- unconvert
23+
- unused
1424
# gosec disabled - false positives for CLI tools that shell out to git
1525
settings:
1626
errcheck:

cmd/abort.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78

@@ -35,7 +36,7 @@ func runAbort(cmd *cobra.Command, args []string) error {
3536
// Check if cascade in progress
3637
st, err := state.Load(g.GetGitDir())
3738
if err != nil {
38-
return fmt.Errorf("no operation in progress")
39+
return errors.New("no operation in progress")
3940
}
4041

4142
// Abort rebase if in progress

cmd/adopt.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78

@@ -88,7 +89,7 @@ func runAdopt(cmd *cobra.Command, args []string) error {
8889
if parentNode != nil {
8990
for _, ancestor := range tree.GetAncestors(parentNode) {
9091
if ancestor.Name == branchName {
91-
return fmt.Errorf("cannot adopt: would create a cycle")
92+
return errors.New("cannot adopt: would create a cycle")
9293
}
9394
}
9495
}

cmd/cascade.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func runCascade(cmd *cobra.Command, args []string) error {
5454

5555
// Check if cascade already in progress
5656
if state.Exists(g.GetGitDir()) {
57-
return fmt.Errorf("operation already in progress; use 'gh stack continue' or 'gh stack abort'")
57+
return errors.New("operation already in progress; use 'gh stack continue' or 'gh stack abort'")
5858
}
5959

6060
currentBranch, err := g.CurrentBranch()
@@ -94,7 +94,7 @@ func runCascade(cmd *cobra.Command, args []string) error {
9494
err = doCascadeWithState(g, cfg, branches, cascadeDryRunFlag, state.OperationCascade, false, false, false, nil, stashRef, s)
9595

9696
// Restore auto-stashed changes after operation (unless conflict, which saves stash in state)
97-
if stashRef != "" && err != ErrConflict {
97+
if stashRef != "" && !errors.Is(err, ErrConflict) {
9898
fmt.Println("Restoring auto-stashed changes...")
9999
if popErr := g.StashPop(stashRef); popErr != nil {
100100
fmt.Printf("%s could not restore stashed changes (commit %s): %v\n", s.WarningIcon(), git.AbbrevSHA(stashRef), popErr)

cmd/continue.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78

@@ -37,14 +38,14 @@ func runContinue(cmd *cobra.Command, args []string) error {
3738
// Check if operation in progress
3839
st, err := state.Load(g.GetGitDir())
3940
if err != nil {
40-
return fmt.Errorf("no operation in progress")
41+
return errors.New("no operation in progress")
4142
}
4243

4344
// Complete the in-progress rebase
4445
if g.IsRebaseInProgress() {
4546
fmt.Println("Continuing rebase...")
4647
if rebaseErr := g.RebaseContinue(); rebaseErr != nil {
47-
return fmt.Errorf("rebase --continue failed; resolve conflicts first")
48+
return errors.New("rebase --continue failed; resolve conflicts first")
4849
}
4950
}
5051

@@ -75,7 +76,7 @@ func runContinue(cmd *cobra.Command, args []string) error {
7576

7677
if cascadeErr := doCascadeWithState(g, cfg, branches, false, st.Operation, st.UpdateOnly, st.Web, st.PushOnly, st.Branches, st.StashRef, s); cascadeErr != nil {
7778
// Stash handling is done by doCascadeWithState (conflict saves in state, errors restore)
78-
if cascadeErr != ErrConflict && st.StashRef != "" {
79+
if !errors.Is(cascadeErr, ErrConflict) && st.StashRef != "" {
7980
fmt.Println("Restoring auto-stashed changes...")
8081
if popErr := g.StashPop(st.StashRef); popErr != nil {
8182
fmt.Printf("%s could not restore stashed changes (commit %s): %v\n", s.WarningIcon(), git.AbbrevSHA(st.StashRef), popErr)

cmd/create.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78

@@ -76,7 +77,7 @@ func runCreate(cmd *cobra.Command, args []string) error {
7677

7778
if hasStaged && !createEmptyFlag {
7879
if createMessageFlag == "" {
79-
return fmt.Errorf("staged changes found but no commit message provided; use -m or --empty")
80+
return errors.New("staged changes found but no commit message provided; use -m or --empty")
8081
}
8182
}
8283

cmd/init.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78

@@ -42,12 +43,13 @@ func runInit(cmd *cobra.Command, args []string) error {
4243
trunk := trunkFlag
4344
if trunk == "" {
4445
// Try main, then master
45-
if g.BranchExists("main") {
46+
switch {
47+
case g.BranchExists("main"):
4648
trunk = "main"
47-
} else if g.BranchExists("master") {
49+
case g.BranchExists("master"):
4850
trunk = "master"
49-
} else {
50-
return fmt.Errorf("could not determine trunk branch; use --trunk to specify")
51+
default:
52+
return errors.New("could not determine trunk branch; use --trunk to specify")
5153
}
5254
}
5355

cmd/log_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ func TestLogMultipleBranches(t *testing.T) {
9898

9999
if featureA == nil {
100100
t.Fatal("feature-a not found")
101+
return
101102
}
102103

103104
if len(featureA.Children) != 1 {

cmd/submit.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
78
"regexp"
@@ -63,10 +64,10 @@ func runSubmit(cmd *cobra.Command, args []string) error {
6364

6465
// Validate flag combinations
6566
if submitPushOnlyFlag && submitUpdateOnlyFlag {
66-
return fmt.Errorf("--push-only and --update-only cannot be used together: --push-only skips all PR operations")
67+
return errors.New("--push-only and --update-only cannot be used together: --push-only skips all PR operations")
6768
}
6869
if submitPushOnlyFlag && submitWebFlag {
69-
return fmt.Errorf("--push-only and --web cannot be used together: --push-only skips all PR operations")
70+
return errors.New("--push-only and --web cannot be used together: --push-only skips all PR operations")
7071
}
7172
if submitFromFlag != "" && submitCurrentOnlyFlag {
7273
return fmt.Errorf("--from and --current-only cannot be used together")
@@ -86,7 +87,7 @@ func runSubmit(cmd *cobra.Command, args []string) error {
8687

8788
// Check if operation already in progress
8889
if state.Exists(g.GetGitDir()) {
89-
return fmt.Errorf("operation already in progress; use 'gh stack continue' or 'gh stack abort'")
90+
return errors.New("operation already in progress; use 'gh stack continue' or 'gh stack abort'")
9091
}
9192

9293
currentBranch, err := g.CurrentBranch()
@@ -174,7 +175,7 @@ func runSubmit(cmd *cobra.Command, args []string) error {
174175
fmt.Println(s.Bold("=== Phase 1: Restack ==="))
175176
if cascadeErr := doCascadeWithState(g, cfg, branches, submitDryRunFlag, state.OperationSubmit, submitUpdateOnlyFlag, submitWebFlag, submitPushOnlyFlag, branchNames, stashRef, s); cascadeErr != nil {
176177
// Stash is saved in state for conflicts; restore on other errors
177-
if cascadeErr != ErrConflict && stashRef != "" {
178+
if !errors.Is(cascadeErr, ErrConflict) && stashRef != "" {
178179
fmt.Println("Restoring auto-stashed changes...")
179180
if popErr := g.StashPop(stashRef); popErr != nil {
180181
fmt.Printf("%s could not restore stashed changes (commit %s): %v\n", s.WarningIcon(), git.AbbrevSHA(stashRef), popErr)
@@ -266,7 +267,8 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
266267

267268
existingPR, _ := cfg.GetPR(b.Name) //nolint:errcheck // 0 is fine
268269

269-
if existingPR > 0 {
270+
switch {
271+
case existingPR > 0:
270272
// Update existing PR
271273
if dryRun {
272274
fmt.Printf("%s Would update PR #%d base to %s\n", s.Muted("dry-run:"), existingPR, s.Branch(parent))
@@ -289,27 +291,28 @@ func doSubmitPRs(g *git.Git, cfg *config.Config, root *tree.Node, branches []*tr
289291
// If PR is a draft and now targets trunk, offer to publish
290292
maybeMarkPRReady(ghClient, existingPR, b.Name, parent, trunk, s)
291293
}
292-
} else if !updateOnly {
294+
case !updateOnly:
293295
// Create new PR
294296
if dryRun {
295297
fmt.Printf("%s Would create PR for %s (base: %s)\n", s.Muted("dry-run:"), s.Branch(b.Name), s.Branch(parent))
296298
} else {
297299
prNum, adopted, err := createPRForBranch(g, ghClient, cfg, root, b.Name, parent, trunk, remoteBranches, s)
298-
if err != nil {
300+
switch {
301+
case err != nil:
299302
fmt.Printf("%s failed to create PR for %s: %v\n", s.WarningIcon(), s.Branch(b.Name), err)
300-
} else if adopted {
303+
case adopted:
301304
fmt.Printf("%s Adopted PR #%d for %s (%s)\n", s.SuccessIcon(), prNum, s.Branch(b.Name), ghClient.PRURL(prNum))
302305
if openWeb {
303306
prURLs = append(prURLs, ghClient.PRURL(prNum))
304307
}
305-
} else {
308+
default:
306309
fmt.Printf("%s Created PR #%d for %s (%s)\n", s.SuccessIcon(), prNum, s.Branch(b.Name), ghClient.PRURL(prNum))
307310
if openWeb {
308311
prURLs = append(prURLs, ghClient.PRURL(prNum))
309312
}
310313
}
311314
}
312-
} else {
315+
default:
313316
fmt.Printf("Skipping %s %s\n", s.Branch(b.Name), s.Muted("(no existing PR, --update-only)"))
314317
}
315318
}
@@ -447,7 +450,7 @@ func promptForPRDetails(branch, defaultTitle, defaultBody string, s *style.Style
447450
}
448451
title = strings.TrimSpace(title)
449452
if title == "" {
450-
return "", "", fmt.Errorf("PR title cannot be empty")
453+
return "", "", errors.New("PR title cannot be empty")
451454
}
452455

453456
// Show the generated body and ask if user wants to edit

cmd/sync.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
package cmd
33

44
import (
5+
"errors"
56
"fmt"
67
"os"
8+
"slices"
79

810
"github.com/boneskull/gh-stack/internal/config"
911
"github.com/boneskull/gh-stack/internal/git"
@@ -184,7 +186,7 @@ func runSync(cmd *cobra.Command, args []string) error {
184186
// Uses git diff to detect when a branch's content is already in trunk
185187
for _, branch := range branches {
186188
// Skip already detected via PR
187-
if sliceContains(merged, branch) {
189+
if slices.Contains(merged, branch) {
188190
continue
189191
}
190192

@@ -215,7 +217,7 @@ func runSync(cmd *cobra.Command, args []string) error {
215217
}
216218

217219
// Skip if parent is already marked as merged (will be handled)
218-
if sliceContains(merged, parent) {
220+
if slices.Contains(merged, parent) {
219221
continue
220222
}
221223

@@ -351,7 +353,7 @@ func runSync(cmd *cobra.Command, args []string) error {
351353
allBranches := []*tree.Node{child}
352354
allBranches = append(allBranches, tree.GetDescendants(child)...)
353355
if err := doCascadeWithState(g, cfg, allBranches, syncDryRunFlag, state.OperationCascade, false, false, false, nil, stashRef, s); err != nil {
354-
if err == ErrConflict {
356+
if errors.Is(err, ErrConflict) {
355357
hitConflict = true
356358
}
357359
return err
@@ -373,16 +375,6 @@ func runSync(cmd *cobra.Command, args []string) error {
373375
return nil
374376
}
375377

376-
// sliceContains returns true if slice contains the given string.
377-
func sliceContains(slice []string, s string) bool {
378-
for _, item := range slice {
379-
if item == s {
380-
return true
381-
}
382-
}
383-
return false
384-
}
385-
386378
// mergedAction represents the user's choice for handling a merged branch.
387379
type mergedAction int
388380

0 commit comments

Comments
 (0)