diff --git a/cue/build.go b/cue/build.go index 5f7aa6209..f7e17fae9 100644 --- a/cue/build.go +++ b/cue/build.go @@ -361,9 +361,9 @@ func resolveFiles(idx *index, p *build.Instance) errors.Error { } 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 @@ func resolveFile(idx *index, f *ast.File, p *build.Instance, allFields map[strin } } } - 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 @@ func resolveFile(idx *index, f *ast.File, p *build.Instance, allFields map[strin 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 @@ func resolveFile(idx *index, f *ast.File, p *build.Instance, allFields map[strin 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 10f8336e2..90b15b7a3 100644 --- a/cue/build_test.go +++ b/cue/build_test.go @@ -53,6 +53,45 @@ func TestFromExpr(t *testing.T) { } } +// 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 @@ func TestBuild(t *testing.T) { }), `"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(