Fixes for Union (#1480)

* Fixes for Union

* More tests

Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java

@@ -83,10 +83,7 @@ public static void setup() {
         r4Provider = new CompositeDataProvider(r4ModelResolver, r4RetrieveProvider);
 
         modelManager = new ModelManager();
-        var compilerOptions = new CqlCompilerOptions(
-                CqlCompilerException.ErrorSeverity.Info,
-                LibraryBuilder.SignatureLevel.All,
-                CqlCompilerOptions.Options.EnableDateRangeOptimization);
+        var compilerOptions = CqlCompilerOptions.defaultOptions();
         libraryManager = new LibraryManager(modelManager, compilerOptions);
         libraryManager.getLibrarySourceLoader().clearProviders();
         libraryManager.getLibrarySourceLoader().registerProvider(new FhirLibrarySourceProvider());

Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/Issue1441.java

@@ -0,0 +1,64 @@
+package org.opencds.cqf.cql.engine.fhir.data;
+
+import static org.junit.jupiter.api.Assertions.assertIterableEquals;
+
+import java.util.Collections;
+import org.hl7.fhir.r4.model.Encounter;
+import org.hl7.fhir.r4.model.Patient;
+import org.hl7.fhir.r4.model.Procedure;
+import org.junit.jupiter.api.Test;
+import org.opencds.cqf.cql.engine.data.CompositeDataProvider;
+import org.opencds.cqf.cql.engine.retrieve.RetrieveProvider;
+import org.opencds.cqf.cql.engine.runtime.Code;
+import org.opencds.cqf.cql.engine.runtime.Interval;
+
+// https://github.com/cqframework/clinical_quality_language/issues/1441
+// unions without aliases are not working
+class Issue1441 extends FhirExecutionTestBase {
+
+    @Test
+    void unionsWithoutAliasesAreTheSameAsUnionsWithAliases() {
+        var patient = new Patient().setId("123");
+        var observation = new Encounter().setId("456");
+        var procedure = new Procedure().setId("789");
+
+        var r = new RetrieveProvider() {
+            @Override
+            public Iterable<Object> retrieve(
+                    String context,
+                    String contextPath,
+                    Object contextValue,
+                    String dataType,
+                    String templateId,
+                    String codePath,
+                    Iterable<Code> codes,
+                    String valueSet,
+                    String datePath,
+                    String dateLowPath,
+                    String dateHighPath,
+                    Interval dateRange) {
+
+                switch (dataType) {
+                    case "Patient":
+                        return Collections.singletonList(patient);
+                    case "Observation":
+                        return Collections.singletonList(observation);
+                    case "Procedure":
+                        return Collections.singletonList(procedure);
+                    default:
+                        return Collections.emptyList();
+                }
+            }
+        };
+
+        var engine = getEngine();
+        engine.getState()
+                .getEnvironment()
+                .registerDataProvider("http://hl7.org/fhir", new CompositeDataProvider(r4ModelResolver, r));
+        var result = engine.evaluate("Issue1441");
+        var x = (Iterable<?>) result.forExpression("x").value();
+        var y = (Iterable<?>) result.forExpression("y").value();
+
+        assertIterableEquals(x, y);
+    }
+}

Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFHIRHelpers.java

@@ -3,17 +3,50 @@
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import org.cqframework.cql.cql2elm.CqlCompilerOptions;
+import org.cqframework.cql.cql2elm.LibraryBuilder;
+import org.cqframework.cql.cql2elm.LibraryManager;
+import org.cqframework.cql.cql2elm.ModelManager;
+import org.cqframework.cql.cql2elm.quick.FhirLibrarySourceProvider;
 import org.junit.jupiter.api.Test;
+import org.opencds.cqf.cql.engine.exception.CqlException;
 import org.opencds.cqf.cql.engine.execution.CqlEngine;
+import org.opencds.cqf.cql.engine.execution.Environment;
 
 class TestFHIRHelpers extends FhirExecutionTestBase {
 
     @Test
-    void test() {
+    void testWithUnambiguousCompilerOptions() {
+        // Test with non-ambiguous compiler options. Should pass.
+        var compilerOptions = CqlCompilerOptions.defaultOptions();
+        compilerOptions.setSignatureLevel(LibraryBuilder.SignatureLevel.Overloads);
+        var modelManager = new ModelManager();
+        var libraryManager = new LibraryManager(modelManager, compilerOptions);
+        libraryManager.getLibrarySourceLoader().clearProviders();
+        libraryManager.getLibrarySourceLoader().registerProvider(new FhirLibrarySourceProvider());
+        libraryManager.getLibrarySourceLoader().registerProvider(new TestLibrarySourceProvider());
+
+        var engine = new CqlEngine(new Environment(libraryManager));
+        engine.getEnvironment().registerDataProvider("http://hl7.org/fhir", r4Provider);
+
+        runTest(engine);
+    }
 
-        CqlEngine engine = getEngine();
+    @Test
+    void standardCompilerOptions() {
+        // Test with standard compiler options. Should throw an Exception as of the
+        // time this test is written because the default compiler options produce
+        // ambiguous ELM output. This test is intended to fail, and if we change the
+        // compiler options to be non-ambiguous, this test should be updated to expect
+        // a different (presumably passing) result.
+        var engine = getEngine();
         engine.getEnvironment().registerDataProvider("http://hl7.org/fhir", r4Provider);
+        assertThrows(CqlException.class, () -> runTest(engine));
+    }
+
+    void runTest(CqlEngine engine) {
         var results = engine.evaluate(library.getIdentifier());
 
         // Primitives

Src/java/engine-fhir/src/test/resources/org/opencds/cqf/cql/engine/fhir/data/Issue1441.cql

@@ -0,0 +1,16 @@
+library "Issue1441" version '1'
+
+// https://github.com/cqframework/clinical_quality_language/issues/1441
+// Unions of unaliased retrieves not working
+
+using FHIR version '4.0.1'
+
+context Patient
+
+define x:
+  [Observation] a
+    union [Procedure] b
+
+define y:
+  [Observation]
+    union [Procedure]

Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/execution/EvaluationVisitor.java

@@ -187,7 +187,7 @@ public Object visitUnion(Union elm, State state) {
                     (org.opencds.cqf.cql.engine.runtime.Interval) rightResult,
                     state);
         } else {
-            return UnionEvaluator.union(left, right, state);
+            return UnionEvaluator.union(leftResult, rightResult, state);
         }
     }
 

Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/TestUnion.java

@@ -2,6 +2,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
@@ -12,10 +13,17 @@ class TestUnion extends CqlTestBase {
     @Test
     void union() {
         var results = engine.evaluate(toElmIdentifier("TestUnion"));
-        var value = results.forExpression("NullAndNull").value();
+
+        var value = results.forExpression("NullAndNullList").value();
         assertNotNull(value);
         assertTrue(((List<?>) value).isEmpty());
 
+        value = results.forExpression("NullAndNullInterval").value();
+        assertNull(value);
+
+        value = results.forExpression("NullAndNullUntyped").value();
+        assertNull(value);
+
         value = results.forExpression("NullAndEmpty").value();
         assertNotNull(value);
         assertTrue(((List<?>) value).isEmpty());

Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/TestUnion.cql

@@ -1,9 +1,20 @@
 library TestUnion
 
 // expect {}
-define "NullAndNull":
+define "NullAndNullList":
     null as List<Integer> union null as List<Integer>
 
+// expect null
+define "NullAndNullInterval":
+    null as Interval<Integer> union null as Interval<Integer>
+
+// expect null
+// Based on the CQL conversion precedence rules,
+// the compiler _should have_ inferred this as Intervals
+// and not Lists.
+define "NullAndNullUntyped":
+    null union null
+
 // expect {}
 define "NullAndEmpty":
     null union {}