The type map references when keyed by attribute may be incorrect. (#361)

* The type map references when keyed by attribute may be incorrect. Fixed to refer to the original expression id.
* Added test for verifying optimization paths being performed correctly

Author: TristonianJones

interpreter/interpreter_test.go

@@ -540,6 +540,47 @@ var (
 				"input": "kathmandu",
 			},
 		},
+		{
+			name:  "nested_proto_field",
+			expr:  `pb3.single_nested_message.bb`,
+			types: []proto.Message{&proto3pb.TestAllTypes{}},
+			env: []*exprpb.Decl{
+				decls.NewVar("pb3",
+					decls.NewObjectType("google.expr.proto3.test.TestAllTypes")),
+			},
+			in: map[string]interface{}{
+				"pb3": &proto3pb.TestAllTypes{
+					NestedType: &proto3pb.TestAllTypes_SingleNestedMessage{
+						SingleNestedMessage: &proto3pb.TestAllTypes_NestedMessage{
+							Bb: 1234,
+						},
+					},
+				},
+			},
+			out: types.Int(1234),
+		},
+		{
+			name:  "nested_proto_field_with_index",
+			expr:  `pb3.map_int64_nested_type[0].child.payload.single_int32 == 1`,
+			types: []proto.Message{&proto3pb.TestAllTypes{}},
+			env: []*exprpb.Decl{
+				decls.NewVar("pb3",
+					decls.NewObjectType("google.expr.proto3.test.TestAllTypes")),
+			},
+			in: map[string]interface{}{
+				"pb3": &proto3pb.TestAllTypes{
+					MapInt64NestedType: map[int64]*proto3pb.NestedTestAllTypes{
+						0: &proto3pb.NestedTestAllTypes{
+							Child: &proto3pb.NestedTestAllTypes{
+								Payload: &proto3pb.TestAllTypes{
+									SingleInt32: 1,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "or_true_1st",
 			expr: `ai == 20 || ar["foo"] == "bar"`,
@@ -1007,6 +1048,53 @@ func TestInterpreter(t *testing.T) {
 	}
 }
 
+func TestInterpreter_ProtoAttributeOpt(t *testing.T) {
+	inst, _, err := program(&testCase{
+		name:  "nested_proto_field_with_index",
+		expr:  `pb3.map_int64_nested_type[0].child.payload.single_int32`,
+		types: []proto.Message{&proto3pb.TestAllTypes{}},
+		env: []*exprpb.Decl{
+			decls.NewVar("pb3",
+				decls.NewObjectType("google.expr.proto3.test.TestAllTypes")),
+		},
+		in: map[string]interface{}{
+			"pb3": &proto3pb.TestAllTypes{
+				MapInt64NestedType: map[int64]*proto3pb.NestedTestAllTypes{
+					0: &proto3pb.NestedTestAllTypes{
+						Child: &proto3pb.NestedTestAllTypes{
+							Payload: &proto3pb.TestAllTypes{
+								SingleInt32: 1,
+							},
+						},
+					},
+				},
+			},
+		},
+	}, Optimize())
+	if err != nil {
+		t.Fatal(err)
+	}
+	attr, ok := inst.(InterpretableAttribute)
+	if !ok {
+		t.Fatalf("got %v, expected attribute value", inst)
+	}
+	absAttr, ok := attr.Attr().(NamespacedAttribute)
+	if !ok {
+		t.Fatalf("got %v, expected global attribute", attr.Attr())
+	}
+	quals := absAttr.Qualifiers()
+	if len(quals) != 5 {
+		t.Errorf("got %d qualifiers, wanted 5", len(quals))
+	}
+	if !isFieldQual(quals[0], "map_int64_nested_type") ||
+		!isConstQual(quals[1], types.IntZero) ||
+		!isFieldQual(quals[2], "child") ||
+		!isFieldQual(quals[3], "payload") ||
+		!isFieldQual(quals[4], "single_int32") {
+		t.Error("unoptimized qualifier types present in optimized attribute")
+	}
+}
+
 func TestInterpreter_LogicalAndMissingType(t *testing.T) {
 	src := common.NewTextSource(`a && TestProto{c: true}.c`)
 	parsed, errors := parser.Parse(src)
@@ -1333,3 +1421,19 @@ func base64_encode(val ref.Val) ref.Val {
 	}
 	return types.String(base64.StdEncoding.EncodeToString([]byte(str)))
 }
+
+func isConstQual(q Qualifier, val ref.Val) bool {
+	c, ok := q.(ConstantQualifier)
+	if !ok {
+		return false
+	}
+	return c.Value().Equal(val) == types.True
+}
+
+func isFieldQual(q Qualifier, fieldName string) bool {
+	f, ok := q.(*fieldQualifier)
+	if !ok {
+		return false
+	}
+	return f.Name == fieldName
+}

interpreter/planner.go

@@ -133,14 +133,14 @@ func (p *planner) decorate(i Interpretable, err error) (Interpretable, error) {
 // planIdent creates an Interpretable that resolves an identifier from an Activation.
 func (p *planner) planIdent(expr *exprpb.Expr) (Interpretable, error) {
 	// Establish whether the identifier is in the reference map.
-	if identRef, found := p.refMap[expr.Id]; found {
-		return p.planCheckedIdent(expr.Id, identRef)
+	if identRef, found := p.refMap[expr.GetId()]; found {
+		return p.planCheckedIdent(expr.GetId(), identRef)
 	}
 	// Create the possible attribute list for the unresolved reference.
 	ident := expr.GetIdentExpr()
 	return &evalAttr{
 		adapter: p.adapter,
-		attr:    p.attrFactory.MaybeAttribute(expr.Id, ident.Name),
+		attr:    p.attrFactory.MaybeAttribute(expr.GetId(), ident.Name),
 	}, nil
 }
 
@@ -178,8 +178,8 @@ func (p *planner) planCheckedIdent(id int64, identRef *exprpb.Reference) (Interp
 func (p *planner) planSelect(expr *exprpb.Expr) (Interpretable, error) {
 	// If the Select id appears in the reference map from the CheckedExpr proto then it is either
 	// a namespaced identifier or enum value.
-	if identRef, found := p.refMap[expr.Id]; found {
-		return p.planCheckedIdent(expr.Id, identRef)
+	if identRef, found := p.refMap[expr.GetId()]; found {
+		return p.planCheckedIdent(expr.GetId(), identRef)
 	}
 
 	sel := expr.GetSelectExpr()
@@ -191,7 +191,7 @@ func (p *planner) planSelect(expr *exprpb.Expr) (Interpretable, error) {
 
 	// Determine the field type if this is a proto message type.
 	var fieldType *ref.FieldType
-	opType := p.typeMap[op.ID()]
+	opType := p.typeMap[sel.GetOperand().GetId()]
 	if opType.GetMessageType() != "" {
 		ft, found := p.provider.FindFieldType(opType.GetMessageType(), sel.Field)
 		if found && ft.IsSet != nil && ft.GetFrom != nil {
@@ -485,7 +485,7 @@ func (p *planner) planCallIndex(expr *exprpb.Expr,
 	}
 	indConst, isIndConst := ind.(InterpretableConst)
 	if isIndConst {
-		opType := p.typeMap[op.ID()]
+		opType := p.typeMap[expr.GetCallExpr().GetTarget().GetId()]
 		qual, err := p.attrFactory.NewQualifier(
 			opType, indConst.ID(), indConst.Value())
 		if err != nil {
@@ -673,7 +673,7 @@ func (p *planner) resolveFunction(expr *exprpb.Expr) (*exprpb.Expr, string, stri
 
 	// Checked expressions always have a reference map entry, and _should_ have the fully qualified
 	// function name as the fnName value.
-	oRef, hasOverload := p.refMap[expr.Id]
+	oRef, hasOverload := p.refMap[expr.GetId()]
 	if hasOverload {
 		if len(oRef.GetOverloadId()) == 1 {
 			return target, fnName, oRef.GetOverloadId()[0]
@@ -727,7 +727,7 @@ func (p *planner) resolveFunction(expr *exprpb.Expr) (*exprpb.Expr, string, stri
 func (p *planner) toQualifiedName(operand *exprpb.Expr) (string, bool) {
 	// If the checker identified the expression as an attribute by the type-checker, then it can't
 	// possibly be part of qualified name in a namespace.
-	_, isAttr := p.refMap[operand.Id]
+	_, isAttr := p.refMap[operand.GetId()]
 	if isAttr {
 		return "", false
 	}