mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

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

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>

Daniel Martí c0d61938 38185284

+75 -4
+7
internal/mod/modimports/modimports.go
··· 144 144 return yield(pf, err) 145 145 } 146 146 defer f.Close() 147 + 148 + // Note that we use cueimports.Read before parser.ParseFile as cue/parser 149 + // will always consume the whole input reader, which is often wasteful. 150 + // 151 + // TODO(mvdan): the need for cueimports.Read can go once cue/parser can work 152 + // on a reader in a streaming manner. 147 153 data, err := cueimports.Read(f) 148 154 if err != nil { 149 155 return yield(pf, err) ··· 152 158 if err != nil { 153 159 return yield(pf, err) 154 160 } 161 + 155 162 if pkgQualifier != "*" && syntax.PackageName() != pkgQualifier { 156 163 return true 157 164 }
+20
internal/mod/modimports/testdata/nopackage.txtar
··· 1 + -- want -- 2 + imports_no_package.cue "dep1" 3 + no_package.cue 4 + -- want-imports -- 5 + dep1 6 + -- no_package.cue -- 7 + // Valid syntax without a package clause. 8 + 9 + foo: "bar" 10 + -- imports_no_package.cue -- 11 + // Valid syntax with an import but without a package clause. 12 + // 13 + // TODO(mvdan): should we collect imports from files without a package clause? 14 + // On one hand cue/load with Config.Package "*" loads files without package as the package "_", 15 + // but on the other, Go skips over `//go:build ignore` files in e.g. `go mod tidy`, 16 + // and perhaps we consider no-package files to be a de facto "ignored from any package". 17 + 18 + import "dep1" 19 + 20 + foo: "bar"
+48 -4
internal/mod/modimports/testdata/parseerror.txtar
··· 1 1 -- want -- 2 - x.cue: error: expected label or ':', found 'IDENT' contents 2 + bad_after_import.cue: error: expected ')', found newline 3 + bad_after_package.cue 4 + bad_after_package_import.cue "dep4" 5 + bad_imports.cue: error: string literal not terminated 6 + bad_lone.cue: error: expected ')', found newline 7 + bad_package_imports.cue: error: string literal not terminated 3 8 -- want-imports -- 4 - error: cannot read "x.cue": expected label or ':', found 'IDENT' contents 5 - -- x.cue -- 6 - bogus contents 9 + error: cannot read "bad_after_import.cue": expected ')', found newline 10 + -- bad_imports.cue -- 11 + // Invalid import syntax without a package clause. 12 + 13 + import "dep1 14 + 15 + -- bad_package_imports.cue -- 16 + // Invalid import syntax with a package clause; should fail. 17 + 18 + package p 19 + 20 + import "dep2 21 + 22 + -- bad_lone.cue -- 23 + // Invalid syntax without a package clause, assumed to be after imports. 24 + // TODO(mvdan): just like bad_after_package.cue, this shouldn't fail. 25 + 26 + (unterminated 27 + 28 + -- bad_after_package.cue -- 29 + // Invalid syntax with a package clause, assumed to be after imports. 30 + 31 + package p 32 + 33 + (unterminated 34 + 35 + -- bad_after_import.cue -- 36 + // Invalid syntax after some imports without a package clause. 37 + // TODO(mvdan): just like bad_after_package_import.cue, this shouldn't fail. 38 + 39 + import "dep3" 40 + 41 + (unterminated 42 + 43 + -- bad_after_package_import.cue -- 44 + // Invalid syntax after some imports with a package clause. 45 + 46 + package p 47 + 48 + import "dep4" 49 + 50 + (unterminated