From 1825b87bb8e9545dbdcfc1529471be03bbdbd1f7 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Mon, 9 Sep 2024 14:12:53 +1200 Subject: [PATCH 1/9] #1408: Resolve function refs using signatures if present --- .../elm/evaluating/SimpleElmEvaluator.java | 10 +++ .../elm/executing/FunctionRefEvaluator.java | 82 ++++++++++++------- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/Src/java/elm/src/main/java/org/cqframework/cql/elm/evaluating/SimpleElmEvaluator.java b/Src/java/elm/src/main/java/org/cqframework/cql/elm/evaluating/SimpleElmEvaluator.java index d1a838c33..4d9294069 100644 --- a/Src/java/elm/src/main/java/org/cqframework/cql/elm/evaluating/SimpleElmEvaluator.java +++ b/Src/java/elm/src/main/java/org/cqframework/cql/elm/evaluating/SimpleElmEvaluator.java @@ -1,6 +1,8 @@ package org.cqframework.cql.elm.evaluating; +import javax.xml.namespace.QName; import org.hl7.elm.r1.Expression; +import org.hl7.elm.r1.TypeSpecifier; public class SimpleElmEvaluator { @@ -41,4 +43,12 @@ public static boolean dateRangesEqual(Expression left, Expression right) { public static boolean codesEqual(Expression left, Expression right) { return engine.codesEqual(left, right); } + + public static boolean typeSpecifiersEqual(TypeSpecifier left, TypeSpecifier right) { + return engine.typeSpecifiersEqual(left, right); + } + + public static boolean qnamesEqual(QName left, QName right) { + return engine.qnamesEqual(left, right); + } } diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index afca5d3ec..6ceea30d3 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -3,11 +3,10 @@ import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.cqframework.cql.elm.evaluating.SimpleElmEvaluator; import org.cqframework.cql.elm.visiting.ElmLibraryVisitor; -import org.hl7.elm.r1.Expression; -import org.hl7.elm.r1.FunctionDef; -import org.hl7.elm.r1.FunctionRef; -import org.hl7.elm.r1.TypeSpecifier; +import org.hl7.elm.r1.*; import org.opencds.cqf.cql.engine.exception.CqlException; import org.opencds.cqf.cql.engine.execution.Libraries; import org.opencds.cqf.cql.engine.execution.State; @@ -65,7 +64,7 @@ protected static FunctionDef resolveOrCacheFunctionDef( } } - FunctionDef functionDef = resolveFunctionDef(state, functionRef, arguments); + FunctionDef functionDef = resolveFunctionRef(state, functionRef, arguments); if (eligibleForCaching) { state.getCache().getFunctionCache().put(functionRef, functionDef); @@ -74,51 +73,76 @@ protected static FunctionDef resolveOrCacheFunctionDef( return functionDef; } - protected static FunctionDef resolveFunctionDef(State state, FunctionRef functionRef, ArrayList arguments) { - return resolveFunctionRef(state, functionRef.getName(), arguments, functionRef.getSignature()); - } + protected static FunctionDef resolveFunctionRef(State state, FunctionRef functionRef, List arguments) { + var name = functionRef.getName(); + var signature = functionRef.getSignature(); - public static FunctionDef resolveFunctionRef( - State state, final String name, final List arguments, final List signature) { - FunctionDef ret; + var functionDefs = resolveFunctionRef(state, name, arguments, signature); final List types = signature.isEmpty() ? arguments : signature; - ret = getResolvedFunctionDef(state, name, types, !signature.isEmpty()); + if (functionDefs.isEmpty()) { + throw new CqlException(String.format( + "Could not resolve call to operator '%s(%s)' in library '%s'.", + name, + getUnresolvedMessage(state, types, name), + state.getCurrentLibrary().getIdentifier().getId())); + } - if (ret != null) { - return ret; + if (functionDefs.size() == 1) { + // Normal case + return functionDefs.get(0); } throw new CqlException(String.format( - "Could not resolve call to operator '%s(%s)' in library '%s'.", + "Ambiguous call to operator '%s(%s)' in library '%s'.", name, getUnresolvedMessage(state, types, name), state.getCurrentLibrary().getIdentifier().getId())); } - private static FunctionDef getResolvedFunctionDef( - State state, final String name, final List types, final boolean hasSignature) { + private static List resolveFunctionRef( + State state, final String name, final List arguments, final List signature) { var namedDefs = Libraries.getFunctionDefs(name, state.getCurrentLibrary()); - var candidateDefs = namedDefs.stream() - .filter(x -> x.getOperand().size() == types.size()) + // If the function ref includes a signature, use the signature to find the matching function defs + if (!signature.isEmpty()) { + return namedDefs.stream() + .filter(x -> functionDefOperandsSignatureEqual(x, signature)) + .collect(Collectors.toList()); + } + + logger.debug( + "Using runtime function resolution for '{}'. It's recommended to always include signatures in ELM", + name); + + return namedDefs.stream() + .filter(x -> state.getEnvironment().matchesTypes(x, arguments)) .collect(Collectors.toList()); + } + + private static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef, List signature) { + var operands = functionDef.getOperand(); + + return operands.size() == signature.size() + && IntStream.range(0, operands.size()) + .allMatch(i -> operandDefTypeSpecifierEqual(operands.get(i), signature.get(i))); + } + + private static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeSpecifier typeSpecifier) { + // An operand def can have a resultTypeSpecifier or resultTypeName - if (candidateDefs.size() == 1) { - return candidateDefs.get(0); + var operandDefResultTypeSpecifier = operandDef.getResultTypeSpecifier(); + if (operandDefResultTypeSpecifier != null) { + return SimpleElmEvaluator.typeSpecifiersEqual(operandDefResultTypeSpecifier, typeSpecifier); } - if (candidateDefs.size() > 1 && !hasSignature) { - logger.debug( - "Using runtime function resolution for '{}'. It's recommended to always include signatures in ELM", - name); + if (typeSpecifier instanceof NamedTypeSpecifier) { + return SimpleElmEvaluator.qnamesEqual( + operandDef.getOperandType(), ((NamedTypeSpecifier) typeSpecifier).getName()); } - return candidateDefs.stream() - .filter(x -> state.getEnvironment().matchesTypes(x, types)) - .findFirst() - .orElse(null); + return false; } private static String getUnresolvedMessage(State state, List arguments, String name) { From 75431a1bdd8993763d2b1ab225c61e1696b5e14a Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Mon, 9 Sep 2024 22:04:18 +1200 Subject: [PATCH 2/9] Run spotless --- .../main/java/org/cqframework/fhir/npm/NpmPackageManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/Src/java/cqf-fhir-npm/src/main/java/org/cqframework/fhir/npm/NpmPackageManager.java b/Src/java/cqf-fhir-npm/src/main/java/org/cqframework/fhir/npm/NpmPackageManager.java index 1e139d07f..9a8a344c0 100644 --- a/Src/java/cqf-fhir-npm/src/main/java/org/cqframework/fhir/npm/NpmPackageManager.java +++ b/Src/java/cqf-fhir-npm/src/main/java/org/cqframework/fhir/npm/NpmPackageManager.java @@ -3,7 +3,6 @@ import ca.uhn.fhir.model.primitive.IdDt; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import org.hl7.fhir.r5.context.IWorkerContext; import org.hl7.fhir.r5.model.ImplementationGuide; From f56c5d001fa6754578ead9bf6ade36f6feb17b1d Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Mon, 9 Sep 2024 22:05:33 +1200 Subject: [PATCH 3/9] Fix tests --- .../elm/executing/FunctionRefEvaluator.java | 16 ++++++++++------ .../cql/engine/execution/CqlFunctionTest.java | 6 ++++++ .../cql/engine/execution/CqlFunctionTests.cql | 3 +++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index 6ceea30d3..49c39a231 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -130,11 +130,11 @@ private static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef } private static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeSpecifier typeSpecifier) { - // An operand def can have a resultTypeSpecifier or resultTypeName + // An operand def can have an operandTypeSpecifier or operandType - var operandDefResultTypeSpecifier = operandDef.getResultTypeSpecifier(); - if (operandDefResultTypeSpecifier != null) { - return SimpleElmEvaluator.typeSpecifiersEqual(operandDefResultTypeSpecifier, typeSpecifier); + var operandDefOperandTypeSpecifier = operandDef.getOperandTypeSpecifier(); + if (operandDefOperandTypeSpecifier != null) { + return SimpleElmEvaluator.typeSpecifiersEqual(operandDefOperandTypeSpecifier, typeSpecifier); } if (typeSpecifier instanceof NamedTypeSpecifier) { @@ -148,8 +148,12 @@ private static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeS private static String getUnresolvedMessage(State state, List arguments, String name) { StringBuilder argStr = new StringBuilder(); if (arguments != null) { - arguments.forEach(a -> argStr.append((argStr.length() > 0) ? ", " : "") - .append(state.getEnvironment().resolveType(a).getTypeName())); + arguments.forEach(a -> { + argStr.append((argStr.length() > 0) ? ", " : ""); + + Class type = state.getEnvironment().resolveType(a); + argStr.append(type == null ? "null" : type.getTypeName()); + }); } return argStr.toString(); diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlFunctionTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlFunctionTest.java index 2305e176f..167e1c173 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlFunctionTest.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlFunctionTest.java @@ -4,12 +4,18 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; +import org.cqframework.cql.cql2elm.CqlCompilerOptions; +import org.cqframework.cql.cql2elm.LibraryBuilder; import org.junit.jupiter.api.Test; class CqlFunctionTest extends CqlTestBase { @Test void all_function_tests() { + var compilerOptions = + CqlCompilerOptions.defaultOptions().withSignatureLevel(LibraryBuilder.SignatureLevel.Overloads); + var engine = getEngine(compilerOptions); + var results = engine.evaluate(toElmIdentifier("CqlFunctionTests")); var value = results.forExpression("FunctionTestStringArg").value(); assertThat(value, is("hello")); diff --git a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlFunctionTests.cql b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlFunctionTests.cql index 4a81de55b..2647423d5 100644 --- a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlFunctionTests.cql +++ b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlFunctionTests.cql @@ -22,4 +22,7 @@ define FunctionTestNullTupleArg: f3(Tuple { y: null as Integer }) define function f3(x Quantity): x.unit define FunctionTestQuantityArg: f3(12'cm') + +// Here the call to f3 can only be unambiguously resolved at runtime +// if the library is compiled with signature level set to Overloads or All define FunctionTestNullQuantityArg: f3(null as Quantity) \ No newline at end of file From 75d889404364ec6a13c4c9bf56af8451e9b28d2f Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Mon, 9 Sep 2024 22:25:51 +1200 Subject: [PATCH 4/9] Fix tests --- .../opencds/cqf/cql/engine/execution/CqlMainSuiteTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlMainSuiteTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlMainSuiteTest.java index 42024405e..ff3d157fe 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlMainSuiteTest.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlMainSuiteTest.java @@ -7,6 +7,7 @@ import java.util.*; import org.cqframework.cql.cql2elm.CqlCompilerException; import org.cqframework.cql.cql2elm.CqlCompilerOptions; +import org.cqframework.cql.cql2elm.LibraryBuilder; import org.junit.jupiter.api.Test; class CqlMainSuiteTest extends CqlTestBase { @@ -69,6 +70,11 @@ protected CqlCompilerOptions testCompilerOptions() { options.getOptions().remove(CqlCompilerOptions.Options.DisableListDemotion); options.getOptions().remove(CqlCompilerOptions.Options.DisableListPromotion); + // When called with the null argument, the toString function in the CqlTestSuite + // library can only be unambiguously resolved at runtime if the library is + // compiled with signature level set to Overloads or All. + options.withSignatureLevel(LibraryBuilder.SignatureLevel.Overloads); + return options; } From ecf68e238e53a12519e7af701a59bd8bd64151b8 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Mon, 9 Sep 2024 22:28:47 +1200 Subject: [PATCH 5/9] Fix tests --- .../opencds/cqf/cql/engine/execution/CqlPerformanceIT.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java index 1bc182602..4a447e600 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlPerformanceIT.java @@ -8,6 +8,7 @@ import java.time.ZonedDateTime; import java.util.TimeZone; import org.cqframework.cql.cql2elm.CqlCompilerOptions; +import org.cqframework.cql.cql2elm.LibraryBuilder; import org.fhir.ucum.UcumException; import org.hl7.elm.r1.VersionedIdentifier; import org.junit.jupiter.api.Test; @@ -98,6 +99,10 @@ protected CqlCompilerOptions testCompilerOptions() { options.getOptions().remove(CqlCompilerOptions.Options.DisableListDemotion); options.getOptions().remove(CqlCompilerOptions.Options.DisableListPromotion); + // When called with the null argument, the toString function in the CqlPerformanceTest + // library can only be unambiguously resolved at runtime if the library is + // compiled with signature level set to Overloads or All. + options.withSignatureLevel(LibraryBuilder.SignatureLevel.Overloads); return options; } } From c09aa55e7d540837202b21fd2b9f7010044064c0 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Tue, 10 Sep 2024 22:30:07 +1200 Subject: [PATCH 6/9] Add tests --- .../elm/executing/FunctionRefEvaluator.java | 59 +++++++++++-------- .../CqlListDistinguishedOverloadsTest.java | 17 +++++- .../CqlListDistinguishedOverloads.cql | 8 ++- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index 49c39a231..6f91881db 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -79,30 +79,11 @@ protected static FunctionDef resolveFunctionRef(State state, FunctionRef functio var functionDefs = resolveFunctionRef(state, name, arguments, signature); - final List types = signature.isEmpty() ? arguments : signature; - - if (functionDefs.isEmpty()) { - throw new CqlException(String.format( - "Could not resolve call to operator '%s(%s)' in library '%s'.", - name, - getUnresolvedMessage(state, types, name), - state.getCurrentLibrary().getIdentifier().getId())); - } - - if (functionDefs.size() == 1) { - // Normal case - return functionDefs.get(0); - } - - throw new CqlException(String.format( - "Ambiguous call to operator '%s(%s)' in library '%s'.", - name, - getUnresolvedMessage(state, types, name), - state.getCurrentLibrary().getIdentifier().getId())); + return pickFunctionDef(state, name, arguments, signature, functionDefs); } - private static List resolveFunctionRef( - State state, final String name, final List arguments, final List signature) { + static List resolveFunctionRef( + State state, String name, List arguments, List signature) { var namedDefs = Libraries.getFunctionDefs(name, state.getCurrentLibrary()); // If the function ref includes a signature, use the signature to find the matching function defs @@ -121,7 +102,7 @@ private static List resolveFunctionRef( .collect(Collectors.toList()); } - private static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef, List signature) { + static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef, List signature) { var operands = functionDef.getOperand(); return operands.size() == signature.size() @@ -129,7 +110,7 @@ private static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef .allMatch(i -> operandDefTypeSpecifierEqual(operands.get(i), signature.get(i))); } - private static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeSpecifier typeSpecifier) { + static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeSpecifier typeSpecifier) { // An operand def can have an operandTypeSpecifier or operandType var operandDefOperandTypeSpecifier = operandDef.getOperandTypeSpecifier(); @@ -145,7 +126,35 @@ private static boolean operandDefTypeSpecifierEqual(OperandDef operandDef, TypeS return false; } - private static String getUnresolvedMessage(State state, List arguments, String name) { + static FunctionDef pickFunctionDef( + State state, + String name, + List arguments, + List signature, + List functionDefs) { + var types = signature.isEmpty() ? arguments : signature; + + if (functionDefs.isEmpty()) { + throw new CqlException(String.format( + "Could not resolve call to operator '%s(%s)' in library '%s'.", + name, + typesToString(state, types), + state.getCurrentLibrary().getIdentifier().getId())); + } + + if (functionDefs.size() == 1) { + // Normal case + return functionDefs.get(0); + } + + throw new CqlException(String.format( + "Ambiguous call to operator '%s(%s)' in library '%s'.", + name, + typesToString(state, types), + state.getCurrentLibrary().getIdentifier().getId())); + } + + static String typesToString(State state, List arguments) { StringBuilder argStr = new StringBuilder(); if (arguments != null) { arguments.forEach(a -> { diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloadsTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloadsTest.java index 67f86cfb9..b7859690f 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloadsTest.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloadsTest.java @@ -1,10 +1,13 @@ package org.opencds.cqf.cql.engine.execution; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import org.cqframework.cql.cql2elm.CqlCompilerOptions; +import org.cqframework.cql.cql2elm.LibraryBuilder; import org.hl7.elm.r1.VersionedIdentifier; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.engine.exception.CqlException; @SuppressWarnings("removal") class CqlListDistinguishedOverloadsTest extends CqlTestBase { @@ -13,9 +16,17 @@ class CqlListDistinguishedOverloadsTest extends CqlTestBase { new VersionedIdentifier().withId("CqlListDistinguishedOverloads"); @Test - @Disabled("There's a bug in the cql engine that is causing it to select the wrong function overload at runtime") void list_overload() { - var value = engine.expression(library, "Test").value(); + var compilerOptions = CqlCompilerOptions.defaultOptions(); + + var engine1 = getEngine(compilerOptions.withSignatureLevel(LibraryBuilder.SignatureLevel.Overloads)); + var value = engine1.expression(library, "Test").value(); assertEquals("1, 2, 3, 4, 5", value); + + var engine2 = getEngine(compilerOptions.withSignatureLevel(LibraryBuilder.SignatureLevel.None)); + var cqlException = assertThrows(CqlException.class, () -> engine2.expression(library, "Test")); + assertEquals( + "Ambiguous call to operator 'toString(java.util.List)' in library 'CqlListDistinguishedOverloads'.", + cqlException.getMessage()); } } diff --git a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloads.cql b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloads.cql index 1af5bba41..c733aa7f7 100644 --- a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloads.cql +++ b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlListDistinguishedOverloads.cql @@ -17,8 +17,12 @@ define function toString(value List): else Combine((value V return toString(V)), ', ') // This is the function that _should_ be selected at runtime for the -// List distinguised overloads test. At the time this CQL was written, -// The engine had a bug that was selecting the List overload +// List distinguished overloads test. The engine currently cannot +// distinguish between the List and List overloads +// (and throws the error "Ambiguous call to function 'toString'") +// unless the library is compiled with signature level set to +// Overloads or All. +// See also https://github.com/cqframework/clinical_quality_language/issues/1408. define function toString(value List): if value is null then 'null' From ee3fa17cdac931b1f5b8966cfbb245705e3bb91a Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Tue, 10 Sep 2024 22:30:18 +1200 Subject: [PATCH 7/9] Add tests --- .../executing/FunctionRefEvaluatorTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluatorTest.java diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluatorTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluatorTest.java new file mode 100644 index 000000000..177445e00 --- /dev/null +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluatorTest.java @@ -0,0 +1,66 @@ +package org.opencds.cqf.cql.engine.elm.executing; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.Arrays; +import java.util.List; +import javax.xml.namespace.QName; +import org.hl7.elm.r1.*; +import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.engine.exception.CqlException; +import org.opencds.cqf.cql.engine.execution.Environment; +import org.opencds.cqf.cql.engine.execution.State; + +class FunctionRefEvaluatorTest { + @Test + void pickFunctionDef() { + var env = new Environment(null); + var state = new State(env); + state.init(new Library().withIdentifier(new VersionedIdentifier().withId("lib"))); + + var cqlException = assertThrows( + CqlException.class, + () -> FunctionRefEvaluator.pickFunctionDef(state, "func", List.of(1, 2, 3), List.of(), List.of())); + assertEquals( + "Could not resolve call to operator 'func(java.lang.Integer, java.lang.Integer, java.lang.Integer)' in library 'lib'.", + cqlException.getMessage()); + } + + @Test + void functionDefOperandsSignatureEqual() { + var functionDefWithOneOperand = new FunctionDef().withOperand(new OperandDef()); + List signatureWithTwoOperands = List.of(new NamedTypeSpecifier(), new NamedTypeSpecifier()); + + assertFalse(FunctionRefEvaluator.functionDefOperandsSignatureEqual( + functionDefWithOneOperand, signatureWithTwoOperands)); + } + + @Test + void operandDefTypeSpecifierEqual() { + var integerTypeName = new QName("urn:hl7-org:elm-types:r1", "Integer"); + var integerNamedTypeSpecifier = new NamedTypeSpecifier().withName(integerTypeName); + var listTypeSpecifier = new ListTypeSpecifier().withElementType(integerNamedTypeSpecifier); + + var listOperandDef = new OperandDef().withOperandTypeSpecifier(listTypeSpecifier); + var integerOperandDef = new OperandDef().withOperandType(integerTypeName); + + assertTrue(FunctionRefEvaluator.operandDefTypeSpecifierEqual(listOperandDef, listTypeSpecifier)); + assertTrue(FunctionRefEvaluator.operandDefTypeSpecifierEqual(integerOperandDef, integerNamedTypeSpecifier)); + assertFalse(FunctionRefEvaluator.operandDefTypeSpecifierEqual(integerOperandDef, null)); + } + + @Test + void typesToString() { + var env = new Environment(null); + var state = new State(env); + + var actual = FunctionRefEvaluator.typesToString(state, List.of("a", "b", "c")); + assertEquals("java.lang.String, java.lang.String, java.lang.String", actual); + + actual = FunctionRefEvaluator.typesToString(state, Arrays.asList(1, 2, null)); + assertEquals("java.lang.Integer, java.lang.Integer, null", actual); + + actual = FunctionRefEvaluator.typesToString(state, null); + assertEquals("", actual); + } +} From 3862ef66ac5544d419682172953a61de0794f702 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Wed, 11 Sep 2024 08:21:40 +1200 Subject: [PATCH 8/9] Simplify jacocoTestReport task config (#1411) --- Src/java/engine-fhir/build.gradle | 6 ++---- Src/java/engine/build.gradle | 5 ++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Src/java/engine-fhir/build.gradle b/Src/java/engine-fhir/build.gradle index 178c6ba77..3072caf70 100644 --- a/Src/java/engine-fhir/build.gradle +++ b/Src/java/engine-fhir/build.gradle @@ -26,17 +26,15 @@ generateSources { } jacocoTestReport { - dependsOn ':cql-to-elm:test' - dependsOn ':engine:test' - dependsOn ':engine-fhir:test' - sourceDirectories.setFrom(files( + "${projectDir}/../elm/src/main/java", "${projectDir}/../cql-to-elm/src/main/java", "${projectDir}/../engine/src/main/java", "${projectDir}/../engine-fhir/src/main/java", )) classDirectories.setFrom(files( + "${projectDir}/../elm/build/classes/java/main", "${projectDir}/../cql-to-elm/build/classes/java/main", "${projectDir}/../engine/build/classes/java/main", "${projectDir}/../engine-fhir/build/classes/java/main", diff --git a/Src/java/engine/build.gradle b/Src/java/engine/build.gradle index 03871059e..03212da9a 100644 --- a/Src/java/engine/build.gradle +++ b/Src/java/engine/build.gradle @@ -12,15 +12,14 @@ dependencies { } jacocoTestReport { - dependsOn ':cql-to-elm:test' - dependsOn ':engine:test' - sourceDirectories.setFrom(files( + "${projectDir}/../elm/src/main/java", "${projectDir}/../cql-to-elm/src/main/java", "${projectDir}/../engine/src/main/java", )) classDirectories.setFrom(files( + "${projectDir}/../elm/build/classes/java/main", "${projectDir}/../cql-to-elm/build/classes/java/main", "${projectDir}/../engine/build/classes/java/main", )) From 2c7d8a9e94242a477d3ee01a409f6e6c100f7f95 Mon Sep 17 00:00:00 2001 From: Anton Vasetenkov Date: Wed, 11 Sep 2024 08:45:05 +1200 Subject: [PATCH 9/9] Trigger Codecov --- .../cqf/cql/engine/elm/executing/FunctionRefEvaluator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java index 6f91881db..3708d7e0d 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/FunctionRefEvaluator.java @@ -105,6 +105,7 @@ static List resolveFunctionRef( static boolean functionDefOperandsSignatureEqual(FunctionDef functionDef, List signature) { var operands = functionDef.getOperand(); + // Check if the number of operands match and if the type specifiers match return operands.size() == signature.size() && IntStream.range(0, operands.size()) .allMatch(i -> operandDefTypeSpecifierEqual(operands.get(i), signature.get(i)));