Include expression id on parser and checker errors (#725)

* Introduce an expression ID to the common.Error value and surface the common.Error type as cel.Error
* Additional tests which validate expression ids are attached to parse / check issues when possible

cel/cel_test.go

@@ -29,7 +29,6 @@ import (
 	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"github.com/google/cel-go/checker"
-	"github.com/google/cel-go/common"
 	"github.com/google/cel-go/common/operators"
 	"github.com/google/cel-go/common/overloads"
 	"github.com/google/cel-go/common/types"
@@ -773,7 +772,7 @@ func TestMacroSubset(t *testing.T) {
 
 func TestCustomMacro(t *testing.T) {
 	joinMacro := NewReceiverMacro("join", 1,
-		func(meh MacroExprHelper, iterRange *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+		func(meh MacroExprHelper, iterRange *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 			delim := args[0]
 			iterIdent := meh.Ident("__iter__")
 			accuIdent := meh.AccuIdent()
@@ -820,7 +819,7 @@ func TestCustomExistsMacro(t *testing.T) {
 		Variable("attr", MapType(StringType, BoolType)),
 		Macros(
 			NewGlobalVarArgMacro("kleeneOr",
-				func(meh MacroExprHelper, unused *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+				func(meh MacroExprHelper, unused *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 					inputs := meh.NewList(args...)
 					eqOne, err := ExistsMacroExpander(meh, inputs, []*exprpb.Expr{
 						meh.Ident("__iter__"),
@@ -850,7 +849,7 @@ func TestCustomExistsMacro(t *testing.T) {
 				},
 			),
 			NewGlobalMacro("kleeneEq", 2,
-				func(meh MacroExprHelper, unused *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+				func(meh MacroExprHelper, unused *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 					attr := args[0]
 					value := args[1]
 					hasAttr, err := HasMacroExpander(meh, nil, []*exprpb.Expr{meh.Copy(attr)})

cel/env.go

@@ -596,6 +596,9 @@ func (e *Env) maybeApplyFeature(feature int, option EnvOption) (*Env, error) {
 	return e, nil
 }
 
+// Error type which references an expression id, a location within source, and a message.
+type Error = common.Error
+
 // Issues defines methods for inspecting the error details of parse and check calls.
 //
 // Note: in the future, non-fatal warnings and notices may be inspectable via the Issues struct.
@@ -622,9 +625,9 @@ func (i *Issues) Err() error {
 }
 
 // Errors returns the collection of errors encountered in more granular detail.
-func (i *Issues) Errors() []common.Error {
+func (i *Issues) Errors() []*Error {
 	if i == nil {
-		return []common.Error{}
+		return []*Error{}
 	}
 	return i.errs.GetErrors()
 }

cel/library.go

@@ -20,7 +20,6 @@ import (
 	"time"
 
 	"github.com/google/cel-go/checker"
-	"github.com/google/cel-go/common"
 	"github.com/google/cel-go/common/operators"
 	"github.com/google/cel-go/common/overloads"
 	"github.com/google/cel-go/common/types"
@@ -206,17 +205,14 @@ func (optionalLibrary) CompileOptions() []EnvOption {
 	}
 }
 
-func optMap(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func optMap(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	varIdent := args[0]
 	varName := ""
 	switch varIdent.GetExprKind().(type) {
 	case *exprpb.Expr_IdentExpr:
 		varName = varIdent.GetIdentExpr().GetName()
 	default:
-		return nil, &common.Error{
-			Message:  "optMap() variable name must be a simple identifier",
-			Location: meh.OffsetLocation(varIdent.GetId()),
-		}
+		return nil, meh.NewError(varIdent.GetId(), "optMap() variable name must be a simple identifier")
 	}
 	mapExpr := args[1]
 	return meh.GlobalCall(

cel/macro.go

@@ -15,7 +15,6 @@
 package cel
 
 import (
-	"github.com/google/cel-go/common"
 	"github.com/google/cel-go/parser"
 
 	exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
@@ -63,21 +62,21 @@ func NewReceiverVarArgMacro(function string, expander MacroExpander) Macro {
 }
 
 // HasMacroExpander expands the input call arguments into a presence test, e.g. has(<operand>.field)
-func HasMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func HasMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	return parser.MakeHas(meh, target, args)
 }
 
 // ExistsMacroExpander expands the input call arguments into a comprehension that returns true if any of the
 // elements in the range match the predicate expressions:
 // <iterRange>.exists(<iterVar>, <predicate>)
-func ExistsMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func ExistsMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	return parser.MakeExists(meh, target, args)
 }
 
 // ExistsOneMacroExpander expands the input call arguments into a comprehension that returns true if exactly
 // one of the elements in the range match the predicate expressions:
 // <iterRange>.exists_one(<iterVar>, <predicate>)
-func ExistsOneMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func ExistsOneMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	return parser.MakeExistsOne(meh, target, args)
 }
 
@@ -91,14 +90,14 @@ func ExistsOneMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*ex
 //
 // In the second form only iterVar values which return true when provided to the predicate expression
 // are transformed.
-func MapMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func MapMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	return parser.MakeMap(meh, target, args)
 }
 
 // FilterMacroExpander expands the input call arguments into a comprehension which produces a list which contains
 // only elements which match the provided predicate expression:
 // <iterRange>.filter(<iterVar>, <predicate>)
-func FilterMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *common.Error) {
+func FilterMacroExpander(meh MacroExprHelper, target *exprpb.Expr, args []*exprpb.Expr) (*exprpb.Expr, *Error) {
 	return parser.MakeFilter(meh, target, args)
 }