Merge pull request #15810 from github/mbg/go/fix-initialised-module-names

go/extractor/cli/go-autobuilder/go-autobuilder.go

@@ -3,7 +3,6 @@ package main
 import (
 	"fmt"
 	"log"
-	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -56,63 +55,6 @@ Build behavior:
 	fmt.Fprintf(os.Stderr, "Usage:\n\n  %s\n", os.Args[0])
 }
 
-// Returns the import path of the package being built, or "" if it cannot be determined.
-func getImportPath() (importpath string) {
-	importpath = os.Getenv("LGTM_INDEX_IMPORT_PATH")
-	if importpath == "" {
-		repourl := os.Getenv("SEMMLE_REPO_URL")
-		if repourl == "" {
-			githubrepo := os.Getenv("GITHUB_REPOSITORY")
-			if githubrepo == "" {
-				log.Printf("Unable to determine import path, as neither LGTM_INDEX_IMPORT_PATH nor GITHUB_REPOSITORY is set\n")
-				return ""
-			} else {
-				importpath = "github.com/" + githubrepo
-			}
-		} else {
-			importpath = getImportPathFromRepoURL(repourl)
-			if importpath == "" {
-				log.Printf("Failed to determine import path from SEMMLE_REPO_URL '%s'\n", repourl)
-				return
-			}
-		}
-	}
-	log.Printf("Import path is '%s'\n", importpath)
-	return
-}
-
-// Returns the import path of the package being built from `repourl`, or "" if it cannot be
-// determined.
-func getImportPathFromRepoURL(repourl string) string {
-	// check for scp-like URL as in "git@github.com:github/codeql-go.git"
-	shorturl := regexp.MustCompile(`^([^@]+@)?([^:]+):([^/].*?)(\.git)?$`)
-	m := shorturl.FindStringSubmatch(repourl)
-	if m != nil {
-		return m[2] + "/" + m[3]
-	}
-
-	// otherwise parse as proper URL
-	u, err := url.Parse(repourl)
-	if err != nil {
-		log.Fatalf("Malformed repository URL '%s'\n", repourl)
-	}
-
-	if u.Scheme == "file" {
-		// we can't determine import paths from file paths
-		return ""
-	}
-
-	if u.Hostname() == "" || u.Path == "" {
-		return ""
-	}
-
-	host := u.Hostname()
-	path := u.Path
-	// strip off leading slashes and trailing `.git` if present
-	path = regexp.MustCompile(`^/+|\.git$`).ReplaceAllString(path, "")
-	return host + "/" + path
-}
-
 func restoreRepoLayout(fromDir string, dirEntries []string, scratchDirName string, toDir string) {
 	for _, dirEntry := range dirEntries {
 		if dirEntry != scratchDirName {
@@ -568,7 +510,7 @@ func installDependenciesAndBuild() {
 	if len(workspaces) == 1 {
 		workspace := workspaces[0]
 
-		importpath := getImportPath()
+		importpath := util.GetImportPath()
 		needGopath := getNeedGopath(workspace, importpath)
 
 		inLGTM := os.Getenv("LGTM_SRC") != "" || os.Getenv("LGTM_INDEX_NEED_GOPATH") != ""

go/extractor/project/project.go

@@ -439,8 +439,9 @@ func getBuildRoots(emitDiagnostics bool) (goWorkspaces []GoWorkspace, totalModul
 			for _, component := range components {
 				path = filepath.Join(path, component)
 
-				// Try to initialize a `go.mod` file automatically for the stray source files.
-				if !slices.Contains(goModDirs, path) {
+				// Try to initialize a `go.mod` file automatically for the stray source files if
+				// doing so would not place it in a parent directory of an existing `go.mod` file.
+				if !startsWithAnyOf(path, goModDirs) {
 					goWorkspaces = append(goWorkspaces, GoWorkspace{
 						BaseDir: path,
 						DepMode: GoGetNoModules,
@@ -477,6 +478,16 @@ func getBuildRoots(emitDiagnostics bool) (goWorkspaces []GoWorkspace, totalModul
 	return
 }
 
+// Determines whether `str` starts with any of `prefixes`.
+func startsWithAnyOf(str string, prefixes []string) bool {
+	for _, prefix := range prefixes {
+		if relPath, err := filepath.Rel(str, prefix); err == nil && !strings.HasPrefix(relPath, "..") {
+			return true
+		}
+	}
+	return false
+}
+
 // Finds Go workspaces in the current working directory.
 func GetWorkspaceInfo(emitDiagnostics bool) []GoWorkspace {
 	bazelPaths := slices.Concat(

go/extractor/project/project_test.go

@@ -0,0 +1,27 @@
+package project
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func testStartsWithAnyOf(t *testing.T, path string, prefix string, expectation bool) {
+	result := startsWithAnyOf(path, []string{prefix})
+	if result != expectation {
+		t.Errorf("Expected startsWithAnyOf(%s, %s) to be %t, but it is %t.", path, prefix, expectation, result)
+	}
+}
+
+func TestStartsWithAnyOf(t *testing.T) {
+	testStartsWithAnyOf(t, ".", ".", true)
+	testStartsWithAnyOf(t, ".", "dir", true)
+	testStartsWithAnyOf(t, ".", filepath.Join("foo", "bar"), true)
+	testStartsWithAnyOf(t, "dir", "dir", true)
+	testStartsWithAnyOf(t, "foo", filepath.Join("foo", "bar"), true)
+	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "bar"), true)
+	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "bar", "baz"), true)
+
+	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), "foo", false)
+	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), "bar", false)
+	testStartsWithAnyOf(t, filepath.Join("foo", "bar"), filepath.Join("foo", "baz"), false)
+}

go/extractor/toolchain/toolchain.go

@@ -5,8 +5,10 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
+	"github.com/github/codeql-go/extractor/util"
 	"golang.org/x/mod/semver"
 )
 
@@ -81,7 +83,20 @@ func TidyModule(path string) *exec.Cmd {
 
 // Run `go mod init` in the directory given by `path`.
 func InitModule(path string) *exec.Cmd {
-	modInit := exec.Command("go", "mod", "init", "codeql/auto-project")
+	moduleName := "codeql/auto-project"
+
+	if importpath := util.GetImportPath(); importpath != "" {
+		// This should be something like `github.com/user/repo`
+		moduleName = importpath
+
+		// If we are not initialising the new module in the root directory of the workspace,
+		// append the relative path to the module name.
+		if relPath, err := filepath.Rel(".", path); err != nil && relPath != "." {
+			moduleName = moduleName + "/" + relPath
+		}
+	}
+
+	modInit := exec.Command("go", "mod", "init", moduleName)
 	modInit.Dir = path
 	return modInit
 }

go/extractor/util/util.go

@@ -6,9 +6,11 @@ import (
 	"io"
 	"io/fs"
 	"log"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"slices"
 	"strings"
@@ -350,3 +352,60 @@ func GetParentDirs(paths []string) []string {
 	}
 	return dirs
 }
+
+// Returns the import path of the package being built, or "" if it cannot be determined.
+func GetImportPath() (importpath string) {
+	importpath = os.Getenv("LGTM_INDEX_IMPORT_PATH")
+	if importpath == "" {
+		repourl := os.Getenv("SEMMLE_REPO_URL")
+		if repourl == "" {
+			githubrepo := os.Getenv("GITHUB_REPOSITORY")
+			if githubrepo == "" {
+				log.Printf("Unable to determine import path, as neither LGTM_INDEX_IMPORT_PATH nor GITHUB_REPOSITORY is set\n")
+				return ""
+			} else {
+				importpath = "github.com/" + githubrepo
+			}
+		} else {
+			importpath = getImportPathFromRepoURL(repourl)
+			if importpath == "" {
+				log.Printf("Failed to determine import path from SEMMLE_REPO_URL '%s'\n", repourl)
+				return
+			}
+		}
+	}
+	log.Printf("Import path is '%s'\n", importpath)
+	return
+}
+
+// Returns the import path of the package being built from `repourl`, or "" if it cannot be
+// determined.
+func getImportPathFromRepoURL(repourl string) string {
+	// check for scp-like URL as in "git@github.com:github/codeql-go.git"
+	shorturl := regexp.MustCompile(`^([^@]+@)?([^:]+):([^/].*?)(\.git)?$`)
+	m := shorturl.FindStringSubmatch(repourl)
+	if m != nil {
+		return m[2] + "/" + m[3]
+	}
+
+	// otherwise parse as proper URL
+	u, err := url.Parse(repourl)
+	if err != nil {
+		log.Fatalf("Malformed repository URL '%s'\n", repourl)
+	}
+
+	if u.Scheme == "file" {
+		// we can't determine import paths from file paths
+		return ""
+	}
+
+	if u.Hostname() == "" || u.Path == "" {
+		return ""
+	}
+
+	host := u.Hostname()
+	path := u.Path
+	// strip off leading slashes and trailing `.git` if present
+	path = regexp.MustCompile(`^/+|\.git$`).ReplaceAllString(path, "")
+	return host + "/" + path
+}

go/extractor/util/util_test.go

@@ -1,4 +1,4 @@
-package main
+package util
 
 import "testing"
 

go/ql/integration-tests/all-platforms/go/mixed-layout/diagnostics.expected

@@ -0,0 +1,28 @@
+{
+  "markdownMessage": "1 `go.work` file was found:\n\n`workspace/go.work`",
+  "severity": "note",
+  "source": {
+    "extractorName": "go",
+    "id": "go/autobuilder/go-work-found",
+    "name": "`go.work` file found"
+  },
+  "visibility": {
+    "cliSummaryTable": false,
+    "statusPage": false,
+    "telemetry": true
+  }
+}
+{
+  "markdownMessage": "Go files were found outside of the Go modules corresponding to these `go.mod` files.\n\n`workspace/subdir/go.mod`, `module/go.mod`",
+  "severity": "note",
+  "source": {
+    "extractorName": "go",
+    "id": "go/autobuilder/go-files-outside-go-modules",
+    "name": "Go files were found outside Go modules"
+  },
+  "visibility": {
+    "cliSummaryTable": false,
+    "statusPage": false,
+    "telemetry": true
+  }
+}

go/ql/integration-tests/all-platforms/go/mixed-layout/force_sequential_test_execution

@@ -0,0 +1,2 @@
+# go get has been observed to sometimes fail when multiple tests try to simultaneously fetch the same package.
+goget

go/ql/integration-tests/all-platforms/go/mixed-layout/src/module/go.mod

@@ -0,0 +1,5 @@
+go 1.14
+
+require golang.org/x/net v0.0.0-20200505041828-1ed23360d12c
+
+module module

go/ql/integration-tests/all-platforms/go/mixed-layout/src/module/go.sum

@@ -0,0 +1,7 @@
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/net v0.0.0-20200505041828-1ed23360d12c h1:zJ0mtu4jCalhKg6Oaukv6iIkb+cOvDrajDH9DH46Q4M=
+golang.org/x/net v0.0.0-20200505041828-1ed23360d12c/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884=
+golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

go/ql/integration-tests/all-platforms/go/mixed-layout/src/module/test.go

@@ -0,0 +1,13 @@
+package subdir
+
+import (
+	"fmt"
+
+	"golang.org/x/net/ipv4"
+)
+
+func test() {
+
+	header := ipv4.Header{}
+	fmt.Print(header.String())
+}

go/ql/integration-tests/all-platforms/go/mixed-layout/src/stray-files/test.go

@@ -0,0 +1,13 @@
+package subdir
+
+import (
+	"fmt"
+
+	"golang.org/x/net/ipv4"
+)
+
+func test() {
+
+	header := ipv4.Header{}
+	fmt.Print(header.String())
+}

go/ql/integration-tests/all-platforms/go/mixed-layout/src/workspace/go.work

@@ -0,0 +1,3 @@
+go 1.22.0
+
+use ./subdir

go/ql/integration-tests/all-platforms/go/mixed-layout/src/workspace/subdir/go.mod

@@ -0,0 +1,5 @@
+go 1.22.0
+
+require golang.org/x/net v0.0.0-20200505041828-1ed23360d12c
+
+module subdir

go/ql/integration-tests/all-platforms/go/mixed-layout/src/workspace/subdir/go.sum

@@ -0,0 +1,7 @@
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/net v0.0.0-20200505041828-1ed23360d12c h1:zJ0mtu4jCalhKg6Oaukv6iIkb+cOvDrajDH9DH46Q4M=
+golang.org/x/net v0.0.0-20200505041828-1ed23360d12c/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884=
+golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

go/ql/integration-tests/all-platforms/go/mixed-layout/src/workspace/subdir/test.go

@@ -0,0 +1,13 @@
+package subdir
+
+import (
+	"fmt"
+
+	"golang.org/x/net/ipv4"
+)
+
+func test() {
+
+	header := ipv4.Header{}
+	fmt.Print(header.String())
+}

go/ql/integration-tests/all-platforms/go/mixed-layout/test.expected

@@ -0,0 +1,8 @@
+extractedFiles
+| src/module/go.mod:0:0:0:0 | src/module/go.mod |
+| src/module/test.go:0:0:0:0 | src/module/test.go |
+| src/stray-files/go.mod:0:0:0:0 | src/stray-files/go.mod |
+| src/stray-files/test.go:0:0:0:0 | src/stray-files/test.go |
+| src/workspace/subdir/go.mod:0:0:0:0 | src/workspace/subdir/go.mod |
+| src/workspace/subdir/test.go:0:0:0:0 | src/workspace/subdir/test.go |
+#select

go/ql/integration-tests/all-platforms/go/mixed-layout/test.py

@@ -0,0 +1,18 @@
+import os
+import subprocess
+
+from create_database_utils import *
+from diagnostics_test_utils import *
+
+# Set up a GOPATH relative to this test's root directory;
+# we set os.environ instead of using extra_env because we
+# need it to be set for the call to "go clean -modcache" later
+goPath = os.path.join(os.path.abspath(os.getcwd()), ".go")
+os.environ['GOPATH'] = goPath
+run_codeql_database_create([], lang="go", source="src")
+
+check_diagnostics()
+
+# Clean up the temporary GOPATH to prevent Bazel failures next
+# time the tests are run; see https://github.com/golang/go/issues/27161
+subprocess.call(["go", "clean", "-modcache"])

go/ql/integration-tests/all-platforms/go/mixed-layout/test.ql

@@ -0,0 +1,8 @@
+import go
+import semmle.go.DiagnosticsReporting
+
+query predicate extractedFiles(File f) { any() }
+
+from string msg, int sev
+where reportableDiagnostics(_, msg, sev)
+select msg, sev

go/ql/integration-tests/all-platforms/go/single-go-mod-and-go-files-not-under-it/test.expected

@@ -1,6 +1,4 @@
 extractedFiles
-| src/go.mod:0:0:0:0 | src/go.mod |
-| src/main.go:0:0:0:0 | src/main.go |
 | src/subdir/go.mod:0:0:0:0 | src/subdir/go.mod |
 | src/subdir/subsubdir/add.go:0:0:0:0 | src/subdir/subsubdir/add.go |
 | src/subdir/test.go:0:0:0:0 | src/subdir/test.go |