cue: fix import usage detection for partially resolved files
Previously, the usage of imports was deteced by matching
them with unresolved references. This did not work, however,
when passing a previously processed ast.File, with already
resolved references.
The problem was previously undetected, as the CUE builder
would always write out files and parse them back in. Recent
optimizations that elide this step exposed this problem.
This change also makes uses of the newer multi-error
infrastructure and improves some of the naming of vars.
Change-Id: Ieda38b989b050d90f0ced49e91acaea58ca47e19
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/5360
Reviewed-by: Jason Wang <jasonwzm@google.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/build.go b/cue/build.go
index 5f7aa62..f7e17fa 100644
--- a/cue/build.go
+++ b/cue/build.go
@@ -361,9 +361,9 @@
}
func resolveFile(idx *index, f *ast.File, p *build.Instance, allFields map[string]ast.Node) errors.Error {
- index := map[string][]*ast.Ident{}
+ unresolved := map[string][]*ast.Ident{}
for _, u := range f.Unresolved {
- index[u.Name] = append(index[u.Name], u)
+ unresolved[u.Name] = append(unresolved[u.Name], u)
}
fields := map[string]ast.Node{}
for _, d := range f.Decls {
@@ -373,7 +373,9 @@
}
}
}
- var errUnused errors.Error
+ var errs errors.Error
+
+ specs := []*ast.ImportSpec{}
for _, spec := range f.Imports {
id, err := strconv.Unquote(spec.Path.Value)
@@ -384,34 +386,65 @@
if imp := p.LookupImport(id); imp != nil {
name = imp.PkgName
} else if _, ok := builtins[id]; !ok {
- // continue
- return nodeErrorf(spec, "package %q not found", id)
+ errs = errors.Append(errs,
+ nodeErrorf(spec, "package %q not found", id))
+ continue
}
if spec.Name != nil {
name = spec.Name.Name
}
if n, ok := fields[name]; ok {
- return nodeErrorf(spec,
+ errs = errors.Append(errs, nodeErrorf(spec,
"%s redeclared as imported package name\n"+
- "\tprevious declaration at %v", name, lineStr(idx, n))
+ "\tprevious declaration at %v", name, lineStr(idx, n)))
+ continue
}
fields[name] = spec
used := false
- for _, u := range index[name] {
+ for _, u := range unresolved[name] {
used = true
u.Node = spec
}
if !used {
+ specs = append(specs, spec)
+ }
+ }
+
+ // Verify each import is used.
+ if len(specs) > 0 {
+ // Find references to imports. This assumes that identifiers in labels
+ // are not resolved or that such errors are caught elsewhere.
+ ast.Walk(f, nil, func(n ast.Node) {
+ if x, ok := n.(*ast.Ident); ok {
+ // As we also visit labels, most nodes will be nil.
+ if x.Node == nil {
+ return
+ }
+ for i, s := range specs {
+ if s == x.Node {
+ specs[i] = nil
+ return
+ }
+ }
+ }
+ })
+
+ // Add errors for unused imports.
+ for _, spec := range specs {
+ if spec == nil {
+ continue
+ }
if spec.Name == nil {
- errUnused = nodeErrorf(spec,
- "imported and not used: %s", spec.Path.Value)
+ errs = errors.Append(errs, nodeErrorf(spec,
+ "imported and not used: %s", spec.Path.Value))
} else {
- errUnused = nodeErrorf(spec,
- "imported and not used: %s as %s", spec.Path.Value, spec.Name)
+ errs = errors.Append(errs, nodeErrorf(spec,
+ "imported and not used: %s as %s", spec.Path.Value, spec.Name))
}
}
}
- i := 0
+
+ k := 0
for _, u := range f.Unresolved {
if u.Node != nil {
continue
@@ -421,17 +454,14 @@
u.Scope = f
continue
}
- f.Unresolved[i] = u
- i++
+ f.Unresolved[k] = u
+ k++
}
- f.Unresolved = f.Unresolved[:i]
+ f.Unresolved = f.Unresolved[:k]
// TODO: also need to resolve types.
// if len(f.Unresolved) > 0 {
// n := f.Unresolved[0]
// return ctx.mkErr(newBase(n), "unresolved reference %s", n.Name)
// }
- if errUnused != nil {
- return errUnused
- }
- return nil
+ return errs
}
diff --git a/cue/build_test.go b/cue/build_test.go
index 10f8336..90b15b7 100644
--- a/cue/build_test.go
+++ b/cue/build_test.go
@@ -53,6 +53,45 @@
}
}
+// TestPartiallyResolved tests that the resolve will detect the usage of
+// imports that are referenced by previously resolved nodes.
+func TestPartiallyResolved(t *testing.T) {
+ const importPath = "acme.com/foo"
+ spec1 := &ast.ImportSpec{
+ Path: ast.NewString(importPath),
+ }
+ spec2 := &ast.ImportSpec{
+ Name: ast.NewIdent("bar"),
+ Path: ast.NewString(importPath),
+ }
+
+ f := &ast.File{
+ Decls: []ast.Decl{
+ &ast.ImportDecl{Specs: []*ast.ImportSpec{spec1, spec2}},
+ &ast.Field{
+ Label: ast.NewIdent("X"),
+ Value: &ast.Ident{Name: "foo", Node: spec1},
+ },
+ &ast.Alias{
+ Ident: ast.NewIdent("Y"),
+ Expr: &ast.Ident{Name: "bar", Node: spec2},
+ },
+ },
+ Imports: []*ast.ImportSpec{spec1, spec2},
+ }
+
+ err := resolveFile(nil, f, &build.Instance{
+ Imports: []*build.Instance{{
+ ImportPath: importPath,
+ PkgName: "foo",
+ }},
+ }, map[string]ast.Node{})
+
+ if err != nil {
+ t.Errorf("exected no error, found %v", err)
+ }
+}
+
func TestBuild(t *testing.T) {
files := func(s ...string) []string { return s }
insts := func(i ...*bimport) []*bimport { return i }
@@ -137,16 +176,17 @@
}),
`"Hello World!"`,
}, {
- insts(pkg1, &bimport{"",
+ insts(pkg1, pkg2, &bimport{"",
files(
`package test
import bar "pkg1"
+ import baz "example.com/foo/pkg2:pkg"
pkg1 Object: 3
"Hello \(pkg1.Object)!"`),
}),
- `imported and not used: "pkg1" as bar`,
+ `imported and not used: "pkg1" as bar (and 1 more errors)`,
}, {
insts(pkg2, &bimport{"",
files(
