Fix some issues with fhirpath expression evaluation

fhirpath/fhirpath_test.go

@@ -47,7 +47,10 @@ type evaluateTestCase struct {
 
 var (
 	patientChu = &ppb.Patient{
-		Id:     fhir.ID("123"),
+		Id: fhir.ID("123"),
+		Text: &dtpb.Narrative{
+			Div: &dtpb.Xhtml{Value: "patient chu record"},
+		},
 		Active: fhir.Boolean(true),
 		Gender: &ppb.Patient_GenderCode{
 			Value: cpb.AdministrativeGenderCode_FEMALE,
@@ -135,6 +138,13 @@ var (
 				},
 			},
 		},
+		RelatesTo: []*drpb.DocumentReference_RelatesTo{
+			{
+				Code: &drpb.DocumentReference_RelatesTo_CodeType{
+					Value: cpb.DocumentRelationshipTypeCode_APPENDS,
+				},
+			},
+		},
 	}
 	questionnaireRef, _ = reference.Typed("Questionnaire", "1234")
 	obsWithRef          = &opb.Observation{
@@ -455,6 +465,12 @@ func TestEvaluate_PathSelection_ReturnsResult(t *testing.T) {
 			inputCollection: []fhirpath.Resource{task},
 			wantCollection:  system.Collection{system.String(fhirconv.DateTimeToString(end.ToProtoDateTime()))},
 		},
+		{
+			name:            "delimited identifier",
+			inputPath:       "Patient.text.`div`",
+			inputCollection: []fhirpath.Resource{patientChu},
+			wantCollection:  system.Collection{fhir.Xhtml("patient chu record")},
+		},
 	}
 	testEvaluate(t, testCases)
 }
@@ -1068,6 +1084,13 @@ func TestFunctionInvocation_Evaluates(t *testing.T) {
 			inputCollection: []fhirpath.Resource{patientChu},
 			wantCollection:  system.Collection{system.Boolean(false), system.Boolean(true)},
 		},
+		{
+			name:            "(legacy) filtering nested fields by field name",
+			inputPath:       "descendants().family",
+			inputCollection: []fhirpath.Resource{patientChu},
+			compileOptions:  []fhirpath.CompileOption{compopts.Permissive()},
+			wantCollection:  system.Collection{patientChu.Name[0].Family, patientChu.Name[1].Family, patientChu.Contact[0].Name.Family},
+		},
 		{
 			name:            "filters child fields with ofType()",
 			inputPath:       "children().ofType(string)",
@@ -1225,6 +1248,12 @@ func TestTypeExpression_Evaluates(t *testing.T) {
 			inputCollection: []fhirpath.Resource{},
 			wantCollection:  system.Collection{system.MustParseDate("2000-12-05")},
 		},
+		{
+			name:            "passes through as code",
+			inputPath:       "relatesTo.code as code",
+			inputCollection: []fhirpath.Resource{docRef},
+			wantCollection:  system.Collection{docRef.RelatesTo[0].Code},
+		},
 	}
 
 	testEvaluate(t, testCases)
@@ -1490,6 +1519,10 @@ func TestCompile_ReturnsError(t *testing.T) {
 			name:      "resolving invalid type specifier",
 			inputPath: "1 is System.Patient",
 		},
