this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

mod/module,cue/load,internal/filetypes: reject version in import qualifier

Give a clear error when a version appears inside the package qualifier
of an import path, such as "foo.com/bar:baz@v0" or "foo.com/bar@v0:baz@v1",
rather than confusing messages like "no files in package directory with
package name other@v0" or "cannot combine scope with file".

The validation is in CheckImportPath, with an early check in the loader
for CLI arguments that may not reach CheckImportPath (e.g. no module).
IsPackage uses the presence of "/" in the path to distinguish package
paths from filetype scopes when the qualifier isn't a valid identifier.

Fixes #2859.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I92e4e9e50e6f56dc35bf51c97f073ca83b1248c5
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1233779
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

+33 -20
+1 -2
cmd/cue/cmd/testdata/script/mod_absolute_package.txtar
··· 29 29 cmp stderr stderr4-golden 30 30 31 31 # Check that the package qualifier must come after the version, not before. 32 - # TODO: this should give a clear error. 33 32 ! exec cue export foo.com/bar:baz@v1.5.0 34 - stderr 'cannot combine scope with file' 33 + stderr 'package qualifier must not contain a version; use path@version:qualifier syntax' 35 34 36 35 -- stdout1-golden -- 37 36 {
+2 -4
cmd/cue/cmd/testdata/script/registry_importpath_bad_version.txtar
··· 4 4 cmp stderr ../expect-stderr-full-version 5 5 6 6 # Test that the version must come before the package qualifier, not after. 7 - # TODO: this should give a clear error. 8 7 cd ../qualifier-before 9 8 ! exec cue export . 10 9 cmp stderr ../expect-stderr-qualifier-before ··· 15 14 cmp stderr ../expect-stderr-full-version-qualifier 16 15 17 16 # Test that a version in the qualifier is not treated as a path version. 18 - # TODO: this should give a clear error. 19 17 cd ../version-in-qualifier 20 18 ! exec cue export . 21 19 cmp stderr ../expect-stderr-version-in-qualifier ··· 29 27 test.org/full-version@v0: import failed: cannot find package "example.com/main@v0.0.2": malformed import path "example.com/main@v0.0.2": import paths can only contain a major version specifier: 30 28 ./main.cue:2:8 31 29 -- expect-stderr-qualifier-before -- 32 - test.org/qualifier-before@v0: import failed: cannot find package "example.com/main": no files in package directory with package name "other@v0": 30 + test.org/qualifier-before@v0: import failed: cannot find package "example.com/main": malformed import path "example.com/main:other@v0": package qualifier must not contain a version; use path@version:qualifier syntax: 33 31 ./main.cue:2:8 34 32 -- expect-stderr-full-version-qualifier -- 35 33 test.org/full-version-qualifier@v0: import failed: cannot find package "example.com/main@v0.0.2": malformed import path "example.com/main@v0.0.2:other": import paths can only contain a major version specifier: 36 34 ./main.cue:2:8 37 35 -- expect-stderr-version-in-qualifier -- 38 - test.org/version-in-qualifier@v0: import failed: cannot find package "example.com/main@v0": no files in package directory with package name "other@v1": 36 + test.org/version-in-qualifier@v0: import failed: cannot find package "example.com/main@v0": malformed import path "example.com/main@v0:other@v1": package qualifier must not contain a version; use path@version:qualifier syntax: 39 37 ./main.cue:2:8 40 38 -- expect-stdout-qualifier-after -- 41 39 {
+5
cue/load/import.go
··· 485 485 return failf("standard library import path %q cannot be imported as a CUE package", p) 486 486 } 487 487 if l.pkgs == nil { 488 + // Validate import paths before reporting "no module" so that 489 + // malformed paths get a clear error rather than a confusing one. 490 + if err := module.CheckImportPath(string(p)); err != nil { 491 + return failf("%s", err) 492 + } 488 493 return failf("imports are unavailable because there is no cue.mod/module.cue file") 489 494 } 490 495 // Extract the package name.
+10 -6
internal/filetypes/util.go
··· 32 32 33 33 ip := ast.ParseImportPath(s) 34 34 if ip.ExplicitQualifier { 35 - if !ast.IsValidIdent(ip.Qualifier) || strings.Contains(ip.Path, ":") || ip.Path == "-" { 36 - // TODO potentially widen the scope of "file-like" 37 - // paths here to include more invalid package paths? 35 + if strings.Contains(ip.Path, ":") || ip.Path == "-" { 38 36 return false 39 37 } 40 - // If it's got an explicit qualifier, the path has a colon in 41 - // which isn't generally allowed in CUE file names. 42 - return true 38 + if ast.IsValidIdent(ip.Qualifier) { 39 + // If it's got an explicit qualifier, the path has a colon 40 + // which isn't generally allowed in CUE file names. 41 + return true 42 + } 43 + // Even with an invalid qualifier (e.g. "pkg@v1" where the 44 + // version was placed after the qualifier), a path with a slash 45 + // indicates a package path, not a filetype scope like "json:". 46 + return strings.Contains(ip.Path, "/") 43 47 } 44 48 if ip.Version != "" { 45 49 if strings.Contains(ip.Version, "/") {
+8 -8
mod/module/module_test.go
··· 489 489 }, { 490 490 path: `foo.com/bar/baz`, 491 491 }, { 492 - // TODO: CheckImportPath should reject a version inside the qualifier. 493 - path: `foo.com/bar:baz@v0`, 494 - modErr: `module path inappropriately contains version`, 495 - fileErr: `malformed file path "foo.com/bar:baz@v0": invalid char ':'`, 492 + path: `foo.com/bar:baz@v0`, 493 + modErr: `module path inappropriately contains version`, 494 + importErr: `malformed import path "foo.com/bar:baz@v0": package qualifier must not contain a version; use path@version:qualifier syntax`, 495 + fileErr: `malformed file path "foo.com/bar:baz@v0": invalid char ':'`, 496 496 }, { 497 - // TODO: CheckImportPath should reject a version inside the qualifier. 498 - path: `foo.com/bar@v0:baz@v1`, 499 - modErr: `module path inappropriately contains version`, 500 - fileErr: `malformed file path "foo.com/bar@v0:baz@v1": invalid char ':'`, 497 + path: `foo.com/bar@v0:baz@v1`, 498 + modErr: `module path inappropriately contains version`, 499 + importErr: `malformed import path "foo.com/bar@v0:baz@v1": package qualifier must not contain a version; use path@version:qualifier syntax`, 500 + fileErr: `malformed file path "foo.com/bar@v0:baz@v1": invalid char ':'`, 501 501 }} 502 502 503 503 func TestCheckPathWithoutVersion(t *testing.T) {
+7
mod/module/path.go
··· 195 195 // on Windows, regardless of case (CON, com1, NuL, and so on). 196 196 func CheckImportPath(path string) error { 197 197 parts := ast.ParseImportPath(path) 198 + if strings.Contains(parts.Qualifier, "@") { 199 + return &InvalidPathError{ 200 + Kind: "import", 201 + Path: path, 202 + Err: fmt.Errorf("package qualifier must not contain a version; use path@version:qualifier syntax"), 203 + } 204 + } 198 205 if semver.Major(parts.Version) != parts.Version { 199 206 return &InvalidPathError{ 200 207 Kind: "import",