Skip to content

Commit

Permalink
[Internal] Bump staticcheck to 0.5.1 and add go 1.23 test coverage (#…
Browse files Browse the repository at this point in the history
…1106)

## What changes are proposed in this pull request?
This PR updates the version of staticcheck used to the latest version
(0.5.1). This is needed for compatibility with recent versions of go.
When running the current version (0.4.0) using go 1.23.4, I get the
following error:

```
panic: Cannot range over: func(yield func(K, V) bool)

goroutine 323 [running]:
honnef.co/go/tools/go/ir.(*builder).rangeStmt(0x140084a1a30, 0x14008dcd900, 0x140088cdb00, 0x0, {0x100aa3ef0, 0x140088cdb00})
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2181 +0x6dc
honnef.co/go/tools/go/ir.(*builder).stmt(0x140084a1a30, 0x14008dcd900, {0x100aa62d8?, 0x140088cdb00?})
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2394 +0x1c0
honnef.co/go/tools/go/ir.(*builder).stmtList(...)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:847
honnef.co/go/tools/go/ir.(*builder).stmt(0x140084a1a30, 0x14008dcd900, {0x100aa5f18?, 0x14003654e10?})
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2352 +0x10c4
honnef.co/go/tools/go/ir.(*builder).buildFunction(0x140084a1a30, 0x14008dcd900)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2464 +0x368
honnef.co/go/tools/go/ir.(*builder).buildFuncDecl(0x140084a1a30, 0x140006af5f0, 0x14003654e70)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2501 +0x1a4
honnef.co/go/tools/go/ir.(*Package).build(0x140006af5f0)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2605 +0x934
sync.(*Once).doSlow(0x140084de0e0?, 0x140088cd6e0?)
        /opt/homebrew/Cellar/go/1.23.4/libexec/src/sync/once.go:76 +0xf8
sync.(*Once).Do(...)
        /opt/homebrew/Cellar/go/1.23.4/libexec/src/sync/once.go:67
honnef.co/go/tools/go/ir.(*Package).Build(...)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/go/ir/builder.go:2523
honnef.co/go/tools/internal/passes/buildir.run(0x14003876e10)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/internal/passes/buildir/buildir.go:86 +0x134
honnef.co/go/tools/lintcmd/runner.(*analyzerRunner).do(0x1400387b920, {0x100aa8998?, 0x140086ef860})
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/lintcmd/runner/runner.go:992 +0x600
honnef.co/go/tools/lintcmd/runner.genericHandle({0x100aa8998, 0x140086ef860}, {0x100aa8998?, 0x140086ef7c0?}, 0x14008dc83f0, 0x140004829b0, 0x140072dfa30)
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/lintcmd/runner/runner.go:817 +0x10c
created by honnef.co/go/tools/lintcmd/runner.(*subrunner).runAnalyzers in goroutine 321
        /Users/miles/go/pkg/mod/honnef.co/go/tools@v0.4.0/lintcmd/runner/runner.go:1061 +0x568
exit status 2
```

Additionally, some things that passed linting before are no longer
allowed. These have been fixed without affecting the behavior.

Note that the one version of staticcheck does not work with every go
version, so I changed the dependency for tests from `lint` to `vendor`
so that linting is not run on every go version that we test on.

Finally, I add unit test coverage for 1.23 and run linting and
formatting with this golang version.

## How is this tested?

No additional tests, but staticcheck is still enforced as part of CI.
  • Loading branch information
mgyucht authored Jan 3, 2025
1 parent 2399d72 commit 28e1a69
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 25 deletions.
27 changes: 17 additions & 10 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
- "1.20"
- "1.21"
- "1.22"
- "1.23"
steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -34,14 +35,6 @@ jobs:
with:
go-version: ${{ matrix.goVersion }}

- name: Set go env
run: |
echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV
echo "$(go env GOPATH)/bin" >> $GITHUB_PATH
- name: Pull external libraries
run: make vendor

- name: Run tests
run: make test

Expand All @@ -50,6 +43,21 @@ jobs:
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

lint:
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.23"

- name: Run linters
run: make lint

fmt:
runs-on: ubuntu-latest

