From 4396e40a31de072f6390c6a50cb67ef2e09ba551 Mon Sep 17 00:00:00 2001 From: Mikel Olasagasti Uranga Date: Mon, 20 Jan 2025 16:27:25 +0100 Subject: [PATCH 1/3] Fix: Ensure constant format strings in fmt and printf calls Go 1.24 introduces stricter checks for format string validation. This commit fixes instances where non-constant format strings were used in calls to functions like `fmt.Errorf`, `fmt.Printf`, and similar. Signed-off-by: Mikel Olasagasti Uranga --- pkg/cmd/auth/token/token.go | 2 +- pkg/cmd/codespace/create_test.go | 6 +++--- pkg/cmd/pr/merge/merge.go | 7 ++++--- pkg/cmd/repo/view/view.go | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 9f4dcc1ac..7ac65d37e 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -88,7 +88,7 @@ func tokenRun(opts *TokenOptions) error { if opts.Username != "" { errMsg += fmt.Sprintf(" account %s", opts.Username) } - return fmt.Errorf(errMsg) + return fmt.Errorf("%s", errMsg) } if val != "" { diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 6466d9be7..85d311a47 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -648,14 +648,14 @@ Alternatively, you can run "create" with the "--default-permissions" option to c assert.EqualError(t, err, tt.wantErr.Error()) } if err != nil && tt.wantErr == nil { - t.Logf(err.Error()) + t.Logf("%s", err.Error()) } if got := stdout.String(); got != tt.wantStdout { - t.Logf(t.Name()) + t.Logf("%s", t.Name()) t.Errorf(" stdout = %v, want %v", got, tt.wantStdout) } if got := stderr.String(); got != tt.wantStderr { - t.Logf(t.Name()) + t.Logf("%s", t.Name()) t.Errorf(" stderr = %v, want %v", got, tt.wantStderr) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 6696fac85..422b5e93e 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -384,13 +384,14 @@ func (m *mergeContext) deleteLocalBranch() error { if m.merged { if m.opts.IO.CanPrompt() && !m.opts.IsDeleteBranchIndicated { - confirmed, err := m.opts.Prompter.Confirm(fmt.Sprintf("Pull request %s#%d was already merged. Delete the branch locally?", ghrepo.FullName(m.baseRepo), m.pr.Number), false) + message := fmt.Sprintf("Pull request %s#%d was already merged. Delete the branch locally?", ghrepo.FullName(m.baseRepo), m.pr.Number) + confirmed, err := m.opts.Prompter.Confirm(message, false) if err != nil { return fmt.Errorf("could not prompt: %w", err) } m.deleteBranch = confirmed } else { - _ = m.warnf(fmt.Sprintf("%s Pull request %s#%d was already merged\n", m.cs.WarningIcon(), ghrepo.FullName(m.baseRepo), m.pr.Number)) + _ = m.warnf("%s Pull request %s#%d was already merged\n", m.cs.WarningIcon(), ghrepo.FullName(m.baseRepo), m.pr.Number) } } @@ -431,7 +432,7 @@ func (m *mergeContext) deleteLocalBranch() error { } if err := m.opts.GitClient.Pull(ctx, baseRemote.Name, targetBranch); err != nil { - _ = m.warnf(fmt.Sprintf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch)) + _ = m.warnf("%s warning: not possible to fast-forward to: %q\n", m.cs.WarningIcon(), targetBranch) } switchedToBranch = targetBranch diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index d0370203a..f8cbcb18e 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -156,7 +156,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(stdout, "description:\t%s\n", repo.Description) if readme != nil { fmt.Fprintln(stdout, "--") - fmt.Fprintf(stdout, readme.Content) + fmt.Fprintf(stdout, "%s", readme.Content) fmt.Fprintln(stdout) } From 0ead3398a71663edf8e24dae836c957aaf2cc280 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 21 Jan 2025 17:29:39 +0100 Subject: [PATCH 2/3] Bump golang ci lint to work with go 1.24 --- .github/workflows/lint.yml | 4 ++-- pkg/cmd/ruleset/shared/shared.go | 2 +- pkg/cmdutil/json_flags.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0a6c0d9db..f1ae1e522 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -32,7 +32,7 @@ jobs: go mod verify go mod download - LINT_VERSION=1.59.1 + LINT_VERSION=1.63.4 curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \ tar xz --strip-components 1 --wildcards \*/golangci-lint mkdir -p bin && mv golangci-lint bin/ @@ -53,6 +53,6 @@ jobs: assert-nothing-changed go fmt ./... assert-nothing-changed go mod tidy - bin/golangci-lint run --out-format=github-actions --timeout=3m || STATUS=$? + bin/golangci-lint run --out-format=colored-line-number --timeout=3m || STATUS=$? exit $STATUS diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 3ec1c83ee..536e8a5a9 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -78,7 +78,7 @@ func ParseRulesForDisplay(rules []RulesetRule) string { for _, rule := range rules { display.WriteString(fmt.Sprintf("- %s", rule.Type)) - if rule.Parameters != nil && len(rule.Parameters) > 0 { + if len(rule.Parameters) > 0 { display.WriteString(": ") // sort these keys too for consistency diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 440e0e0b8..596c2f216 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -254,7 +254,7 @@ func (e *jsonExporter) exportData(v reflect.Value) interface{} { } return m.Interface() case reflect.Struct: - if v.CanAddr() && reflect.PtrTo(v.Type()).Implements(exportableType) { + if v.CanAddr() && reflect.PointerTo(v.Type()).Implements(exportableType) { ve := v.Addr().Interface().(exportable) return ve.ExportData(e.fields) } else if v.Type().Implements(exportableType) { From 0a3706a4042f5ef319b8a8e619cfafa741b8dc86 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 21 Jan 2025 18:04:47 +0100 Subject: [PATCH 3/3] Remove unncessary printf usage --- pkg/cmd/auth/token/token.go | 3 ++- pkg/cmd/codespace/create_test.go | 6 +++--- pkg/cmd/repo/view/view.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 7ac65d37e..d9faac7d8 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -1,6 +1,7 @@ package token import ( + "errors" "fmt" "github.com/MakeNowJust/heredoc" @@ -88,7 +89,7 @@ func tokenRun(opts *TokenOptions) error { if opts.Username != "" { errMsg += fmt.Sprintf(" account %s", opts.Username) } - return fmt.Errorf("%s", errMsg) + return errors.New(errMsg) } if val != "" { diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index 85d311a47..c5f11bd2f 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -648,14 +648,14 @@ Alternatively, you can run "create" with the "--default-permissions" option to c assert.EqualError(t, err, tt.wantErr.Error()) } if err != nil && tt.wantErr == nil { - t.Logf("%s", err.Error()) + t.Log(err.Error()) } if got := stdout.String(); got != tt.wantStdout { - t.Logf("%s", t.Name()) + t.Log(t.Name()) t.Errorf(" stdout = %v, want %v", got, tt.wantStdout) } if got := stderr.String(); got != tt.wantStderr { - t.Logf("%s", t.Name()) + t.Log(t.Name()) t.Errorf(" stderr = %v, want %v", got, tt.wantStderr) } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index f8cbcb18e..06f85d048 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -156,7 +156,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(stdout, "description:\t%s\n", repo.Description) if readme != nil { fmt.Fprintln(stdout, "--") - fmt.Fprintf(stdout, "%s", readme.Content) + fmt.Fprint(stdout, readme.Content) fmt.Fprintln(stdout) }