Repository: openllb/hlb
Pull request #124 (2020-04-26 20:47:28)

Allow option funcs to be used from imported
- Previously didn't allow `run "echo foo" with myimport.foo`
- We need some way of running codegen tests with imports, but going to take more work since we need a sandboxed buildkitd or some small mock framework for imports.


Patch:
diff --git a/checker/checker.go b/checker/checker.go
index 2089eef..a9c2dd9 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -200,8 +200,6 @@ func (c *checker) CheckSelectors(mod *parser.Module) error {
 			obj := mod.Scope.Lookup(n.Name())
 			if obj.Kind == parser.DeclKind {
 				if _, ok := obj.Node.(*parser.ImportDecl); ok {
-					importScope := obj.Data.(*parser.Scope)
-
 					typ := fun.Type.ObjType
 					if typ == parser.Option {
 						// Inherit the secondary type from the calling function name.
@@ -210,7 +208,7 @@ func (c *checker) CheckSelectors(mod *parser.Module) error {
 
 					// Check call signature against the imported module's scope since it was
 					// declared there.
-					params, err := c.checkCallSignature(importScope, typ, n, args)
+					params, err := c.checkCallSignature(mod.Scope, typ, n, args)
 					if err != nil {
 						c.errs = append(c.errs, err)
 						return false
@@ -300,7 +298,7 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
 		}
 
 		call := stmt.Call
-		if call.Func == nil || (call.Func.Ident != nil && call.Func.Ident.Name == "breakpoint") {
+		if call.Func == nil || call.Func.Name() == "breakpoint" {
 			continue
 		}
 
@@ -343,7 +341,7 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
 		if !ok {
 			obj := scope.Lookup(name)
 			if obj == nil {
-				return ErrIdentNotDefined{Ident: call.Func.Ident}
+				return ErrIdentNotDefined{Ident: call.Func.IdentNode()}
 			}
 
 			// The retrieved object may be either a function declaration or a field
@@ -357,7 +355,7 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
 				case *parser.AliasDecl:
 					callType = n.Func.Type
 				case *parser.ImportDecl:
-					c.errs = append(c.errs, ErrUseModuleWithoutSelector{Ident: call.Func.Ident})
+					c.errs = append(c.errs, ErrUseModuleWithoutSelector{Ident: call.Func.IdentNode()})
 					continue
 				}
 			case parser.FieldKind:
@@ -383,6 +381,10 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
 }
 
 func (c *checker) checkCallStmt(scope *parser.Scope, typ parser.ObjType, call *parser.CallStmt) error {
+	if call.Func.Selector != nil {
+		return nil
+	}
+
 	params, err := c.checkCallSignature(scope, typ, call.Func, call.Args)
 	if err != nil {
 		return err
@@ -392,30 +394,18 @@ func (c *checker) checkCallStmt(scope *parser.Scope, typ parser.ObjType, call *p
 }
 
 func (c *checker) checkCallSignature(scope *parser.Scope, typ parser.ObjType, expr *parser.Expr, args []*parser.Expr) ([]*parser.Field, error) {
-	var ident *parser.Ident
-	switch {
-	case expr.Ident != nil:
-		ident = expr.Ident
-	case expr.Selector != nil:
-		ident = expr.Selector.Select
-	}
-
 	var signature []*parser.Field
-	fun, ok := builtin.Lookup.ByType[typ].Func[ident.Name]
+	fun, ok := builtin.Lookup.ByType[typ].Func[expr.Name()]
 	if !ok && typ == parser.Group {
-		fun, ok = builtin.Lookup.ByType[parser.Filesystem].Func[ident.Name]
+		fun, ok = builtin.Lookup.ByType[parser.Filesystem].Func[expr.Name()]
 	}
 
 	if ok {
 		signature = fun.Params
 	} else {
-		obj := scope.Lookup(ident.Name)
+		obj := scope.Lookup(expr.Name())
 		if obj == nil {
-			return nil, ErrIdentUndefined{ident}
-		}
-
-		if expr.Selector != nil && !obj.Exported {
-			return nil, ErrCallUnexported{expr.Selector}
+			return nil, ErrIdentUndefined{expr.IdentNode()}
 		}
 
 		if obj.Kind == parser.DeclKind {
@@ -425,7 +415,23 @@ func (c *checker) checkCallSignature(scope *parser.Scope, typ parser.ObjType, ex
 			case *parser.AliasDecl:
 				signature = n.Func.Params.List
 			case *parser.ImportDecl:
-				panic("todo: ErrCallImport")
+				importScope := obj.Data.(*parser.Scope)
+				importObj := importScope.Lookup(expr.Selector.Select.Name)
+				if importObj == nil {
+					return nil, ErrIdentUndefined{expr.Selector.Select}
+				}
+				if !importObj.Exported {
+					return nil, ErrCallUnexported{expr.Selector}
+				}
+
+				switch m := importObj.Node.(type) {
+				case *parser.FuncDecl:
+					signature = m.Params.List
+				case *parser.AliasDecl:
+					signature = m.Func.Params.List
+				default:
+					panic("implementation error")
+				}
 			default:
 				panic("implementation error")
 			}
diff --git a/codegen/chain.go b/codegen/chain.go
index b066308..d084456 100644
--- a/codegen/chain.go
+++ b/codegen/chain.go
@@ -82,6 +82,10 @@ func (cg *CodeGen) EmitFilesystemChainStmt(ctx context.Context, scope *parser.Sc
 		case *parser.ImportDecl:
 			importScope := obj.Data.(*parser.Scope)
 			importObj := importScope.Lookup(expr.Selector.Select.Name)
+			if importObj == nil {
+				return nil, errors.WithStack(ErrCodeGen{expr, errors.Errorf("could not find reference")})
+			}
+
 			switch m := importObj.Node.(type) {
 			case *parser.FuncDecl:
 				v, err = cg.EmitFuncDecl(ctx, scope, m, args, ac, chainStart)
@@ -314,7 +318,7 @@ func (cg *CodeGen) EmitFilesystemBuiltinChainStmt(ctx context.Context, scope *pa
 				// to be in the context of a specific function run is in.
 			case with.Expr.FuncLit != nil:
 				for _, stmt := range with.Expr.FuncLit.Body.NonEmptyStmts() {
-					if stmt.Call.Func.Ident.Name != "mount" || stmt.Call.Alias == nil {
+					if stmt.Call.Func.Name() != "mount" || stmt.Call.Alias == nil {
 						continue
 					}
 
diff --git a/codegen/codegen.go b/codegen/codegen.go
index 24d4db4..ea665d2 100644
--- a/codegen/codegen.go
+++ b/codegen/codegen.go
@@ -285,7 +285,7 @@ type aliasCallback func(*parser.CallStmt, interface{}) bool
 func noopAliasCallback(_ *parser.CallStmt, _ interface{}) bool { return true }
 
 func isBreakpoint(call *parser.CallStmt) bool {
-	return call.Func.Ident != nil && call.Func.Ident.Name == "breakpoint"
+	return call.Func.Name() == "breakpoint"
 }
 
 func (cg *CodeGen) EmitBlock(ctx context.Context, scope *parser.Scope, typ parser.ObjType, stmts []*parser.Stmt, ac aliasCallback, chainStart interface{}) (interface{}, error) {
@@ -435,7 +435,7 @@ func (cg *CodeGen) EmitImageOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "resolve":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
@@ -463,6 +463,15 @@ func (cg *CodeGen) EmitOptionLookup(ctx context.Context, scope *parser.Scope, ex
 		switch n := obj.Node.(type) {
 		case *parser.FuncDecl:
 			return cg.EmitOptionFuncDecl(ctx, scope, n, args)
+		case *parser.ImportDecl:
+			importScope := obj.Data.(*parser.Scope)
+			importObj := importScope.Lookup(expr.Selector.Select.Name)
+			switch m := importObj.Node.(type) {
+			case *parser.FuncDecl:
+				return cg.EmitOptionFuncDecl(ctx, scope, m, args)
+			default:
+				return opts, errors.WithStack(ErrCodeGen{expr, errors.Errorf("unknown option decl kind")})
+			}
 		default:
 			return opts, errors.WithStack(ErrCodeGen{expr, errors.Errorf("unknown option decl kind")})
 		}
@@ -480,7 +489,7 @@ func (cg *CodeGen) EmitHTTPOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "checksum":
 				dgst, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -515,7 +524,7 @@ func (cg *CodeGen) EmitGitOptions(ctx context.Context, scope *parser.Scope, op s
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "keepGitDir":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
@@ -540,7 +549,7 @@ func (cg *CodeGen) EmitLocalOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "includePatterns":
 				patterns := make([]string, len(args))
 				for i, arg := range args {
@@ -598,7 +607,7 @@ func (cg *CodeGen) EmitFrontendOptions(ctx context.Context, scope *parser.Scope,
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "input":
 				key, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -639,7 +648,7 @@ func (cg *CodeGen) EmitMkdirOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "createParents":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
@@ -680,7 +689,7 @@ func (cg *CodeGen) EmitMkfileOptions(ctx context.Context, scope *parser.Scope, o
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "chown":
 				owner, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -715,7 +724,7 @@ func (cg *CodeGen) EmitRmOptions(ctx context.Context, scope *parser.Scope, op st
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "allowNotFound":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
@@ -801,7 +810,7 @@ func (cg *CodeGen) EmitCopyOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "followSymlinks":
 				follow, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
@@ -882,7 +891,7 @@ func (cg *CodeGen) EmitTemplateOptions(ctx context.Context, scope *parser.Scope,
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "stringField":
 				name, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -1307,7 +1316,7 @@ func (cg *CodeGen) EmitSSHOptions(ctx context.Context, scope *parser.Scope, op s
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "target":
 				target, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -1399,7 +1408,7 @@ func (cg *CodeGen) EmitSecretOptions(ctx context.Context, scope *parser.Scope, o
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "id":
 				id, err := cg.EmitStringExpr(ctx, scope, args[0])
 				if err != nil {
@@ -1476,7 +1485,7 @@ func (cg *CodeGen) EmitMountOptions(ctx context.Context, scope *parser.Scope, op
 	for _, stmt := range stmts {
 		if stmt.Call != nil {
 			args := stmt.Call.Args
-			switch stmt.Call.Func.Ident.Name {
+			switch stmt.Call.Func.Name() {
 			case "readonly":
 				v, err := cg.MaybeEmitBoolExpr(ctx, scope, args)
 				if err != nil {
diff --git a/codegen/debug.go b/codegen/debug.go
index 7e12a6a..5e06ead 100644
--- a/codegen/debug.go
+++ b/codegen/debug.go
@@ -149,7 +149,7 @@ func NewDebugger(c *client.Client, w io.Writer, r *bufio.Reader, ibs map[string]
 								Func: n,
 							}
 						case *parser.CallStmt:
-							if n.Func.Ident.Name == "breakpoint" {
+							if n.Func.Name() == "breakpoint" {
 								fmt.Fprintf(w, "%s cannot break at breakpoint\n", checker.FormatPos(n.Pos))
 								continue
 							}
diff --git a/parser/cst.go b/parser/cst.go
index 083e950..357bc31 100644
--- a/parser/cst.go
+++ b/parser/cst.go
@@ -357,13 +357,17 @@ func (e *Expr) End() lexer.Position {
 }
 
 func (e *Expr) Name() string {
+	return e.IdentNode().Name
+}
+
+func (e *Expr) IdentNode() *Ident {
 	switch {
 	case e.Selector != nil:
-		return e.Selector.Ident.Name
+		return e.Selector.Ident
 	case e.Ident != nil:
-		return e.Ident.Name
+		return e.Ident
 	default:
-		return ""
+		return &Ident{}
 	}
 }
 


Test patch:
diff --git a/checker/checker_test.go b/checker/checker_test.go
index 02d9c23..8771ef7 100644
--- a/checker/checker_test.go
+++ b/checker/checker_test.go
@@ -419,14 +419,38 @@ func TestChecker_CheckSelectors(t *testing.T) {
 		}
 		`,
 		nil,
+	}, {
+		"use imported option",
+		`
+		import myImportedModule "./myModule.hlb"
+	
+		fs default(string foo) {
+			image "busybox" with myImportedModule.resolveImage
+		}
+		`,
+		nil,
+	}, {
+		"merge imported option",
+		`
+		import myImportedModule "./myModule.hlb"
+	
+		fs default(string foo) {
+			image "busybox" with option {
+				myImportedModule.resolveImage
+			}
+		}
+		`,
+		nil,
 	}} {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			importedModuleDefinition := `
 				export validSelector
 				export validSelectorWithArgs
+				export resolveImage
 				fs validSelector() {}
 				fs validSelectorWithArgs(string bar) {}
+				option::image resolveImage() { resolve; }
 			`
 
 			importedModule, err := parser.Parse(strings.NewReader(importedModuleDefinition))