From 44a16cef6c7ef26df5da08922c6046de284f9670 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:21:26 +0100 Subject: [PATCH 1/5] Go: Use `Toolchain` directives in `go.work` files, if available --- go/extractor/project/project.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 149dfe274bda..b574a37222d0 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -54,8 +54,14 @@ type GoVersionInfo = util.SemVer // 1. The Go version specified in the `go.work` file, if any. // 2. The greatest Go version specified in any `go.mod` file, if any. func (workspace *GoWorkspace) RequiredGoVersion() util.SemVer { - if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { - // If we have parsed a `go.work` file, return the version number from it. + // If we have parsed a `go.work` file, we prioritise versions from it over those in individual `go.mod` + // files. We are interested in toolchain versions, so if there is an explicit toolchain declaration in + // a `go.work` file, we use that. Otherwise, we fall back to the language version in the `go.work` file + // and use that as toolchain version. If we didn't parse a `go.work` file, then we try to find the + // greatest version contained in `go.mod` files. + if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Toolchain != nil { + return util.NewSemVer(workspace.WorkspaceFile.Toolchain.Name) + } else if workspace.WorkspaceFile != nil && workspace.WorkspaceFile.Go != nil { return util.NewSemVer(workspace.WorkspaceFile.Go.Version) } else if workspace.Modules != nil && len(workspace.Modules) > 0 { // Otherwise, if we have `go.work` files, find the greatest Go version in those. From 1d6f09c75012d32d9415acee26a4836c1eb8d6dc Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:27:16 +0100 Subject: [PATCH 2/5] Go: Refactor `go.mod` version retrieval into its own method --- go/extractor/project/project.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index b574a37222d0..7e937f27b332 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -36,6 +36,15 @@ type GoModule struct { Module *modfile.File // The parsed contents of the `go.mod` file } +// Tries to find the Go toolchain version required for this module. +func (module *GoModule) RequiredGoVersion() util.SemVer { + if module.Module != nil && module.Module.Go != nil { + return util.NewSemVer(module.Module.Go.Version) + } else { + return tryReadGoDirective(module.Path) + } +} + // Represents information about a Go project workspace: this may either be a folder containing // a `go.work` file or a collection of `go.mod` files. type GoWorkspace struct { @@ -67,17 +76,10 @@ func (workspace *GoWorkspace) RequiredGoVersion() util.SemVer { // Otherwise, if we have `go.work` files, find the greatest Go version in those. var greatestVersion util.SemVer = nil for _, module := range workspace.Modules { - if module.Module != nil && module.Module.Go != nil { - // If we have parsed the file, retrieve the version number we have already obtained. - modVersion := util.NewSemVer(module.Module.Go.Version) - if greatestVersion == nil || modVersion.IsNewerThan(greatestVersion) { - greatestVersion = modVersion - } - } else { - modVersion := tryReadGoDirective(module.Path) - if modVersion != nil && (greatestVersion == nil || modVersion.IsNewerThan(greatestVersion)) { - greatestVersion = modVersion - } + modVersion := module.RequiredGoVersion() + + if modVersion != nil && (greatestVersion == nil || modVersion.IsNewerThan(greatestVersion)) { + greatestVersion = modVersion } } From 504a23329974b2ace241f038b93f611c06dfd466 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 12:40:41 +0100 Subject: [PATCH 3/5] Go: Use `Toolchain` directives in `go.mod` files, if available --- go/extractor/project/project.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 7e937f27b332..842ce33e4ba0 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -38,6 +38,9 @@ type GoModule struct { // Tries to find the Go toolchain version required for this module. func (module *GoModule) RequiredGoVersion() util.SemVer { + if module.Module != nil && module.Module.Toolchain != nil { + return util.NewSemVer(module.Module.Toolchain.Name) + } if module.Module != nil && module.Module.Go != nil { return util.NewSemVer(module.Module.Go.Version) } else { From 881b2586e1a716c27e2045b3032f4b695f02fbf0 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 8 May 2024 13:05:12 +0100 Subject: [PATCH 4/5] Go: Add tests for `RequiredGoVersion` --- go/extractor/project/BUILD.bazel | 5 +- go/extractor/project/project_test.go | 82 +++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/go/extractor/project/BUILD.bazel b/go/extractor/project/BUILD.bazel index b4401187ce2d..9170ff95be35 100644 --- a/go/extractor/project/BUILD.bazel +++ b/go/extractor/project/BUILD.bazel @@ -19,5 +19,8 @@ go_test( name = "project_test", srcs = ["project_test.go"], embed = [":project"], - deps = ["//go/extractor/vendor/golang.org/x/mod/modfile"], + deps = [ + "//go/extractor/util", + "//go/extractor/vendor/golang.org/x/mod/modfile", + ], ) diff --git a/go/extractor/project/project_test.go b/go/extractor/project/project_test.go index b7485960b5fd..149a9723ec29 100644 --- a/go/extractor/project/project_test.go +++ b/go/extractor/project/project_test.go @@ -4,6 +4,7 @@ import ( "path/filepath" "testing" + "github.com/github/codeql-go/extractor/util" "golang.org/x/mod/modfile" ) @@ -28,14 +29,18 @@ func TestStartsWithAnyOf(t *testing.T) { testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "baz"), false) } -func testHasInvalidToolchainVersion(t *testing.T, contents string) bool { - modFile, err := modfile.Parse("test.go", []byte(contents), nil) +func parseModFile(t *testing.T, contents string) *modfile.File { + modFile, err := modfile.Parse("go.mod", []byte(contents), nil) if err != nil { t.Errorf("Unable to parse %s: %s.\n", contents, err.Error()) } - return hasInvalidToolchainVersion(modFile) + return modFile +} + +func testHasInvalidToolchainVersion(t *testing.T, contents string) bool { + return hasInvalidToolchainVersion(parseModFile(t, contents)) } func TestHasInvalidToolchainVersion(t *testing.T) { @@ -62,3 +67,74 @@ func TestHasInvalidToolchainVersion(t *testing.T) { } } } + +func parseWorkFile(t *testing.T, contents string) *modfile.WorkFile { + workFile, err := modfile.ParseWork("go.work", []byte(contents), nil) + + if err != nil { + t.Errorf("Unable to parse %s: %s.\n", contents, err.Error()) + } + + return workFile +} + +func TestRequiredGoVersion(t *testing.T) { + type ModVersionPair struct { + FileContents string + ExpectedVersion string + } + + modules := []ModVersionPair{ + {"go 1.20", "v1.20"}, + {"go 1.21.2", "v1.21.2"}, + {"go 1.21rc1", "v1.21.0-rc1"}, + {"go 1.21rc1\ntoolchain go1.22.0", "v1.22.0"}, + {"go 1.21rc1\ntoolchain go1.22rc1", "v1.22.0-rc1"}, + } + + for _, testData := range modules { + // `go.mod` and `go.work` files have mostly the same format + modFile := parseModFile(t, testData.FileContents) + workFile := parseWorkFile(t, testData.FileContents) + mod := GoModule{ + Path: "test", // irrelevant + Module: modFile, + } + work := GoWorkspace{ + WorkspaceFile: workFile, + } + + result := mod.RequiredGoVersion() + if result == nil { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got nothing:\n%s", + testData.ExpectedVersion, + testData.FileContents, + ) + } else if result != util.NewSemVer(testData.ExpectedVersion) { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.mod` file, but got %s:\n%s", + testData.ExpectedVersion, + result, + testData.FileContents, + ) + } + + result = work.RequiredGoVersion() + if result == nil { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got nothing:\n%s", + testData.ExpectedVersion, + testData.FileContents, + ) + } else if result != util.NewSemVer(testData.ExpectedVersion) { + t.Errorf( + "Expected mod.RequiredGoVersion() to return %s for the below `go.work` file, but got %s:\n%s", + testData.ExpectedVersion, + result, + testData.FileContents, + ) + } + } + +} From d4adc373c6fc3585f4cb5b8eff754de820a20493 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 10 Jun 2024 15:48:29 +0100 Subject: [PATCH 5/5] Replace `if` with `else if` in `RequiredGoVersion` --- go/extractor/project/project.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go/extractor/project/project.go b/go/extractor/project/project.go index 842ce33e4ba0..853e871d62bb 100644 --- a/go/extractor/project/project.go +++ b/go/extractor/project/project.go @@ -40,8 +40,7 @@ type GoModule struct { func (module *GoModule) RequiredGoVersion() util.SemVer { if module.Module != nil && module.Module.Toolchain != nil { return util.NewSemVer(module.Module.Toolchain.Name) - } - if module.Module != nil && module.Module.Go != nil { + } else if module.Module != nil && module.Module.Go != nil { return util.NewSemVer(module.Module.Go.Version) } else { return tryReadGoDirective(module.Path)