Allow macros to return an empty Expr to indicate a non-expansion (#605)

* Allow macros to return an empty Expr to indicate a non-expansion at the implementation level

* Switch from empty Expr to nil expr to indicate non-expansion

cel/macro.go

@@ -27,8 +27,11 @@ import (
 // a Macro should be created per arg-count or as a var arg macro.
 type Macro = parser.Macro
 
-// MacroExpander converts a call and its associated arguments into a new CEL abstract syntax tree, or an error
-// if the input arguments are not suitable for the expansion requirements for the macro in question.
+// MacroExpander converts a call and its associated arguments into a new CEL abstract syntax tree.
+//
+// If the MacroExpander determines within the implementation that an expansion is not needed it may return
+// a nil Expr value to indicate a non-match. However, if an expansion is to be performed, but the arguments
+// are not well-formed, the result of the expansion will be an error.
 //
 // The MacroExpander accepts as arguments a MacroExprHelper as well as the arguments used in the function call
 // and produces as output an Expr ast node.

ext/protos.go

@@ -80,8 +80,8 @@ func (protoLib) ProgramOptions() []cel.ProgramOption {
 
 // hasProtoExt generates a test-only select expression for a fully-qualified extension name on a protobuf message.
 func hasProtoExt(meh cel.MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
-	if call, isExt := isExtCall(meh, hasExtension, target, args); !isExt {
-		return call, nil
+	if !isExtCall(meh, hasExtension, target, args) {
+		return nil, nil
 	}
 	extensionField, err := getExtFieldName(meh, args[1])
 	if err != nil {
@@ -92,8 +92,8 @@ func hasProtoExt(meh cel.MacroExprHelper, target *exprpb.Expr, args []*exprpb.Ex
 
 // getProtoExt generates a select expression for a fully-qualified extension name on a protobuf message.
 func getProtoExt(meh cel.MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
-	if call, isExt := isExtCall(meh, getExtension, target, args); !isExt {
-		return call, nil
+	if !isExtCall(meh, getExtension, target, args) {
+		return nil, nil
 	}
 	extFieldName, err := getExtFieldName(meh, args[1])
 	if err != nil {
@@ -102,16 +102,16 @@ func getProtoExt(meh cel.MacroExprHelper, target *exprpb.Expr, args []*exprpb.Ex
 	return meh.Select(args[0], extFieldName), nil
 }
 
-func isExtCall(meh cel.MacroExprHelper, fnName string, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, bool) {
+func isExtCall(meh cel.MacroExprHelper, fnName string, target *exprpb.Expr, args []*exprpb.Expr) bool {
 	switch target.GetExprKind().(type) {
 	case *exprpb.Expr_IdentExpr:
 		if target.GetIdentExpr().GetName() != protoNamespace {
-			return meh.ReceiverCall(fnName, target, args...), false
+			return false
 		}
+		return true
 	default:
-		return meh.ReceiverCall(fnName, target, args...), false
+		return false
 	}
-	return nil, true
 }
 
 func getExtFieldName(meh cel.MacroExprHelper, expr *exprpb.Expr) (string, *common.Error) {

parser/macro.go

@@ -132,8 +132,11 @@ func makeVarArgMacroKey(name string, receiverStyle bool) string {
 	return fmt.Sprintf("%s:*:%v", name, receiverStyle)
 }
 
-// MacroExpander converts a call and its associated arguments into a new CEL abstract syntax tree, or an error
-// if the input arguments are not suitable for the expansion requirements for the macro in question.
+// MacroExpander converts a call and its associated arguments into a new CEL abstract syntax tree.
+//
+// If the MacroExpander determines within the implementation that an expansion is not needed it may return
+// a nil Expr value to indicate a non-match. However, if an expansion is to be performed, but the arguments
+// are not well-formed, the result of the expansion will be an error.
 //
 // The MacroExpander accepts as arguments a MacroExprHelper as well as the arguments used in the function call
 // and produces as output an Expr ast node.

parser/parser.go

@@ -873,12 +873,18 @@ func (p *parser) expandMacro(exprID int64, function string, target *exprpb.Expr,
 	eh.parserHelper = p.helper
 	eh.id = exprID
 	expr, err := macro.Expander()(eh, target, args)
+	// An error indicates that the macro was matched, but the arguments were not well-formed.
 	if err != nil {
 		if err.Location != nil {
 			return p.reportError(err.Location, err.Message), true
 		}
 		return p.reportError(p.helper.getLocation(exprID), err.Message), true
 	}
+	// A nil value from the macro indicates that the macro implementation decided that
+	// an expansion should not be performed.
+	if expr == nil {
+		return nil, false
+	}
 	if p.populateMacroCalls {
 		p.helper.addMacroCall(expr.GetId(), function, target, args...)
 	}

parser/parser_test.go

@@ -1650,6 +1650,18 @@ var testCases = []testInfo{
 			)^#7:*expr.Expr_CallExpr#
 		  )^#9:*expr.Expr_CallExpr#`,
 	},
+	{
+		I: `noop_macro(123)`,
+		Opts: []Option{
+			Macros(NewGlobalVarArgMacro("noop_macro",
+				func(eh ExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+					return nil, nil
+				})),
+		},
+		P: `noop_macro(
+			123^#2:*expr.Constant_Int64Value#
+		  )^#1:*expr.Expr_CallExpr#`,
+	},
 }
 
 type testInfo struct {