Skip to content

Conversation

alexandear
Copy link
Contributor

@alexandear alexandear commented Feb 12, 2025

This PR fixes the problem with building go-critic on Homebrew/homebrew-core#201070 with Go 1.24. Also, it fixes failing tests.

go-critic: test failure: Minitest::Assertion: Expected /sloppyLen:\ len(str)\ <=\ 0\ can\ be\ len(str)\ ==\ 0/ to match "internal error: package "fmt" without types was imported from "command-line-arguments"\n".

$ go version
go version go1.24.0 darwin/arm64
$ go test ./...
?       github.com/go-critic/go-critic  [no test files]
2025/02/12 14:55:19 internal error: package "bytes" without types was imported from "github.com/go-critic/go-critic/checkers/testdata/argOrder"
FAIL    github.com/go-critic/go-critic/checkers 4.654s
?       github.com/go-critic/go-critic/checkers/analyzer        [no test files]
?       github.com/go-critic/go-critic/checkers/internal/astwalk        [no test files]
?       github.com/go-critic/go-critic/checkers/internal/linttest       [no test files]
?       github.com/go-critic/go-critic/checkers/internal/lintutil       [no test files]
?       github.com/go-critic/go-critic/checkers/rules   [no test files]
?       github.com/go-critic/go-critic/checkers/rulesdata       [no test files]
ok      github.com/go-critic/go-critic/cmd/gocritic     (cached)
?       github.com/go-critic/go-critic/cmd/gocritic-analysis    [no test files]
?       github.com/go-critic/go-critic/cmd/makedocs     [no test files]
ok      github.com/go-critic/go-critic/linter   (cached)
ok      github.com/go-critic/go-critic/rulestest        1.303s
FAIL

This update is done by hand because @dependabot can't update the Go version in go.mod.

Diff: golang/tools@v0.23.0...v0.30.0

Closes #1436

@alexandear
Copy link
Contributor Author

Hi @cristaloleg,

Could you please review this PR at your earliest convenience? If everything looks good, would you mind making a new release after merging?

Thank you!

@alexandear alexandear changed the title deps: bump golang.org/x/tools from 0.23.0 to 0.30.0 deps: bump x/tools to 0.30.0 to support Go 1.24 Feb 12, 2025
@cristaloleg
Copy link
Member

I assume that you've tested this locally and compiling go-critic with Go 1.24 works fine, am I right?

@alexandear
Copy link
Contributor Author

I assume that you've tested this locally and compiling go-critic with Go 1.24 works fine, am I right?

Yep, I tested on the newest golangci-lint codebase.

Before (error):

❯ git switch master
Already on 'master'
Your branch is up to date with 'origin/master'.

❯ go version
go version go1.24.0 darwin/arm64

❯ make build-release
mkdir -p bin
go build -o bin/gocritic -ldflags "-X 'main.Version='" ./cmd/gocritic

❯ cd ~/src/github.com/golangci/golangci-lint

❯ ~/src/github.com/go-critic/go-critic/bin/gocritic check ./...
internal error: package "errors" without types was imported from "github.com/golangci/golangci-lint/internal/go/robustio"

After (warnings, as expected):

...
❯ git switch bump-x-tools-0.30.0
Switched to branch 'bump-x-tools-0.30.0'
Your branch is up to date with 'fork/bump-x-tools-0.30.0'.

❯ ~/src/github.com/go-critic/go-critic/bin/gocritic check ./...
./internal/x/tools/diff/unified.go:132:4: commentFormatting: put a space between `//` and comment text
./internal/x/tools/diff/unified.go:134:4: commentFormatting: put a space between `//` and comment text
./internal/x/tools/diff/unified.go:137:4: commentFormatting: put a space between `//` and comment text
./internal/x/tools/diff/unified.go:219:3: ifElseChain: rewrite if-else to switch statement
./internal/x/tools/diff/unified.go:227:3: ifElseChain: rewrite if-else to switch statement
./internal/x/tools/diff/lcs/labels.go:22:14: captLocal: `D' should not be capitalized
./internal/x/tools/diff/lcs/labels.go:29:21: captLocal: `D' should not be capitalized
./internal/x/tools/diff/lcs/old.go:91:27: captLocal: `D' should not be capitalized
./internal/x/tools/diff/lcs/old.go:147:32: captLocal: `D' should not be capitalized
./internal/x/tools/diff/lcs/old.go:191:27: captLocal: `D' should not be capitalized
./internal/x/tools/diff/lcs/old.go:251:33: captLocal: `D' should not be capitalized
./pkg/golinters/internal/staticcheck_common.go:106:12: elseif: can replace 'else {if cond {}}' with 'else if cond {}'
./pkg/golinters/internal/staticcheck_common.go:88:3: ifElseChain: rewrite if-else to switch statement
./pkg/golinters/internal/staticcheck_common.go:96:39: unlambda: replace `func(r rune) bool { return unicode.IsNumber(r) }` with `unicode.IsNumber`
./pkg/golinters/internal/staticcheck_common.go:99:33: unlambda: replace `func(r rune) bool { return unicode.IsNumber(r) }` with `unicode.IsNumber`

@cristaloleg cristaloleg merged commit 96ffa98 into go-critic:master Feb 12, 2025
2 checks passed
@cristaloleg
Copy link
Member

Thanks @alexandear v0.12.0 landed

@alexandear alexandear deleted the bump-x-tools-0.30.0 branch February 12, 2025 13:29
@alexandear
Copy link
Contributor Author

@cristaloleg, thank you for the prompt response and for reviewing the PR.

@marcelloh
Copy link

confirmed: it works again :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants