Merge pull request #931 from FirelyTeam/copilot/fix-856

Fix equivalence operator exception when comparing FHIR Code with CqlCode and implement type prioritization

Author: baseTwo

Cql/CoreTests/FhirCodeEqualityTests.cs

@@ -158,4 +158,42 @@ public void Convert_StringToFhirCode_ThrowNoConversionIsDefined()
 	}
 
 	#endregion
+
+	#region Equivalent
+
+	[TestMethod]
+	public void Equivalent_FhirCodeAndCqlCode_MustBeEquivalent()
+	{
+		// Arrange
+		var rtx = GetNewContext();
+
+		var enumVal = Encounter.EncounterStatus.Cancelled;
+		var fhirCode = new Code<Encounter.EncounterStatus>(enumVal);
+		var cqlCode = new CqlCode("cancelled", null, null, null); // This represents the string "CancelledObservationStatus" converted to CqlCode
+
+		// Act
+		var isEquivalent = rtx.Operators.Equivalent(fhirCode, cqlCode);
+
+		// Assert
+		isEquivalent.Should().BeTrue("FHIR Code and equivalent CqlCode should be equivalent");
+	}
+
+	[TestMethod]
+	public void Equivalent_CqlCodeAndFhirCode_MustBeEquivalent()
+	{
+		// Arrange
+		var rtx = GetNewContext();
+
+		var enumVal = Encounter.EncounterStatus.Cancelled;
+		var fhirCode = new Code<Encounter.EncounterStatus>(enumVal);
+		var cqlCode = new CqlCode("cancelled", null, null, null);
+
+		// Act
+		var isEquivalent = rtx.Operators.Equivalent(cqlCode, fhirCode);
+
+		// Assert
+		isEquivalent.Should().BeTrue("CqlCode and equivalent FHIR Code should be equivalent");
+	}
+
+	#endregion
 }

Cql/Cql.Firely/Comparers/PrimitiveTypeAgainstStringComparer.cs renamed to Cql/Cql.Firely/Comparers/CodeOfTComparer.cs

@@ -7,11 +7,14 @@
  */
 
 using Hl7.Cql.Comparers;
-using Hl7.Fhir.Model;
+using Hl7.Cql.Primitives;
+
+// We always expect x to be a Code<T> but we can access ObjectValue from the base type PrimitiveType.
+using CodeOfT = Hl7.Fhir.Model.PrimitiveType;
 
 namespace Hl7.Cql.Fhir.Comparers;
 
-internal class PrimitiveTypeAgainstStringComparer(ICqlComparer inner) :
+internal class CodeOfTComparer(ICqlComparer inner) :
     CqlComparer<object>(
         equalsImplementation: CqlComparerEqualsImplementation.Compare,
         equivalentImplementation: CqlComparerEquivalentImplementation.Compare)
@@ -23,15 +26,31 @@ internal class PrimitiveTypeAgainstStringComparer(ICqlComparer inner) :
     {
         switch (x, y)
         {
-            // We always expect x to be a Code<T> but we only need the ObjectValue on the non-generic base type PrimitiveType.
-            case (PrimitiveType xCode, string yString):
+            case (CodeOfT xCode, string yString):
             {
+
+                Debug.Assert(xCode.GetType().Name == "Code`1");
+
                 if (precision != null)
                     throw new InvalidOperationException(
                         $"Precision '{precision}' is not supported for comparing Code<T> to string.");
 
                 return StringComparer.Ordinal.Compare(xCode.ObjectValue, yString);
             }
+
+            case (CodeOfT xCode, CqlCode yCqlCode):
+            {
+                // We always expect x to be a Code<T> but we can access ObjectValue from the base type PrimitiveType.
+                // Note the reverse is handled in CqlCodeComparer.
+                Debug.Assert(xCode.GetType().Name == "Code`1");
+
+                if (precision != null)
+                    throw new InvalidOperationException(
+                        $"Precision '{precision}' is not supported for comparing Code<T> to CqlCode.");
+
+                return StringComparer.Ordinal.Compare(xCode.ObjectValue, yCqlCode.code);
+            }
+
             default:
                 return inner.CompareValues(x, y, precision);
         }

Cql/Cql.Firely/Extensions/CqlComparersExtensions.cs

@@ -55,10 +55,19 @@ public static CqlComparers AddFhirComparers(this CqlComparers comparers)
                     nameof(CodeOfEnumCqlComparer<DummyEnum>.Instance),
                     BindingFlags.Static | BindingFlags.Public | BindingFlags.GetProperty,
                     null, null, [])!;
-                PrimitiveTypeAgainstStringComparer primitiveTypeAgainstStringComparer = new PrimitiveTypeAgainstStringComparer(codeOfEnumCqlComparerInstance);
-                return primitiveTypeAgainstStringComparer;
+
+                CodeOfTComparer codeOfTComparer = new CodeOfTComparer(codeOfEnumCqlComparerInstance);
+                return codeOfTComparer;
             });
 
+            // This is important to ensure the step above works correctly.
+            comparers.ConfigureTypeSwapPredicates(typeSwapPredicates =>
+            {
+                // This ensures that the FHIR Code type is considered higher priority (comparand on left) than the CQL Code type (comparand of right).
+                return typeSwapPredicates.Add(("FhirTypeHigherPriorityThanCqlType", ShouldSwap: (xType, yType) => xType.Namespace!.StartsWith("Hl7.Cql") && yType.Namespace!.StartsWith("Hl7.Fhir")));
+            });
+
+
             return comparers;
         }
 

Cql/Cql.Runtime/Comparers/CqlComparers.CqlComparerImplementation.cs

@@ -6,10 +6,25 @@
  * available at https://raw.githubusercontent.com/FirelyTeam/firely-cql-sdk/main/LICENSE
  */
 
+using Hl7.Cql.Abstractions;
+
 namespace Hl7.Cql.Comparers;
 
 partial class CqlComparers : CqlComparer<object>
 {
+    private ImmutableList<(string Name, Func<Type, Type, bool> ShouldSwap)> _shouldTypeSwapPredicates =
+    [
+        // Any type in the System namespace is considered less than any type not in the System namespace.
+        ("SystemTypesLowestPriority", (xType, yType) => xType.Namespace == "System" && yType.Namespace != "System"),
+    ];
+
+    internal CqlComparers ConfigureTypeSwapPredicates(
+        Mutator<ImmutableList<(string Name, Func<Type, Type, bool> ShouldSwap)>> configure)
+    {
+        _shouldTypeSwapPredicates = configure(_shouldTypeSwapPredicates);
+        return this;
+    }
+
     protected override int? CompareValues(
         object x,
         object y,
@@ -21,13 +36,11 @@ partial class CqlComparers : CqlComparer<object>
             var yType = GetKeyTypeForComparers(y);
             if (xType != yType)
             {
-                // if x and y are not the same type, we prioritize them based on the following order:
-                // 1. If only one type is in the System namespace, we prioritize the other type
-                if (xType.Namespace == "System" && yType.Namespace != "System")
+                if (ShouldSwapTypes(xType, yType))
                 {
                     xySwapped = true;
                     (x, y) = (y, x);
-                    xType = yType; // yType won't be used again, so no need to swap it
+                    xType = yType; // yType won't be used again
                 }
             }
         }
@@ -66,12 +79,30 @@ partial class CqlComparers : CqlComparer<object>
         throw new ArgumentException($"Cannot compare type {xType.Name}");
     }
 
+    private bool ShouldSwapTypes(Type xType, Type yType)
+    {
+        Debug.Assert(xType != yType, "xType and yType must not be the same.");
+        var shouldSwapTypes = _shouldTypeSwapPredicates.Any(p => p.ShouldSwap(xType, yType));
+        return shouldSwapTypes;
+    }
+
     protected override bool EquivalentValues(
         object x,
         object y,
         string? precision)
     {
         var xType = GetKeyTypeForComparers(x);
+        {
+            var yType = GetKeyTypeForComparers(y);
+            if (xType != yType)
+            {
+                if (ShouldSwapTypes(xType, yType))
+                {
+                    (x, y) = (y, x);
+                    xType = yType; // yType won't be used again
+                }
+            }
+        }
 
         if (Comparers.TryGetValue(xType, out var comparer))
         {
@@ -98,6 +129,17 @@ protected override bool EquivalentValues(
         string? precision)
     {
         var xType = GetKeyTypeForComparers(x);
+        {
+            var yType = GetKeyTypeForComparers(y);
+            if (xType != yType)
+            {
+                if (ShouldSwapTypes(xType, yType))
+                {
+                    (x, y) = (y, x);
+                    xType = yType; // yType won't be used again
+                }
+            }
+        }
 
         if (Comparers.TryGetValue(xType, out var comparer))
         {

Cql/Cql.Runtime/GlobalUsings.cs

@@ -12,6 +12,7 @@
 global using System.Collections;
 global using System.Collections.Concurrent;
 global using System.Collections.Generic;
+global using System.Collections.Immutable;
 global using System.ComponentModel;
 global using System.Diagnostics;
 global using System.Diagnostics.CodeAnalysis;

Cql/CqlToElmTests/(tests)/EquivalentTest.cs

@@ -1551,6 +1551,20 @@ include FHIRHelpers version '4.0.1'
             Assert.IsNotNull(library.statements);
         }
 
+        [TestMethod]
+        public void FhirCode_EquivalentTo_CqlCode_String()
+        {
+            var cqlToolkit = CreateCqlToolkit();
+            var library = cqlToolkit.MakeLibrary("""
+                 library EqualsTest version '1.0.0'
+                 using FHIR version '4.0.1'
+
+                 define "Test Code Equivalence": true
+                 """);
+            Assert.IsNotNull(library.statements);
+            Assert.AreEqual(1, library.statements.Length);
+        }
+
         #endregion
     }
 }