-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
go1.21: load rules: parse rules file: typechecker error: ...: could not import #449
Comments
Perhaps a newer version of x/tools should be used yet again. :) |
See #450 |
Sadly, the update of x/tools doesn't fix the problem. |
The rc4 is here, the GA is closer than ever. @quasilyte can I help you? |
go1.21 is available https://go.dev/blog/go1.21 |
parseGoEnv() in https://github.com/quasilyte/go-ruleguard/blob/master/internal/goenv/goenv.go has to handle both double-quoted and single-quoted formats now, something like: } else {
// Line format is: `$name="$value"` before go1.21
// Line format is: `$name='$value'` in go1.21+
for _, l := range lines {
parts := strings.Split(strings.TrimSpace(l), "=")
if len(parts) != 2 {
continue
}
val := parts[1]
switch {
case val == `""` || val == `''`:
val = ""
case len(val) > 2 && val[0] == '"' && val[len(val)-1] == '"':
// go < 1.21 before https://github.com/golang/go/commit/f379e78951a405e7e99a60fb231eeedbf976c108
var err error
val, err = strconv.Unquote(parts[1])
if err != nil {
continue
}
case len(val) > 2 && val[0] == '\'' && val[len(val)-1] == '\'':
// go >= 1.21 after https://github.com/golang/go/commit/f379e78951a405e7e99a60fb231eeedbf976c108
// trim leading/trailing single-quotes
val = val[1 : len(val)-1]
// replace internally escaped single-quotes
val = strings.ReplaceAll(val, `'\''`, `'`)
default:
val = ""
}
vars[parts[0]] = val
}
} |
AFAIK parseGoEnv is only used in the fallback importer, if everything else fails. |
not sure how it got there, but resolving this fixed go-critic/go-critic#1359 (comment) |
go team also suggested the following in https://go-review.googlesource.com/c/go/+/493535:
|
The link seems private. The public link is https://go-review.googlesource.com/c/go/+/488375 |
Looks like public git with extra steps http://go.dev/cl/488375 |
I try your suggestion (I will remove it after) inside my PR #451 |
Maybe you can open a PR. |
in fact, it works locally but not in the CI... |
I can reproduce the failure locally: $ make test
go test -timeout=10m -count=1 -race -coverpkg=./... -coverprofile=coverage.txt -covermode=atomic ./...
? d github.com/quasilyte/go-ruleguardt dsl[no test files]ATH_DIR internal/ lint README.md ruleguard/ rules.go _test/ test-master
ok github.com/quasilyte/go-ruleguard/analyzer 60.713s coverage: 75.4% of statements in ./...
? github.com/quasilyte/go-ruleguard/analyzer/testanalyzer [no test files]
? github.com/quasilyte/go-ruleguard/cmd/ruleguard [no test files]
? github.com/quasilyte/go-ruleguard/internal/golist [no test files]
? github.com/quasilyte/go-ruleguard/internal/xtypes [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/goutil [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/ir [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/irprint [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/profiling [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/internal/evaltest [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qfmt [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qstrconv [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qstrings [no test files]
ok github.com/quasilyte/go-ruleguard/internal/goenv 1.007s coverage: 67.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/internal/xsrcimporter 2.394s coverage: 100.0% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard 2.659s coverage: 41.4% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/irconv 1.368s coverage: 45.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/quasigo 13.643s coverage: 85.8% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/textmatch 1.018s coverage: 95.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/typematch 1.012s coverage: 66.2% of statements in ./...
go test -count=3 -run=TestE2E ./analyzer
ok github.com/quasilyte/go-ruleguard/analyzer 76.182s
cd rules && go test -v .
=== RUN TestRules
analysistest.go:295: error analyzing ruleguard@_/home/ldez/sources/golangci-lint/go-ruleguard/rules/testdata: load rules: parse rules file: typechecker error: diag.go:4:2: could not import github.com/quasilyte/go-ruleguard/dsl (can't find import: "github.com/quasilyte/go-ruleguard/dsl")
--- FAIL: TestRules (0.33s)
FAIL
FAIL github.com/quasilyte/go-ruleguard/rules 0.336s
FAIL
make: *** [Makefile:19: test] Error 1 |
opened #455 with the fix suggested by the go team |
🤔 it still fails: $ make test
go test -timeout=10m -count=1 -race -coverpkg=./... -coverprofile=coverage.txt -covermode=atomic ./...
? github.com/quasilyte/go-ruleguard [no test files]
ok github.com/quasilyte/go-ruleguard/analyzer 64.546s coverage: 75.6% of statements in ./...
? github.com/quasilyte/go-ruleguard/analyzer/testanalyzer [no test files]
? github.com/quasilyte/go-ruleguard/cmd/ruleguard [no test files]
? github.com/quasilyte/go-ruleguard/internal/golist [no test files]
? github.com/quasilyte/go-ruleguard/internal/xtypes [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/goutil [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/ir [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/irprint [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/profiling [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/internal/evaltest [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qfmt [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qstrconv [no test files]
? github.com/quasilyte/go-ruleguard/ruleguard/quasigo/stdlib/qstrings [no test files]
ok github.com/quasilyte/go-ruleguard/internal/goenv 1.008s coverage: 66.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/internal/xsrcimporter 2.433s coverage: 100.0% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard 2.790s coverage: 41.6% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/irconv 1.411s coverage: 45.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/quasigo 13.914s coverage: 85.8% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/textmatch 1.018s coverage: 95.7% of statements in ./...
ok github.com/quasilyte/go-ruleguard/ruleguard/typematch 1.015s coverage: 66.2% of statements in ./...
go test -count=3 -run=TestE2E ./analyzer
ok github.com/quasilyte/go-ruleguard/analyzer 73.428s
cd rules && go test -v .
=== RUN TestRules
analysistest.go:295: error analyzing ruleguard@_/home/ldez/sources/golangci-lint/go-ruleguard/rules/testdata: load rules: parse rules file: typechecker error: diag.go:4:2: could not import github.com/quasilyte/go-ruleguard/dsl (can't find import: "github.com/quasilyte/go-ruleguard/dsl")
--- FAIL: TestRules (0.34s)
FAIL
FAIL github.com/quasilyte/go-ruleguard/rules 0.343s
FAIL
make: *** [Makefile:19: test] Error 1 https://github.com/ldez/go-ruleguard/actions/runs/5814009305 |
hrmm... my fix resolved the issue I was seeing finding stdlib imports... the error you're seeing looks to be in resolving non-stdlib imports there may be more issues to resolve here, though |
Maybe someone can reopen this issue. |
I bisected Go and the first commit that fails is golang/go@f379e78
It's the commit related to the same CL https://go-review.googlesource.com/c/go/+/488375 |
I'm pretty sure it's the issue that's already fixed at HEAD, but we need to:
see comment at #457 (comment) |
see my PR #451 |
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually. One downside of updating golangci-lint to v1.54.0 without also updating logcheck is that golangci-lint now complains about logcheck using the old plugin API: ERROR: level=warning msg="plugin: 'AnalyzerPlugin' plugins are deprecated, please use the new plugin signature: https://golangci-lint.run/contributing/new-linters/#create-a-plugin"
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually.
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually.
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually.
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually. The new ginkgolinter finds some issues in tests in the release-1.27 branch that were fixed on master, but not backported. These issues don't need to be fixed in a release branch, therefore the ginkgolinter gets disabled.
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually.
That release is the first one with official support for Go 1.21. go-ruleguard must be >= 0.3.20 because of quasilyte/go-ruleguard#449 with Go 1.21. golangci-lint itself doesn't depend on a recent enough release yet, so this was done manually.
Hello,
I know it's a bit early but I started to work on go1.21 for golangci-lint.
golangci/golangci-lint#3922
And there is an issue with go-ruleguard.
I run the tests of go-ruleguard and there is the same problem:
Currently, I don't know the root cause but I want to share it with you, I think will you see the problem faster than me.
Maybe it's related to the new package initialization order, I don't know.
I already created an issue on go-critic.
go test
The text was updated successfully, but these errors were encountered: