From 9d033bc16c48429cdb21546f50dc8e203eba08c9 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 15 Jan 2025 15:58:04 +0000 Subject: [PATCH 1/5] Go: Only use `EmitInvalidToolchainVersion` if installed toolchain is `>=1.21 && <1.23` --- go/extractor/project/project.go | 20 ++++++++++++++++---- go/extractor/toolchain/toolchain.go | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index c870e6a5458a..5bed5786059b 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -195,11 +195,23 @@ func findGoModFiles(root string) []string { // A regular expression for the Go toolchain version syntax. var toolchainVersionRe *regexp.Regexp = regexp.MustCompile(`(?m)^([0-9]+\.[0-9]+\.[0-9]+)$`) -// Returns true if the `go.mod` file specifies a Go language version, that version is `1.21` or greater, and -// there is no `toolchain` directive, and the Go language version is not a valid toolchain version. +// Returns true if the `go.mod` file specifies a Go language version which is not of the format that +// is expected by the Go 1.21 and Go 1.22 toolchains for toolchain versions, and there is no +// explicit toolchain version declared. func hasInvalidToolchainVersion(modFile *modfile.File) bool { - return modFile.Toolchain == nil && modFile.Go != nil && - !toolchainVersionRe.Match([]byte(modFile.Go.Version)) && util.NewSemVer(modFile.Go.Version).IsAtLeast(toolchain.V1_21) + if modFile.Toolchain != nil { + // There is an explicit toolchain directive, so it doesn't matter what format the + // Go language version is in, since it will not be used as a fallback toolchain version. + return false + } else if modFile.Go != nil && !toolchainVersionRe.Match([]byte(modFile.Go.Version)) { + // There's no explicit toolchain directive, but we have a language version which + // does not match the toolchain version format in Go 1.21 and Go 1.22. + // This is a problem if the installed Go toolchain is within that version range + // as it will try to use the language version as the toolchain version. + installed := util.NewSemVer(toolchain.GetEnvGoVersion()) + return installed.IsAtLeast(toolchain.V1_21) && installed.IsOlderThan(toolchain.V1_23) + } + return false } // Given a list of `go.mod` file paths, try to parse them all. The resulting array of `GoModule` objects diff --git a/go/extractor/toolchain/toolchain.go b/go/extractor/toolchain/toolchain.go index 119e3782f6f6..123694c4c586 100644 --- a/go/extractor/toolchain/toolchain.go +++ b/go/extractor/toolchain/toolchain.go @@ -17,6 +17,7 @@ var V1_14 = util.NewSemVer("v1.14.0") var V1_16 = util.NewSemVer("v1.16.0") var V1_18 = util.NewSemVer("v1.18.0") var V1_21 = util.NewSemVer("v1.21.0") +var V1_23 = util.NewSemVer("v1.23.0") // Check if Go is installed in the environment. func IsInstalled() bool { From 97832b95bc6303156c80e03aece847ec4e67708d Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 15 Jan 2025 16:15:09 +0000 Subject: [PATCH 2/5] Go: Remove `invalid-go-toolchain-version` from test expectations This is reasonable because the tests use a new enough toolchain --- .../diagnostics.expected | 17 ----------------- .../go-version-bump/diagnostics.expected | 17 ----------------- .../newer-go-needed/diagnostics.expected | 17 ----------------- 3 files changed, 51 deletions(-) diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected index 2e5d732e53e7..56d774b7037c 100644 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected +++ b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected @@ -1,20 +1,3 @@ -{ - "location": { - "file": "go.mod" - }, - "markdownMessage": "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).\n\n`1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", - "severity": "warning", - "source": { - "extractorName": "go", - "id": "go/autobuilder/invalid-go-toolchain-version", - "name": "Invalid Go toolchain version" - }, - "visibility": { - "cliSummaryTable": true, - "statusPage": true, - "telemetry": true - } -} { "markdownMessage": "A single `go.mod` file was found.\n\n`go.mod`", "severity": "note", diff --git a/go/ql/integration-tests/go-version-bump/diagnostics.expected b/go/ql/integration-tests/go-version-bump/diagnostics.expected index 2e5d732e53e7..56d774b7037c 100644 --- a/go/ql/integration-tests/go-version-bump/diagnostics.expected +++ b/go/ql/integration-tests/go-version-bump/diagnostics.expected @@ -1,20 +1,3 @@ -{ - "location": { - "file": "go.mod" - }, - "markdownMessage": "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).\n\n`1.21` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", - "severity": "warning", - "source": { - "extractorName": "go", - "id": "go/autobuilder/invalid-go-toolchain-version", - "name": "Invalid Go toolchain version" - }, - "visibility": { - "cliSummaryTable": true, - "statusPage": true, - "telemetry": true - } -} { "markdownMessage": "A single `go.mod` file was found.\n\n`go.mod`", "severity": "note", diff --git a/go/ql/integration-tests/resolve-build-environment/newer-go-needed/diagnostics.expected b/go/ql/integration-tests/resolve-build-environment/newer-go-needed/diagnostics.expected index b64c1397938d..56d774b7037c 100644 --- a/go/ql/integration-tests/resolve-build-environment/newer-go-needed/diagnostics.expected +++ b/go/ql/integration-tests/resolve-build-environment/newer-go-needed/diagnostics.expected @@ -1,20 +1,3 @@ -{ - "location": { - "file": "go.mod" - }, - "markdownMessage": "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).\n\n`1.22` in `go.mod` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", - "severity": "warning", - "source": { - "extractorName": "go", - "id": "go/autobuilder/invalid-go-toolchain-version", - "name": "Invalid Go toolchain version" - }, - "visibility": { - "cliSummaryTable": true, - "statusPage": true, - "telemetry": true - } -} { "markdownMessage": "A single `go.mod` file was found.\n\n`go.mod`", "severity": "note", From 2f4157cb8a3ee307a0e2c71d261c17f12d6d9ef2 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 15 Jan 2025 17:20:10 +0000 Subject: [PATCH 3/5] Go: Make installed toolchain version a parameter of `hasInvalidToolchainVersion` --- go/extractor/project/project.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 5bed5786059b..7fc5f9a7e225 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -198,7 +198,7 @@ var toolchainVersionRe *regexp.Regexp = regexp.MustCompile(`(?m)^([0-9]+\.[0-9]+ // Returns true if the `go.mod` file specifies a Go language version which is not of the format that // is expected by the Go 1.21 and Go 1.22 toolchains for toolchain versions, and there is no // explicit toolchain version declared. -func hasInvalidToolchainVersion(modFile *modfile.File) bool { +func hasInvalidToolchainVersion(installedToolchainVersion util.SemVer, modFile *modfile.File) bool { if modFile.Toolchain != nil { // There is an explicit toolchain directive, so it doesn't matter what format the // Go language version is in, since it will not be used as a fallback toolchain version. @@ -208,8 +208,7 @@ func hasInvalidToolchainVersion(modFile *modfile.File) bool { // does not match the toolchain version format in Go 1.21 and Go 1.22. // This is a problem if the installed Go toolchain is within that version range // as it will try to use the language version as the toolchain version. - installed := util.NewSemVer(toolchain.GetEnvGoVersion()) - return installed.IsAtLeast(toolchain.V1_21) && installed.IsOlderThan(toolchain.V1_23) + return installedToolchainVersion.IsAtLeast(toolchain.V1_21) && installedToolchainVersion.IsOlderThan(toolchain.V1_23) } return false } @@ -244,7 +243,7 @@ func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { // there is no `toolchain` directive, check that it is a valid Go toolchain version. Otherwise, // `go` commands which try to download the right version of the Go toolchain will fail. We detect // this situation and emit a diagnostic. - if hasInvalidToolchainVersion(modFile) { + if hasInvalidToolchainVersion(util.NewSemVer(toolchain.GetEnvGoVersion()), modFile) { diagnostics.EmitInvalidToolchainVersion(goModFilePath, modFile.Go.Version) } } From 24c72392f967273666d6107474b80648dfe0a2c4 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 15 Jan 2025 17:27:35 +0000 Subject: [PATCH 4/5] Go: Restore check for whether the version is `>= 1.21` to begin with --- go/extractor/project/project.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 7fc5f9a7e225..441546542e92 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -208,7 +208,9 @@ func hasInvalidToolchainVersion(installedToolchainVersion util.SemVer, modFile * // does not match the toolchain version format in Go 1.21 and Go 1.22. // This is a problem if the installed Go toolchain is within that version range // as it will try to use the language version as the toolchain version. - return installedToolchainVersion.IsAtLeast(toolchain.V1_21) && installedToolchainVersion.IsOlderThan(toolchain.V1_23) + return util.NewSemVer(modFile.Go.Version).IsAtLeast(toolchain.V1_21) && + installedToolchainVersion.IsAtLeast(toolchain.V1_21) && + installedToolchainVersion.IsOlderThan(toolchain.V1_23) } return false } From 0d9a0dd522d0ffa2f2dc4c90789e2a8d812853af Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 15 Jan 2025 17:27:54 +0000 Subject: [PATCH 5/5] Go: Update unit tests for `hasInvalidToolchainVersion` to assume installed version is `1.21` --- go/extractor/project/project_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/go/extractor/project/project_test.go b/go/extractor/project/project_test.go index 149a9723ec29..fe17ccb7535e 100644 --- a/go/extractor/project/project_test.go +++ b/go/extractor/project/project_test.go @@ -39,18 +39,20 @@ func parseModFile(t *testing.T, contents string) *modfile.File { return modFile } -func testHasInvalidToolchainVersion(t *testing.T, contents string) bool { - return hasInvalidToolchainVersion(parseModFile(t, contents)) +func testHasInvalidToolchainVersion(t *testing.T, installedToolchainVersion util.SemVer, contents string) bool { + return hasInvalidToolchainVersion(installedToolchainVersion, parseModFile(t, contents)) } func TestHasInvalidToolchainVersion(t *testing.T) { + installedToolchainVersion := util.NewSemVer("1.21") + invalid := []string{ "go 1.21\n", "go 1.22\n", } for _, v := range invalid { - if !testHasInvalidToolchainVersion(t, v) { + if !testHasInvalidToolchainVersion(t, installedToolchainVersion, v) { t.Errorf("Expected testHasInvalidToolchainVersion(\"%s\") to be true, but got false", v) } } @@ -62,7 +64,7 @@ func TestHasInvalidToolchainVersion(t *testing.T) { } for _, v := range valid { - if testHasInvalidToolchainVersion(t, v) { + if testHasInvalidToolchainVersion(t, installedToolchainVersion, v) { t.Errorf("Expected testHasInvalidToolchainVersion(\"%s\") to be false, but got true", v) } }