From 69b87a63b851a64c4a89c495f3f58ae6e5a40c8d Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 24 Apr 2025 14:41:05 +0200 Subject: [PATCH] Go: remove invalid toolchain version diagnostics This diagnostic was introduced by https://github.com/github/codeql/pull/15979. However in the meantime the Go team [has backtracked](https://github.com/golang/go/issues/62278#issuecomment-2062002018) on their decision, which leads to confusing alerts for user (e.g. https://github.com/github/codeql-action/issues/2868). Even using Go toolchains from 1.21 to 1.22 we weren't immediately able to reproduce the problem that this diagnostics was meant to guard against. Therefore it was deemed simpler to just remove it. _En passant_ the `Makefile` now accepts `rtjo` not being set. --- go/Makefile | 2 ++ .../cli/go-autobuilder/go-autobuilder.go | 2 +- go/extractor/diagnostics/diagnostics.go | 15 -------- go/extractor/project/project.go | 34 +++++-------------- go/extractor/project/project_test.go | 29 ---------------- .../build_environment.expected | 5 --- .../diagnostics.expected | 31 ----------------- .../invalid-toolchain-version/src/go.mod | 3 -- .../invalid-toolchain-version/src/main.go | 5 --- .../invalid-toolchain-version/test.py | 6 ---- .../go-version-bump/diagnostics.expected | 17 ---------- .../newer-go-needed/diagnostics.expected | 17 ---------- 12 files changed, 11 insertions(+), 155 deletions(-) delete mode 100644 go/ql/integration-tests/diagnostics/invalid-toolchain-version/build_environment.expected delete mode 100644 go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected delete mode 100644 go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/go.mod delete mode 100644 go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/main.go delete mode 100644 go/ql/integration-tests/diagnostics/invalid-toolchain-version/test.py diff --git a/go/Makefile b/go/Makefile index b32b61bc5c38..5bf6d70e0e69 100644 --- a/go/Makefile +++ b/go/Makefile @@ -5,6 +5,8 @@ # and that we actually get are too big, the build fails on CI. BAZEL := $(shell bash -c "which bazel") +rtjo ?= none + all: gen extractor EXTRACTOR_PACK_OUT = extractor-pack diff --git a/go/extractor/cli/go-autobuilder/go-autobuilder.go b/go/extractor/cli/go-autobuilder/go-autobuilder.go index aeff4f0bd6e1..50118c8dc9ff 100644 --- a/go/extractor/cli/go-autobuilder/go-autobuilder.go +++ b/go/extractor/cli/go-autobuilder/go-autobuilder.go @@ -425,7 +425,7 @@ func installDependencies(workspace project.GoWorkspace) { } else { if workspace.Modules == nil { project.InitGoModForLegacyProject(workspace.BaseDir) - workspace.Modules = project.LoadGoModules(true, []string{filepath.Join(workspace.BaseDir, "go.mod")}) + workspace.Modules = project.LoadGoModules([]string{filepath.Join(workspace.BaseDir, "go.mod")}) } // get dependencies for all modules diff --git a/go/extractor/diagnostics/diagnostics.go b/go/extractor/diagnostics/diagnostics.go index 385c611aac81..00179b98cca5 100644 --- a/go/extractor/diagnostics/diagnostics.go +++ b/go/extractor/diagnostics/diagnostics.go @@ -508,18 +508,3 @@ func EmitExtractionFailedForProjects(path []string) { noLocation, ) } - -func EmitInvalidToolchainVersion(goModPath string, version string) { - emitDiagnostic( - "go/autobuilder/invalid-go-toolchain-version", - "Invalid Go toolchain version", - strings.Join([]string{ - "As of Go 1.21, toolchain versions [must use the 1.N.P syntax](https://go.dev/doc/toolchain#version).", - fmt.Sprintf("`%s` in `%s` does not match this syntax and there is no additional `toolchain` directive, which may cause some `go` commands to fail.", version, goModPath), - }, - "\n\n"), - severityWarning, - fullVisibility, - &locationStruct{File: goModPath}, - ) -} diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index eff69c0e9cac..1377b2513418 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -192,20 +192,10 @@ func findGoModFiles(root string) []string { return util.FindAllFilesWithName(root, "go.mod", util.SkipVendorChecks...) } -// A regular expression for the Go toolchain version syntax. -var toolchainVersionRe *regexp.Regexp = regexp.MustCompile(`(?m)^([0-9]+\.[0-9]+(\.[0-9]+|rc[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. -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) -} - // Given a list of `go.mod` file paths, try to parse them all. The resulting array of `GoModule` objects // will be the same length as the input array and the objects will contain at least the `go.mod` path. // If parsing the corresponding file is successful, then the parsed contents will also be available. -func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { +func LoadGoModules(goModFilePaths []string) []*GoModule { results := make([]*GoModule, len(goModFilePaths)) for i, goModFilePath := range goModFilePaths { @@ -227,14 +217,6 @@ func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { } results[i].Module = modFile - - // If this `go.mod` file specifies a Go language version, that version is `1.21` or greater, and - // 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) { - diagnostics.EmitInvalidToolchainVersion(goModFilePath, modFile.Go.Version) - } } return results @@ -243,7 +225,7 @@ func LoadGoModules(emitDiagnostics bool, goModFilePaths []string) []*GoModule { // Given a path to a `go.work` file, this function attempts to parse the `go.work` file. If unsuccessful, // we attempt to discover `go.mod` files within subdirectories of the directory containing the `go.work` // file ourselves. -func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { +func discoverWorkspace(workFilePath string) GoWorkspace { log.Printf("Loading %s...\n", workFilePath) baseDir := filepath.Dir(workFilePath) workFileSrc, err := os.ReadFile(workFilePath) @@ -257,7 +239,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, baseDir), } @@ -274,7 +256,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, baseDir), } @@ -297,7 +279,7 @@ func discoverWorkspace(emitDiagnostics bool, workFilePath string) GoWorkspace { return GoWorkspace{ BaseDir: baseDir, WorkspaceFile: workFile, - Modules: LoadGoModules(emitDiagnostics, goModFilePaths), + Modules: LoadGoModules(goModFilePaths), DepMode: GoGetWithModules, ModMode: ModReadonly, // Workspaces only support "readonly" } @@ -325,7 +307,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { for i, goModFile := range goModFiles { results[i] = GoWorkspace{ BaseDir: filepath.Dir(goModFile), - Modules: LoadGoModules(emitDiagnostics, []string{goModFile}), + Modules: LoadGoModules([]string{goModFile}), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, filepath.Dir(goModFile)), } @@ -342,7 +324,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { results := make([]GoWorkspace, len(goWorkFiles)) for i, workFilePath := range goWorkFiles { - results[i] = discoverWorkspace(emitDiagnostics, workFilePath) + results[i] = discoverWorkspace(workFilePath) } // Add all stray `go.mod` files (i.e. those not referenced by `go.work` files) @@ -374,7 +356,7 @@ func discoverWorkspaces(emitDiagnostics bool) []GoWorkspace { log.Printf("Module %s is not referenced by any go.work file; adding it separately.\n", goModFile) results = append(results, GoWorkspace{ BaseDir: filepath.Dir(goModFile), - Modules: LoadGoModules(emitDiagnostics, []string{goModFile}), + Modules: LoadGoModules([]string{goModFile}), DepMode: GoGetWithModules, ModMode: getModMode(GoGetWithModules, filepath.Dir(goModFile)), }) diff --git a/go/extractor/project/project_test.go b/go/extractor/project/project_test.go index 149a9723ec29..55cb163e9197 100644 --- a/go/extractor/project/project_test.go +++ b/go/extractor/project/project_test.go @@ -39,35 +39,6 @@ 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) { - invalid := []string{ - "go 1.21\n", - "go 1.22\n", - } - - for _, v := range invalid { - if !testHasInvalidToolchainVersion(t, v) { - t.Errorf("Expected testHasInvalidToolchainVersion(\"%s\") to be true, but got false", v) - } - } - - valid := []string{ - "go 1.20\n", - "go 1.21.1\n", - "go 1.22\n\ntoolchain go1.22.0\n", - } - - for _, v := range valid { - if testHasInvalidToolchainVersion(t, v) { - t.Errorf("Expected testHasInvalidToolchainVersion(\"%s\") to be false, but got true", v) - } - } -} - func parseWorkFile(t *testing.T, contents string) *modfile.WorkFile { workFile, err := modfile.ParseWork("go.work", []byte(contents), nil) diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/build_environment.expected b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/build_environment.expected deleted file mode 100644 index 0b225ce00857..000000000000 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/build_environment.expected +++ /dev/null @@ -1,5 +0,0 @@ -{ - "configuration" : { - "go" : { } - } -} diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected deleted file mode 100644 index 2e5d732e53e7..000000000000 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/diagnostics.expected +++ /dev/null @@ -1,31 +0,0 @@ -{ - "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", - "source": { - "extractorName": "go", - "id": "go/autobuilder/single-root-go-mod-found", - "name": "A single `go.mod` file was found in the root" - }, - "visibility": { - "cliSummaryTable": false, - "statusPage": false, - "telemetry": true - } -} diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/go.mod b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/go.mod deleted file mode 100644 index bee3365b8999..000000000000 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/go.mod +++ /dev/null @@ -1,3 +0,0 @@ -go 1.21 - -module example diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/main.go b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/main.go deleted file mode 100644 index 79058077776c..000000000000 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/src/main.go +++ /dev/null @@ -1,5 +0,0 @@ -package main - -func main() { - -} diff --git a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/test.py b/go/ql/integration-tests/diagnostics/invalid-toolchain-version/test.py deleted file mode 100644 index 44aa92d67584..000000000000 --- a/go/ql/integration-tests/diagnostics/invalid-toolchain-version/test.py +++ /dev/null @@ -1,6 +0,0 @@ -import os - - -def test(codeql, go): - os.environ["LGTM_INDEX_IMPORT_PATH"] = "test" - codeql.database.create(source_root="src") 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",