internal/mod/modimports: add more syntax edge cases to testdata

I was surprised to see that modimports used cueimports.Read
to only consume enough tokens to read a CUE file's import declarations.
This seems to have been inherited from Go, where there is this API:

    package imports

    func ReadImports(f io.Reader, reportSyntaxError bool, imports *[]string) ([]byte, error)

The reason ends up being the same for both Go and CUE, as CUE inherited
multiple Go API decisions from Go: both go/parser and cue/parser
will consume an entire input io.Reader, so when there are any large
CUE files in a module, we would read the entire file into memory
when in practice the imports are likely a tiny fraction of the bytes.

Add a comment explaining that, to prevent others from being surprised
just like I was, and add a TODO about cue/parser being better.

Add plenty more testdata txtar cases for edge cases.
Note that a few don't behave the way I might expect, so add TODOs.

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

internal/mod/modimports/modimports.go

@@ -144,6 +144,12 @@ func yieldPackageFile(fsys fs.FS, fpath, pkgQualifier string, yield func(ModuleF
 		return yield(pf, err)
 	}
 	defer f.Close()
+
+	// Note that we use cueimports.Read before parser.ParseFile as cue/parser
+	// will always consume the whole input reader, which is often wasteful.
+	//
+	// TODO(mvdan): the need for cueimports.Read can go once cue/parser can work
+	// on a reader in a streaming manner.
 	data, err := cueimports.Read(f)
 	if err != nil {
 		return yield(pf, err)
@@ -152,6 +158,7 @@ func yieldPackageFile(fsys fs.FS, fpath, pkgQualifier string, yield func(ModuleF
 	if err != nil {
 		return yield(pf, err)
 	}
+
 	if pkgQualifier != "*" && syntax.PackageName() != pkgQualifier {
 		return true
 	}

internal/mod/modimports/testdata/nopackage.txtar

@@ -0,0 +1,20 @@
+-- want --
+imports_no_package.cue "dep1"
+no_package.cue
+-- want-imports --
+dep1
+-- no_package.cue --
+// Valid syntax without a package clause.
+
+foo: "bar"
+-- imports_no_package.cue --
+// Valid syntax with an import but without a package clause.
+//
+// TODO(mvdan): should we collect imports from files without a package clause?
+// On one hand cue/load with Config.Package "*" loads files without package as the package "_",
+// but on the other, Go skips over `//go:build ignore` files in e.g. `go mod tidy`,
+// and perhaps we consider no-package files to be a de facto "ignored from any package".
+
+import "dep1"
+
+foo: "bar"

internal/mod/modimports/testdata/parseerror.txtar

@@ -1,6 +1,50 @@
 -- want --
-x.cue: error: expected label or ':', found 'IDENT' contents
+bad_after_import.cue: error: expected ')', found newline
+bad_after_package.cue
+bad_after_package_import.cue "dep4"
+bad_imports.cue: error: string literal not terminated
+bad_lone.cue: error: expected ')', found newline
+bad_package_imports.cue: error: string literal not terminated
 -- want-imports --
-error: cannot read "x.cue": expected label or ':', found 'IDENT' contents
--- x.cue --
-bogus contents
+error: cannot read "bad_after_import.cue": expected ')', found newline
+-- bad_imports.cue --
+// Invalid import syntax without a package clause.
+
+import "dep1
+
+-- bad_package_imports.cue --
+// Invalid import syntax with a package clause; should fail.
+
+package p
+
+import "dep2
+
+-- bad_lone.cue --
+// Invalid syntax without a package clause, assumed to be after imports.
+// TODO(mvdan): just like bad_after_package.cue, this shouldn't fail.
+
+(unterminated
+
+-- bad_after_package.cue --
+// Invalid syntax with a package clause, assumed to be after imports.
+
+package p
+
+(unterminated
+
+-- bad_after_import.cue --
+// Invalid syntax after some imports without a package clause.
+// TODO(mvdan): just like bad_after_package_import.cue, this shouldn't fail.
+
+import "dep3"
+
+(unterminated
+
+-- bad_after_package_import.cue --
+// Invalid syntax after some imports with a package clause.
+
+package p
+
+import "dep4"
+
+(unterminated