From 28cae24d1024b3a1d6cfe8028ef1d66bc4ef0ea0 Mon Sep 17 00:00:00 2001 From: Bryn Rhodes Date: Mon, 28 Oct 2024 10:29:00 -0600 Subject: [PATCH] More fixes --- .../cql/cql2elm/Cql2ElmVisitor.java | 4 +- .../cql/cql2elm/LibraryBuilder.java | 86 ++++++++++++++++--- .../operators/CqlListOperatorsTest.java | 2 +- .../execution/CqlStringOperatorsTest.cql | 2 +- .../java/org/hl7/cql/model/IntervalType.java | 12 +++ .../main/java/org/hl7/cql/model/ListType.java | 12 +++ 6 files changed, 104 insertions(+), 14 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 index c8b6a4cda..015bf6451 100755 --- 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 @@ -760,8 +760,10 @@ public Object visitListSelector(cqlParser.ListSelectorContext ctx) { } if (elementType == null) { + // In FHIRPath, the empty list is equivalent to null (i.e. { } is how you spell null in FHIRPath) elementType = - inferredElementType == null ? libraryBuilder.resolveTypeName("System", "Any") : inferredElementType; + inferredElementType == null ? libraryBuilder.buildWildcardType() : inferredElementType; + //inferredElementType == null ? libraryBuilder.resolveTypeName("System", "Any") : inferredElementType; } for (Expression element : elements) { diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java index cf2586aa0..25657982e 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java @@ -927,29 +927,70 @@ private BinaryWrapper normalizeListTypes(Expression left, Expression right) { && !(leftListType.isCompatibleWith(rightListType) || rightListType.isCompatibleWith(leftListType))) { Set elementTypes = new HashSet(); + DataType wildcardType = null; if (leftListType.getElementType() instanceof ChoiceType) { for (DataType choice : ((ChoiceType) leftListType.getElementType()).getTypes()) { - elementTypes.add(choice); + if (choice.isWildcard()) { + if (wildcardType == null) { + wildcardType = choice; + } + } else { + elementTypes.add(choice); + } } } else { - elementTypes.add(leftListType.getElementType()); + if (leftListType.getElementType().isWildcard()) { + if (wildcardType == null) { + wildcardType = leftListType.getElementType(); + } + } else { + elementTypes.add(leftListType.getElementType()); + } } if (rightListType.getElementType() instanceof ChoiceType) { for (DataType choice : ((ChoiceType) rightListType.getElementType()).getTypes()) { - elementTypes.add(choice); + if (choice.isWildcard()) { + if (wildcardType == null) { + wildcardType = choice; + } + } else { + elementTypes.add(choice); + } } } else { - elementTypes.add(rightListType.getElementType()); + if (rightListType.getElementType().isWildcard()) { + if (wildcardType == null) { + wildcardType = rightListType.getElementType(); + } + } else { + elementTypes.add(rightListType.getElementType()); + } + } + + boolean needsCast = false; + if (elementTypes.size() == 0 && wildcardType != null) { + elementTypes.add(wildcardType); + needsCast = true; } if (elementTypes.size() > 1) { - ListType targetType = new ListType(new ChoiceType(elementTypes)); - left = of.createAs().withOperand(left).withAsTypeSpecifier(dataTypeToTypeSpecifier(targetType)); - left.setResultType(targetType); + needsCast = true; + } + + if (needsCast) { + DataType targetElementType + = elementTypes.size() > 1 ? new ChoiceType(elementTypes) : elementTypes.iterator().next(); + ListType targetType = new ListType(targetElementType); + if (!left.getResultType().equals(targetType)) { + left = of.createAs().withOperand(left).withAsTypeSpecifier(dataTypeToTypeSpecifier(targetType)); + left.setResultType(targetType); + } - right = of.createAs().withOperand(right).withAsTypeSpecifier(dataTypeToTypeSpecifier(targetType)); - right.setResultType(targetType); + if (!right.getResultType().equals(targetType)) { + right = of.createAs().withOperand(right).withAsTypeSpecifier(dataTypeToTypeSpecifier(targetType)); + right.setResultType(targetType); + } } } } @@ -1467,6 +1508,21 @@ private FunctionRef buildFunctionRef(String libraryName, String functionName, It return fun; } + private Signature preserveSignature(Iterable params) { + List dataTypes = new ArrayList(); + for (Expression param : params) { + dataTypes.add(param.getResultType()); + } + return new Signature(dataTypes.toArray(new DataType[dataTypes.size()])); + } + + private void restoreSignature(Iterable params, Signature preservedSignature) { + Iterator operandTypes = preservedSignature.getOperandTypes().iterator(); + for (Expression param : params) { + param.setResultType(operandTypes.next()); + } + } + public Invocation resolveFunction( String libraryName, String functionName, @@ -1478,6 +1534,7 @@ public Invocation resolveFunction( // Attempt normal resolution, but don't require one Invocation invocation = new FunctionRefInvocation(fun); + Signature preservedSignature = preserveSignature(paramList); fun = (FunctionRef) resolveCall( fun.getLibraryName(), fun.getName(), invocation, false, allowPromotionAndDemotion, allowFluent); if (fun != null) { @@ -1488,16 +1545,23 @@ public Invocation resolveFunction( // In this case, we preserve the invocation result type from the resolution and use it DataType invocationResultType = invocation.getResolution().getInvocationResultType(); if ("System".equals(invocation.getResolution().getOperator().getLibraryName())) { + // Save the resolvedSignature so it can be restored in case the system function call does not resolve + Signature resolvedSignature = preserveSignature(paramList); + // Rebuild the fun from the original arguments, restoring the original signature, + // otherwise it will resolve with conversions in place + restoreSignature(paramList, preservedSignature); FunctionRef systemFun = buildFunctionRef( libraryName, functionName, - paramList); // Rebuild the fun from the original arguments, otherwise it will resolve with - // conversions in place + paramList); Invocation systemFunctionInvocation = systemFunctionResolver.resolveSystemFunction(systemFun); if (systemFunctionInvocation != null) { systemFunctionInvocation.getResolution().setInvocationResultType(invocationResultType); systemFunctionInvocation.setResultType(invocationResultType); return systemFunctionInvocation; + } else { + // Restore the originally resolved signature + restoreSignature(paramList, resolvedSignature); } } else { // If the invocation is to a local function or a function in a non-system library, check literal context diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/operators/CqlListOperatorsTest.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/operators/CqlListOperatorsTest.java index 5d5ee1507..c8e413dcd 100644 --- a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/operators/CqlListOperatorsTest.java +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/operators/CqlListOperatorsTest.java @@ -42,6 +42,6 @@ static void setup() throws IOException { @Test void union() { ExpressionDef def = defs.get("Union123AndEmpty"); - assertThat(def, hasTypeAndResult(Union.class, "list")); + assertThat(def, hasTypeAndResult(Union.class, "list")); } } diff --git a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlStringOperatorsTest.cql b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlStringOperatorsTest.cql index a2dea83f1..b62a6b0de 100644 --- a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlStringOperatorsTest.cql +++ b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlStringOperatorsTest.cql @@ -4,7 +4,7 @@ codesystem "LOINC": 'http://loinc.org' //Combine define CombineNull : Combine(null) -define CombineEmptyList : Combine({}) +define CombineEmptyList : Combine({} as List) define CombineABC : Combine({'a', 'b', 'c'}) define CombineABCSepDash : Combine({'a', 'b', 'c'}, '-') diff --git a/Src/java/model/src/main/java/org/hl7/cql/model/IntervalType.java b/Src/java/model/src/main/java/org/hl7/cql/model/IntervalType.java index 8e79c6fd7..492fb7a09 100644 --- a/Src/java/model/src/main/java/org/hl7/cql/model/IntervalType.java +++ b/Src/java/model/src/main/java/org/hl7/cql/model/IntervalType.java @@ -52,6 +52,18 @@ public boolean isSuperTypeOf(DataType other) { return super.isSuperTypeOf(other); } + @Override + public boolean isCompatibleWith(DataType other) { + if (other instanceof IntervalType) { + IntervalType otherIntervalType = (IntervalType)other; + if (pointType.isCompatibleWith(otherIntervalType.pointType)) { + return true; + } + } + + return super.isCompatibleWith(other); + } + @Override public String toString() { return String.format("interval<%s>", pointType.toString()); diff --git a/Src/java/model/src/main/java/org/hl7/cql/model/ListType.java b/Src/java/model/src/main/java/org/hl7/cql/model/ListType.java index 6279288f0..29458d632 100644 --- a/Src/java/model/src/main/java/org/hl7/cql/model/ListType.java +++ b/Src/java/model/src/main/java/org/hl7/cql/model/ListType.java @@ -52,6 +52,18 @@ public boolean isSuperTypeOf(DataType other) { return super.isSuperTypeOf(other); } + @Override + public boolean isCompatibleWith(DataType other) { + if (other instanceof ListType) { + ListType otherListType = (ListType)other; + if (elementType.isCompatibleWith(otherListType.elementType)) { + return true; + } + } + + return super.isCompatibleWith(other); + } + @Override public String toString() { return String.format("list<%s>", elementType.toString());