Expand All @@ -60,7 +68,7 @@ jobs:
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.22"
go-version: "1.23"

# No need to download cached dependencies when running gofmt.
cache: false
Expand All @@ -75,4 +83,3 @@ jobs:
run: |
# Exit with status code 1 if there are differences (i.e. unformatted files)
git diff --exit-code
10 changes: 2 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ fmt:

lint: vendor
@echo "✓ Linting source code with https://staticcheck.io/ ..."
@go run honnef.co/go/tools/cmd/staticcheck@v0.4.0 ./...
@go run honnef.co/go/tools/cmd/staticcheck@v0.5.1 ./...

test: lint
test: vendor
@echo "✓ Running tests ..."
@go run gotest.tools/gotestsum@latest --format pkgname-and-test-fails \
--no-summary=skipped --raw-command go test -v \
Expand All @@ -32,10 +32,4 @@ doc:
@echo "Open http://localhost:6060"
@go run golang.org/x/tools/cmd/godoc@latest -http=localhost:6060

install-codegen: vendor
@go build -o ~/go/bin/oac openapi/gen/main.go

gen:
@go run openapi/gen/main.go

.PHONY: fmt vendor fmt coverage test lint doc
2 changes: 1 addition & 1 deletion httpclient/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestMakeRequestBodyJsonError(t *testing.T) {
type failingUrlEncode string

func (fue failingUrlEncode) EncodeValues(key string, v *url.Values) error {
return fmt.Errorf(string(fue))
return fmt.Errorf("%s", string(fue))
}

func TestMakeRequestBodyQueryFailingEncode(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion retries/retries.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Continue(err error) *Err {
}

func Continues(msg string) *Err {
return Continue(fmt.Errorf(msg))
return Continue(fmt.Errorf("%s", msg))
}

func Continuef(format string, err error, args ...interface{}) *Err {
Expand Down
4 changes: 2 additions & 2 deletions service/compute/ext_results.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package compute

import (
"fmt"
"errors"
"html"
"regexp"
"strings"
Expand Down Expand Up @@ -38,7 +38,7 @@ func (r *Results) Err() error {
if !r.Failed() {
return nil
}
return fmt.Errorf(r.Error())
return errors.New(r.Error())
}

// Error returns error in a bit more friendly way
Expand Down
2 changes: 1 addition & 1 deletion service/jobs/ext_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func (a *JobsAPI) GetRun(ctx context.Context, request GetRunRequest) (*Run, erro

// When querying a Job run, a page token is returned when there are more than 100 tasks. No iterations are defined for a Job run. Therefore, the next page in the response only includes the next page of tasks.
// When querying a ForEach task run, a page token is returned when there are more than 100 iterations. Only a single task is returned, corresponding to the ForEach task itself. Therefore, the client only reads the iterations from the next page and not the tasks.
isPaginatingIterations := run.Iterations != nil && len(run.Iterations) > 0
isPaginatingIterations := len(run.Iterations) > 0

pageToken := run.NextPageToken
for pageToken != "" {
Expand Down
4 changes: 2 additions & 2 deletions service/sql/ext_utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (a *StatementExecutionAPI) ExecuteAndWait(ctx context.Context, request Exec
if status.Error != nil {
msg = fmt.Sprintf("%s: %s %s", msg, status.Error.ErrorCode, status.Error.Message)
}
return nil, fmt.Errorf(msg)
return nil, fmt.Errorf("%s", msg)
default:
// TODO: parse request.WaitTimeout and use it here
return retries.Poll[StatementResponse](ctx, 20*time.Minute,
Expand All @@ -50,7 +50,7 @@ func (a *StatementExecutionAPI) ExecuteAndWait(ctx context.Context, request Exec
if status.Error != nil {
msg = fmt.Sprintf("%s: %s %s", msg, status.Error.ErrorCode, status.Error.Message)
}
return nil, retries.Halt(fmt.Errorf(msg))
return nil, retries.Halt(fmt.Errorf("%s", msg))
default:
return nil, retries.Continues(status.State.String())
}
Expand Down

0 comments on commit 28e1a69

Please sign in to comment.