+		{
+			name:      "reserved keyword not delimited",
+			inputPath: "Patient.text.div",
+		},
 	}
 
 	for _, tc := range testCases {

fhirpath/internal/expr/context.go

@@ -31,6 +31,9 @@ type Context struct {
 	// is used in the 'resolve()' FHIRPath function.
 	Resolver resolver.Resolver
 
+	// Permissive is a legacy option to allow FHIRpaths with *invalid* fields to be
+	// compiled (to reduce breakages).
+	Permissive bool
 	// Service is an optional mechanism for providing a terminology service
 	// which can be used to validate code in valueSet
 	TermService terminology.Service
@@ -66,6 +69,7 @@ func (c *Context) Clone() *Context {
 		ExternalConstants: c.ExternalConstants,
 		LastResult:        c.LastResult,
 		Resolver:          c.Resolver,
+		Permissive:        c.Permissive,
 		TermService:       c.TermService,
 		GoContext:         c.GoContext,
 	}

fhirpath/internal/expr/expressions.go

@@ -161,6 +161,9 @@ func (e *FieldExpression) Evaluate(ctx *Context, input system.Collection) (syste
 			fieldName = fieldName + "_value"
 			field = reflect.Descriptor().Fields().ByName(protoreflect.Name(fieldName))
 			if field == nil {
+				if e.Permissive {
+					continue
+				}
 				return nil, fmt.Errorf("%w: %s not a field on %T", ErrInvalidField, fieldName, message)
 			}
 		}
@@ -442,8 +445,9 @@ var _ Expression = (*EqualityExpression)(nil)
 // FunctionExpression enables evaluation of Function Invocation expressions.
 // It holds the function and function arguments.
 type FunctionExpression struct {
-	Fn   func(*Context, system.Collection, ...Expression) (system.Collection, error)
-	Args []Expression
+	Fn         func(*Context, system.Collection, ...Expression) (system.Collection, error)
+	Args       []Expression
+	Permissive bool
 }
 
 // Evaluate evaluates the function with respect to its arguments. Returns the result

fhirpath/internal/expr/expressions_test.go

@@ -210,6 +210,12 @@ func TestFieldExpression_Gets_DesiredField(t *testing.T) {
 			input:          system.Collection{patientMissingName},
 			wantCollection: system.Collection{patientBirthDay},
 		},
+		{
+			name:           "(Legacy) input item doesn't have field",
+			fieldExp:       &expr.FieldExpression{FieldName: "given", Permissive: true},
+			input:          system.Collection{patientFirstHumanName, patientContactPoint[0]},
+			wantCollection: system.Collection{patientFirstHumanName.Given[0], patientFirstHumanName.Given[1]},
+		},
 		{
 			name:           "(Legacy) input contains non-resource items",
 			fieldExp:       &expr.FieldExpression{FieldName: "birthDate", Permissive: true},

fhirpath/internal/funcs/impl/navigation.go

@@ -44,7 +44,7 @@ func Children(ctx *expr.Context, input system.Collection, args ...expr.Expressio
 			}
 		}
 		for _, f := range fields {
-			fe := expr.FieldExpression{FieldName: f}
+			fe := expr.FieldExpression{FieldName: f, Permissive: ctx.Permissive}
 			messages, err := fe.Evaluate(ctx, system.Collection{base})
 			if err != nil {
 				return nil, err

fhirpath/internal/parser/visitor.go

@@ -17,10 +17,11 @@ import (
 )
 
 var (
-	errNotSupported       = errors.New("expression not currently supported")
-	errTooManyQualifiers  = errors.New("too many type qualifiers")
-	errVisitingChildren   = errors.New("error while visiting child expressions")
-	errUnresolvedFunction = errors.New("function identifier can't be resolved")
+	errNotSupported               = errors.New("expression not currently supported")
+	errTooManyQualifiers          = errors.New("too many type qualifiers")
+	errVisitingChildren           = errors.New("error while visiting child expressions")
+	errUnresolvedFunction         = errors.New("function identifier can't be resolved")
+	errInvalidDelimitedIdentifier = errors.New("invalid delimited identifier")
 )
 
 type FHIRPathVisitor struct {
@@ -443,6 +444,12 @@ func (v *FHIRPathVisitor) VisitExternalConstant(ctx *grammar.ExternalConstantCon
 // root of the expression. If so, it will return a TypeExpression. Otherwise, it returns a FieldExpression.
 func (v *FHIRPathVisitor) VisitMemberInvocation(ctx *grammar.MemberInvocationContext) interface{} {
 	identifier := ctx.GetText()
+	if ctx.Identifier().DELIMITEDIDENTIFIER() != nil {
+		if len(identifier) < 2 || identifier[0] != '`' || identifier[len(identifier)-1] != '`' {
+			return &VisitResult{nil, fmt.Errorf("%w: %s", errInvalidDelimitedIdentifier, identifier)}
+		}
+		identifier = identifier[1 : len(identifier)-1]
+	}
 	var expression expr.Expression
 
 	if resource.IsType(identifier) && !v.visitedRoot {
@@ -502,8 +509,9 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{
 
 		return v.transformedVisitResult(
 			&expr.FunctionExpression{
-				Fn:   fn.Func,
-				Args: []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}},
+				Fn:         fn.Func,
+				Args:       []expr.Expression{&expr.TypeExpression{Type: typeSpecifier.String()}},
+				Permissive: v.Permissive,
 			},
 		)
 	}
@@ -522,7 +530,7 @@ func (v *FHIRPathVisitor) VisitFunction(ctx *grammar.FunctionContext) interface{
 	if len(expressions) < fn.MinArity || len(expressions) > fn.MaxArity {
 		return &VisitResult{nil, fmt.Errorf("%w: input arity outside of function arity bounds", impl.ErrWrongArity)}
 	}
-	return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions})
+	return v.transformedVisitResult(&expr.FunctionExpression{Fn: fn.Func, Args: expressions, Permissive: v.Permissive})
 }
 
 func (v *FHIRPathVisitor) VisitParamList(ctx *grammar.ParamListContext) interface{} {

fhirpath/resolver/bundleresolver.go

@@ -173,7 +173,7 @@ func getLatestResource(resources []fhir.Resource) (fhir.Resource, error) {
 			return nil, ErrMissingMetaOrLastUpdated
 		}
 		resLastUpdated := res.GetMeta().GetLastUpdated().GetValueUs()
-		if resolvedLastUpdated > resLastUpdated {
+		if resolvedLastUpdated < resLastUpdated {
 			resolvedResource = res
 			resolvedLastUpdated = resLastUpdated
 		}

fhirpath/resolver/bundleresolver_test.go

@@ -56,7 +56,7 @@ func TestBundleResolver_Resolve(t *testing.T) {
 	patient123Latest := &ppb.Patient{
 		Id: fhir.ID("123"),
 		Meta: &dtpb.Meta{
-			LastUpdated: &dtpb.Instant{ValueUs: 500},
+			LastUpdated: &dtpb.Instant{ValueUs: 5000},
 		},
 	}
 

fhirpath/system/types.go

@@ -52,7 +52,7 @@ func IsPrimitive(input any) bool {
 	switch v := input.(type) {
 	case *dtpb.Boolean, *dtpb.String, *dtpb.Uri, *dtpb.Url, *dtpb.Canonical, *dtpb.Code, *dtpb.Oid, *dtpb.Id, *dtpb.Uuid, *dtpb.Markdown,
 		*dtpb.Base64Binary, *dtpb.Integer, *dtpb.UnsignedInt, *dtpb.PositiveInt, *dtpb.Decimal, *dtpb.Date,
-		*dtpb.Time, *dtpb.DateTime, *dtpb.Instant, *dtpb.Quantity, Any:
+		*dtpb.Time, *dtpb.DateTime, *dtpb.Instant, *dtpb.Quantity, *dtpb.Xhtml, Any:
 		return true
 	case fhir.Base:
 		return protofields.IsCodeField(v)
@@ -94,6 +94,8 @@ func From(input any) (Any, error) {
 		return Integer(v.Value), nil
 	case *dtpb.PositiveInt:
 		return Integer(v.Value), nil
+	case *dtpb.Xhtml:
+		return String(v.Value), nil
 	case *dtpb.Decimal:
 		value, err := decimal.NewFromString(v.Value)
 		if err != nil {

fhirpath/system/types_test.go

@@ -97,6 +97,12 @@ var testCases []testCase = []testCase{
 		want:       system.String("aGVsbG8gd29ybGQ="),
 		shouldCast: true,
 	},
+	{
+		name:       "converts xhtml",
+		input:      fhir.Xhtml("xhtml"),
+		want:       system.String("xhtml"),
+		shouldCast: true,
+	},
 	{
 		name:       "converts integer",
 		input:      fhir.Integer(123),

internal/fhir/elements_primitive.go

@@ -341,3 +341,12 @@ func UUID(value string) *dtpb.Uuid {
 func RandomUUID() *dtpb.Uuid {
 	return UUID(uuid.NewString())
 }
+
+// Xhtml creates an R4 FHIR XHTML element from a string value.
+//
+// See: https://hl7.org/fhir/R4/narrative.html#xhtml
+func Xhtml(value string) *dtpb.Xhtml {
+	return &dtpb.Xhtml{
+		Value: value,
+	}
+}

internal/fhir/elements_primitive_test.go

@@ -388,3 +388,13 @@ func TestURIFromUUID(t *testing.T) {
 		})
 	}
 }
+
+func TestXhtml(t *testing.T) {
+	want := "xhtml"
+
+	sut := fhir.Xhtml(want)
+
+	if got := sut.GetValue(); !cmp.Equal(got, want) {
+		t.Errorf("Xhtml: got %v, want %v", got, want)
+	}
+}

internal/protofields/fields.go

@@ -1,8 +1,7 @@
 package protofields
 
 import (
-	"strings"
-
+	apb "github.com/google/fhir/go/proto/google/fhir/proto/annotations_go_proto"
 	dtpb "github.com/google/fhir/go/proto/google/fhir/proto/r4/core/datatypes_go_proto"
 	bcrpb "github.com/google/fhir/go/proto/google/fhir/proto/r4/core/resources/bundle_and_contained_resource_go_proto"
 	"github.com/iancoleman/strcase"
@@ -105,12 +104,14 @@ func UnwrapOneofField(element proto.Message, fieldName string) proto.Message {
 // Codes with enum values and string values are both considered valid.
 func IsCodeField(message proto.Message) bool {
 	reflect := message.ProtoReflect()
-	name := string(reflect.Descriptor().Name())
+	if !proto.HasExtension(reflect.Descriptor().Options(), apb.E_FhirValuesetUrl) {
+		return false
+	}
 	field := reflect.Descriptor().Fields().ByName(protoreflect.Name("value"))
 	if field != nil {
 		allowedKinds := []protoreflect.Kind{protoreflect.EnumKind, protoreflect.StringKind}
 		isValidFieldType := slices.Includes(allowedKinds, field.Kind())
-		return strings.HasSuffix(name, "Code") && isValidFieldType
+		return isValidFieldType
 	}
 	return false
 }