From 00735d67f28958cf4b23ae1eda51753d0efaf324 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Fri, 31 Jul 2026 13:36:06 +0300 Subject: [PATCH] splitModule should conform to Go spec Signed-off-by: Mohammed Al Sahaf --- cmd/packagesfuncs.go | 24 ++++++++++++++-------- cmd/packagesfuncs_test.go | 42 +++++++++++++++++---------------------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/cmd/packagesfuncs.go b/cmd/packagesfuncs.go index 53e91f425..58d9b6181 100644 --- a/cmd/packagesfuncs.go +++ b/cmd/packagesfuncs.go @@ -49,20 +49,28 @@ func cmdUpgrade(fl Flags) (int, error) { func splitModule(arg string) (module, version string, err error) { const versionSplit = "@" - // accommodate module paths that have @ in them, but we can only tolerate that if there's also - // a version, otherwise we don't know if it's a version separator or part of the file path - lastVersionSplit := strings.LastIndex(arg, versionSplit) - if lastVersionSplit < 0 { + // Per https://go.dev/ref/mod#go-mod-file-ident, module paths consist of + // ASCII letters, digits, and `-`, `.`, `_`, `~` only, so `@` is never a + // valid path character. That makes the split unambiguous: at most one + // `@` may appear, and it separates the module path from the version. + switch strings.Count(arg, versionSplit) { + case 0: module = arg - } else { - module, version = arg[:lastVersionSplit], arg[lastVersionSplit+1:] + case 1: + idx := strings.Index(arg, versionSplit) + module, version = arg[:idx], arg[idx+1:] + if version == "" { + return "", "", fmt.Errorf("version is required after '@'") + } + default: + return "", "", fmt.Errorf("module path must not contain '@'") } if module == "" { - err = fmt.Errorf("module name is required") + return "", "", fmt.Errorf("module name is required") } - return module, version, err + return module, version, nil } func cmdAddPackage(fl Flags) (int, error) { diff --git a/cmd/packagesfuncs_test.go b/cmd/packagesfuncs_test.go index bc7f53cc3..e9e98fbf2 100644 --- a/cmd/packagesfuncs_test.go +++ b/cmd/packagesfuncs_test.go @@ -105,27 +105,21 @@ func TestSplitModule(t *testing.T) { expectError: false, }, { - name: "module with @ in path and version", + // Per go.dev/ref/mod#go-mod-file-ident, '@' is not a valid module + // path character, so any input with more than one '@' is invalid. + name: "module with @ in path is rejected", input: "github.com/@user/module@v1.0.0", - expectedModule: "github.com/@user/module", - expectedVersion: "v1.0.0", - expectError: false, + expectedModule: "", + expectedVersion: "", + expectError: true, }, { - name: "module with multiple @ in path", + name: "module with multiple @ in path is rejected", input: "github.com/@org/@user/module@v2.3.4", - expectedModule: "github.com/@org/@user/module", - expectedVersion: "v2.3.4", - expectError: false, + expectedModule: "", + expectedVersion: "", + expectError: true, }, - // TODO: decide on the behavior for this case; it fails currently - // { - // name: "module with @ in path but no version", - // input: "github.com/@user/module", - // expectedModule: "github.com/@user/module", - // expectedVersion: "", - // expectError: false, - // }, { name: "empty string", input: "", @@ -144,22 +138,22 @@ func TestSplitModule(t *testing.T) { name: "@ at start", input: "@v1.0.0", expectedModule: "", - expectedVersion: "v1.0.0", + expectedVersion: "", expectError: true, }, { - name: "@ at end", + name: "trailing @ with no version is rejected", input: "github.com/user/module@", - expectedModule: "github.com/user/module", + expectedModule: "", expectedVersion: "", - expectError: false, + expectError: true, }, { - name: "multiple consecutive @", + name: "multiple consecutive @ is rejected", input: "github.com/user/module@@v1.0.0", - expectedModule: "github.com/user/module@", - expectedVersion: "v1.0.0", - expectError: false, + expectedModule: "", + expectedVersion: "", + expectError: true, }, { name: "version with latest tag",