vdl/compile: Modify CompileExpr to support compilation of package-path
qualified identifiers.
CompileExpr used to use a dummy file that didn't have imports that were
compiled into Env.
This caused compilation of package-path quailified identifiers to fail.
We solved this by introducing ExtractExprPackagePaths that allows users to
retrieve all package paths in expressions. Then CompileExpr will include
all needed expr that are also compiled into Env in the file.
Now CompileExprs will succeed in compiling "a/b/c".Foo if "a/b/c" is
in the Env.
Change-Id: I100bf7378b76bb6a4c5b6518774138b31caf94d0
diff --git a/cmd/principal/caveat.go b/cmd/principal/caveat.go
index aa77faa..30df4a7 100644
--- a/cmd/principal/caveat.go
+++ b/cmd/principal/caveat.go
@@ -6,13 +6,13 @@
import (
"fmt"
- "regexp"
"strings"
"v.io/v23/security"
"v.io/v23/vdl"
"v.io/x/ref/lib/vdl/build"
"v.io/x/ref/lib/vdl/compile"
+ "v.io/x/ref/lib/vdl/parse"
)
// caveatsFlag defines a flag.Value for receiving multiple caveat definitions.
@@ -21,7 +21,7 @@
}
type caveatInfo struct {
- pkg, expr, params string
+ expr, params string
}
// Implements flag.Value.Get
@@ -37,21 +37,8 @@
if len(exprAndParam) != 2 {
return fmt.Errorf("incorrect caveat format: %s", s)
}
- expr, param := exprAndParam[0], exprAndParam[1]
- // If the caveatExpr is of the form "path/to/package".ConstName we
- // need to extract the package to later obtain the imports.
- var pkg string
- pkgre, err := regexp.Compile(`\A"(.*)"\.`)
- if err != nil {
- return fmt.Errorf("failed to parse pkgregex: %v", err)
- }
- match := pkgre.FindStringSubmatch(expr)
- if len(match) == 2 {
- pkg = match[1]
- }
-
- c.caveatInfos = append(c.caveatInfos, caveatInfo{pkg, expr, param})
+ c.caveatInfos = append(c.caveatInfos, caveatInfo{exprAndParam[0], exprAndParam[1]})
return nil
}
@@ -68,19 +55,11 @@
if len(c.caveatInfos) == 0 {
return nil, nil
}
- var caveats []security.Caveat
env := compile.NewEnv(-1)
-
- var pkgs []string
- for _, info := range c.caveatInfos {
- if len(info.pkg) > 0 {
- pkgs = append(pkgs, info.pkg)
- }
- }
- if err := buildPackages(pkgs, env); err != nil {
+ if err := buildPackages(c.caveatInfos, env); err != nil {
return nil, err
}
-
+ var caveats []security.Caveat
for _, info := range c.caveatInfos {
caveat, err := newCaveat(info, env)
if err != nil {
@@ -91,8 +70,34 @@
return caveats, nil
}
+func buildPackages(caveatInfos []caveatInfo, env *compile.Env) error {
+ var (
+ pkgNames []string
+ exprs []string
+ )
+ for _, info := range caveatInfos {
+ exprs = append(exprs, info.expr, info.params)
+ }
+ for _, pexpr := range parse.ParseExprs(strings.Join(exprs, ","), env.Errors) {
+ pkgNames = append(pkgNames, parse.ExtractExprPackagePaths(pexpr)...)
+ }
+ if !env.Errors.IsEmpty() {
+ return fmt.Errorf("can't build expressions %v:\n%v", exprs, env.Errors)
+ }
+ pkgs := build.TransitivePackages(pkgNames, build.UnknownPathIsError, build.Opts{}, env.Errors)
+ if !env.Errors.IsEmpty() {
+ return fmt.Errorf("failed to get transitive packages packages %v: %s", pkgNames, env.Errors)
+ }
+ for _, p := range pkgs {
+ if build.BuildPackage(p, env); !env.Errors.IsEmpty() {
+ return fmt.Errorf("failed to build package(%v): %v", p, env.Errors)
+ }
+ }
+ return nil
+}
+
func newCaveat(info caveatInfo, env *compile.Env) (security.Caveat, error) {
- caveatDesc, err := compileCaveatDesc(info.pkg, info.expr, env)
+ caveatDesc, err := compileCaveatDesc(info.expr, env)
if err != nil {
return security.Caveat{}, err
}
@@ -100,61 +105,31 @@
if err != nil {
return security.Caveat{}, err
}
-
return security.NewCaveat(caveatDesc, param)
}
+func compileCaveatDesc(expr string, env *compile.Env) (security.CaveatDescriptor, error) {
+ vdlValues := build.BuildExprs(expr, []*vdl.Type{vdl.TypeOf(security.CaveatDescriptor{})}, env)
+ if err := env.Errors.ToError(); err != nil {
+ return security.CaveatDescriptor{}, fmt.Errorf("can't build caveat desc %s:\n%v", expr, err)
+ }
+ if len(vdlValues) == 0 {
+ return security.CaveatDescriptor{}, fmt.Errorf("no caveat descriptors were built")
+ }
+ var desc security.CaveatDescriptor
+ if err := vdl.Convert(&desc, vdlValues[0]); err != nil {
+ return security.CaveatDescriptor{}, err
+ }
+ return desc, nil
+}
+
func compileParams(paramData string, vdlType *vdl.Type, env *compile.Env) (interface{}, error) {
params := build.BuildExprs(paramData, []*vdl.Type{vdlType}, env)
- if err := env.Errors.ToError(); err != nil {
- return nil, fmt.Errorf("can't parse param data %s:\n%v", paramData, err)
- }
-
- return params[0], nil
-}
-
-func buildPackages(packages []string, env *compile.Env) error {
- pkgs := build.TransitivePackages(packages, build.UnknownPathIsError, build.Opts{}, env.Errors)
if !env.Errors.IsEmpty() {
- return fmt.Errorf("failed to get transitive packages packages: %s", env.Errors)
+ return nil, fmt.Errorf("can't build param data %s:\n%v", paramData, env.Errors)
}
- for _, p := range pkgs {
- build.BuildPackage(p, env)
- if !env.Errors.IsEmpty() {
- return fmt.Errorf("failed to build package(%v): %s", p, env.Errors)
- }
+ if len(params) == 0 {
+ return security.CaveatDescriptor{}, fmt.Errorf("no caveat params were built")
}
- return nil
-}
-
-func compileCaveatDesc(pkg, expr string, env *compile.Env) (security.CaveatDescriptor, error) {
- var vdlValue *vdl.Value
- // In the case that the expr is of the form "path/to/package".ConstName we need to get the
- // resolve the const name instead of building the expressions.
- // TODO(suharshs,toddw): We need to fix BuildExprs so that both these cases will work with it.
- // The issue is that BuildExprs returns a dummy file with no imports instead and errors out instead of
- // looking at the imports in the env.
- if len(pkg) > 0 {
- spl := strings.SplitN(expr, "\".", 2)
- constdef := env.ResolvePackage(pkg).ResolveConst(spl[1])
- if constdef == nil {
- return security.CaveatDescriptor{}, fmt.Errorf("failed to find caveat %v", expr)
- }
- vdlValue = constdef.Value
- } else {
- vdlValues := build.BuildExprs(expr, []*vdl.Type{vdl.TypeOf(security.CaveatDescriptor{})}, env)
- if err := env.Errors.ToError(); err != nil {
- return security.CaveatDescriptor{}, fmt.Errorf("can't build caveat desc %s:\n%v", expr, err)
- }
- if len(vdlValues) == 0 {
- return security.CaveatDescriptor{}, fmt.Errorf("no caveat descriptors were built")
- }
- vdlValue = vdlValues[0]
- }
-
- var desc security.CaveatDescriptor
- if err := vdl.Convert(&desc, vdlValue); err != nil {
- return security.CaveatDescriptor{}, err
- }
- return desc, nil
+ return params[0], nil
}
diff --git a/lib/vdl/compile/compile.go b/lib/vdl/compile/compile.go
index dd8cb1d..c4bd775 100644
--- a/lib/vdl/compile/compile.go
+++ b/lib/vdl/compile/compile.go
@@ -92,10 +92,16 @@
// that expr depends on must already have been compiled and populated into env.
// If t is non-nil, the returned value will be of that type.
func CompileExpr(t *vdl.Type, expr parse.ConstExpr, env *Env) *vdl.Value {
- // Set up a dummy file and compile expr into a value.
file := &File{
BaseName: "_expr.vdl",
Package: newPackage("_expr", "_expr", "_expr", vdltool.Config{}),
+ imports: make(map[string]*importPath),
+ }
+ // Add imports to the "File" if the are in env and used in the Expression.
+ for _, pkg := range parse.ExtractExprPackagePaths(expr) {
+ if env.pkgs[pkg] != nil {
+ file.imports[pkg] = &importPath{path: pkg}
+ }
}
return compileConst("expression", t, expr, file, env)
}
diff --git a/lib/vdl/compile/compile_test.go b/lib/vdl/compile/compile_test.go
index 63bfe8e..76164b5 100644
--- a/lib/vdl/compile/compile_test.go
+++ b/lib/vdl/compile/compile_test.go
@@ -9,6 +9,7 @@
"path"
"testing"
+ "v.io/v23/vdl"
"v.io/x/ref/lib/vdl/build"
"v.io/x/ref/lib/vdl/compile"
"v.io/x/ref/lib/vdl/internal/vdltest"
@@ -45,6 +46,32 @@
}
}
+func TestParseAndCompileExprs(t *testing.T) {
+ env := compile.NewEnv(-1)
+ path := path.Join("a/b/test1")
+ buildPkg := vdltest.FakeBuildPackage("test1", path, f{"1.vdl": pkg1file1, "2.vdl": pkg1file2})
+ pkg := build.BuildPackage(buildPkg, env)
+ if pkg == nil {
+ t.Fatal("failed to build package")
+ }
+ // Test that expressions from the built packages compile correctly.
+ scalarsType := env.ResolvePackage("a/b/test1").ResolveType("Scalars").Type
+ exprTests := []struct {
+ data string
+ vtype *vdl.Type
+ }{
+ {`"a/b/test1".Cint64`, vdl.Int64Type},
+ {`"a/b/test1".FiveSquared + "a/b/test1".Cint32`, vdl.Int32Type},
+ {`"a/b/test1".Scalars{A:true,C:"a/b/test1".FiveSquared+"a/b/test1".Cint32}`, scalarsType},
+ }
+ for _, test := range exprTests {
+ vals := build.BuildExprs(test.data, []*vdl.Type{test.vtype}, env)
+ if !env.Errors.IsEmpty() || len(vals) != 1 {
+ t.Errorf("failed to build %v: %v", test.data, env.Errors)
+ }
+ }
+}
+
const pkg1file1 = `package test1
type Scalars struct {
diff --git a/lib/vdl/parse/const.go b/lib/vdl/parse/const.go
index b15aaf6..6c8f887 100644
--- a/lib/vdl/parse/const.go
+++ b/lib/vdl/parse/const.go
@@ -10,6 +10,12 @@
"strconv"
)
+// ConstDef represents a user-defined named const.
+type ConstDef struct {
+ NamePos
+ Expr ConstExpr
+}
+
// ConstExpr is the interface for all nodes in an expression.
type ConstExpr interface {
String() string
@@ -84,12 +90,6 @@
P Pos
}
-// ConstDef represents a user-defined named const.
-type ConstDef struct {
- NamePos
- Expr ConstExpr
-}
-
// cvString returns a human-readable string representing the const value.
func cvString(val interface{}) string {
switch tv := val.(type) {
diff --git a/lib/vdl/parse/parse.go b/lib/vdl/parse/parse.go
index c4f8394..75fa6c7 100644
--- a/lib/vdl/parse/parse.go
+++ b/lib/vdl/parse/parse.go
@@ -114,6 +114,81 @@
return lex.exprs
}
+// ExtractExprPackagePaths returns any package paths that appear in named constants
+// in expr. i.e. "a/b/c".Foo => "a/b/c".
+func ExtractExprPackagePaths(expr ConstExpr) []string {
+ var paths []string
+ switch e := expr.(type) {
+ case *ConstNamed:
+ if path := packageFromName(e.Name); len(path) > 0 {
+ paths = append(paths, path)
+ }
+ case *ConstCompositeLit:
+ for _, kv := range e.KVList {
+ paths = append(paths, ExtractExprPackagePaths(kv.Key)...)
+ paths = append(paths, ExtractExprPackagePaths(kv.Value)...)
+ }
+ paths = append(paths, ExtractTypePackagePaths(e.Type)...)
+ case *ConstIndexed:
+ paths = append(paths, ExtractExprPackagePaths(e.Expr)...)
+ paths = append(paths, ExtractExprPackagePaths(e.IndexExpr)...)
+ case *ConstTypeConv:
+ paths = append(paths, ExtractTypePackagePaths(e.Type)...)
+ paths = append(paths, ExtractExprPackagePaths(e.Expr)...)
+ case *ConstTypeObject:
+ paths = append(paths, ExtractTypePackagePaths(e.Type)...)
+ case *ConstBinaryOp:
+ paths = append(paths, ExtractExprPackagePaths(e.Lexpr)...)
+ paths = append(paths, ExtractExprPackagePaths(e.Rexpr)...)
+ case *ConstUnaryOp:
+ paths = append(paths, ExtractExprPackagePaths(e.Expr)...)
+ default:
+ // leaf expression with no embedded expressions or types.
+ }
+ return paths
+}
+
+func ExtractTypePackagePaths(typ Type) []string {
+ var paths []string
+ switch t := typ.(type) {
+ case *TypeNamed:
+ if path := packageFromName(t.Name); len(path) > 0 {
+ paths = append(paths, path)
+ }
+ case *TypeArray:
+ paths = append(paths, ExtractTypePackagePaths(t.Elem)...)
+ case *TypeList:
+ paths = append(paths, ExtractTypePackagePaths(t.Elem)...)
+ case *TypeSet:
+ paths = append(paths, ExtractTypePackagePaths(t.Key)...)
+ case *TypeMap:
+ paths = append(paths, ExtractTypePackagePaths(t.Key)...)
+ paths = append(paths, ExtractTypePackagePaths(t.Elem)...)
+ case *TypeStruct:
+ for _, f := range t.Fields {
+ paths = append(paths, ExtractTypePackagePaths(f.Type)...)
+ }
+ case *TypeUnion:
+ for _, f := range t.Fields {
+ paths = append(paths, ExtractTypePackagePaths(f.Type)...)
+ }
+ case *TypeOptional:
+ paths = append(paths, ExtractTypePackagePaths(t.Base)...)
+ default:
+ // leaf type with no embedded types.
+ }
+ return paths
+}
+
+func packageFromName(name string) string {
+ if strings.HasPrefix(name, `"`) {
+ if parts := strings.SplitN(name[1:], `".`, 2); len(parts) == 2 {
+ return parts[0]
+ }
+ }
+ return ""
+}
+
// lexer implements the yyLexer interface for the yacc-generated parser.
//
// An oddity: lexer also holds the result of the parse. Most yacc examples hold
diff --git a/lib/vdl/parse/parse_test.go b/lib/vdl/parse/parse_test.go
index 90849e6..6e3ba87 100644
--- a/lib/vdl/parse/parse_test.go
+++ b/lib/vdl/parse/parse_test.go
@@ -1375,6 +1375,7 @@
{`true`, []parse.ConstExpr{cn("true", 1, 1)}, ""},
{`false`, []parse.ConstExpr{cn("false", 1, 1)}, ""},
{`abc`, []parse.ConstExpr{cn("abc", 1, 1)}, ""},
+ {`"a/b/c".abc`, []parse.ConstExpr{cn(`"a/b/c".abc`, 1, 1)}, ""},
{`"abc"`, []parse.ConstExpr{cl("abc", 1, 1)}, ""},
{`1`, []parse.ConstExpr{cl(big.NewInt(1), 1, 1)}, ""},
{`123`, []parse.ConstExpr{cl(big.NewInt(123), 1, 1)}, ""},
diff --git a/lib/vdl/parse/type.go b/lib/vdl/parse/type.go
index d72359a..8493edf 100644
--- a/lib/vdl/parse/type.go
+++ b/lib/vdl/parse/type.go
@@ -8,6 +8,12 @@
"fmt"
)
+// TypeDef represents a user-defined named type.
+type TypeDef struct {
+ NamePos // name assigned by the user, pos and doc
+ Type Type // the underlying type of the type definition.
+}
+
// Type is an interface representing symbolic occurrences of types in VDL files.
type Type interface {
// String returns a human-readable description of the type.
@@ -75,12 +81,6 @@
P Pos
}
-// TypeDef represents a user-defined named type.
-type TypeDef struct {
- NamePos // name assigned by the user, pos and doc
- Type Type // the underlying type of the type definition.
-}
-
func (t *TypeNamed) Pos() Pos { return t.P }
func (t *TypeEnum) Pos() Pos { return t.P }
func (t *TypeArray) Pos() Pos { return t.P }
