Make sure per is set to something when visiting expand and `colla…

…pse` (#1425) * Make sure `per` is set to something when visiting `expand` and `collapse` * Fix test case --------- Co-authored-by: JP <[email protected]>
cqframework · Nov 5, 2024 · 3bdd109 · 3bdd109
1 parent 8390d7a
commit 3bdd109
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 10 deletions.
diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/Cql2ElmVisitor.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/Cql2ElmVisitor.java
@@ -2758,7 +2758,9 @@ public Object visitAggregateExpressionTerm(cqlParser.AggregateExpressionTermCont
     @Override
     public Object visitSetAggregateExpressionTerm(cqlParser.SetAggregateExpressionTermContext ctx) {
         Expression source = parseExpression(ctx.expression(0));
-        Expression per = null;
+
+        // If `per` is not set, it will remain `null as System.Quantity`.
+        Expression per = libraryBuilder.buildNull(libraryBuilder.resolveTypeName("System", "Quantity"));
         if (ctx.dateTimePrecision() != null) {
             per = libraryBuilder.createQuantity(BigDecimal.valueOf(1.0), parseString(ctx.dateTimePrecision()));
         } else if (ctx.expression().size() > 1) {
@@ -2771,8 +2773,6 @@ public Object visitSetAggregateExpressionTerm(cqlParser.SetAggregateExpressionTe
                     IntervalType intervalType = (IntervalType) listType.getElementType();
                     DataType pointType = intervalType.getPointType();
 
-                    per = libraryBuilder.buildNull(libraryBuilder.resolveTypeName("System", "Quantity"));
-
                     // TODO: Test this...
                     // // Successor(MinValue<T>) - MinValue<T>
                     // MinValue minimum = libraryBuilder.buildMinimum(pointType);
@@ -2788,8 +2788,6 @@ public Object visitSetAggregateExpressionTerm(cqlParser.SetAggregateExpressionTe
                     // libraryBuilder.resolveBinaryCall("System", "Subtract", subtract);
                     // per = subtract;
                 }
-            } else {
-                per = libraryBuilder.buildNull(libraryBuilder.resolveTypeName("System", "Quantity"));
             }
         }
 

diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/SemanticTests.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/SemanticTests.java
@@ -382,11 +382,11 @@ void issue435() throws IOException {
 
     @Test
     void issue587() throws IOException {
-        CqlTranslator translator = TestUtils.runSemanticTest("Issue587.cql", 1);
-        // This doesn't resolve correctly, collapse null should work, but it's related to this issue:
-        // [#435](https://github.com/cqframework/clinical_quality_language/issues/435)
-        // So keeping as a verification of current behavior here, will address as part of vNext
-        assertThat(translator.getErrors().size(), equalTo(1));
+        // Both `collapse null` and `collapse { null }` now translate with no errors (expectedErrors = 0).
+        // The old errors were related to [#435](https://github.com/cqframework/clinical_quality_language/issues/435)
+        // and fixed by [#1428](https://github.com/cqframework/clinical_quality_language/pull/1428) and
+        // [#1425](https://github.com/cqframework/clinical_quality_language/pull/1425).
+        TestUtils.runSemanticTest("Issue587.cql", 0);
     }
 
     @Test