Skip to content

Commit

Permalink
Fixes from regression testing
Browse files Browse the repository at this point in the history
  • Loading branch information
@brynrhodes
brynrhodes committed Oct 30, 2024
1 parent ad33708 commit 49ff33b
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 67 deletions.
36 changes: 36 additions & 0 deletions Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1098,12 +1098,48 @@ public Invocation resolveProperContainsInvocation(
return resolveBinaryInvocation("System", "ProperContains", properContains);
}

private int getTypeScore(OperatorResolution resolution) {
int typeScore = ConversionMap.ConversionScore.ExactMatch.score();
for (DataType operand :
resolution.getOperator().getSignature().getOperandTypes()) {
typeScore += ConversionMap.getTypePrecedenceScore(operand);
}

return typeScore;
}

private Expression lowestScoringInvocation(Invocation primary, Invocation secondary) {
if (primary != null) {
if (secondary != null) {
if (secondary.getResolution().getScore()
< primary.getResolution().getScore()) {
return secondary.getExpression();
} else if (primary.getResolution().getScore() < secondary.getResolution().getScore()) {
return primary.getExpression();
}
if (primary.getResolution().getScore() == secondary.getResolution().getScore()) {
int primaryTypeScore = getTypeScore(primary.getResolution());
int secondaryTypeScore = getTypeScore(secondary.getResolution());

if (secondaryTypeScore < primaryTypeScore) {
return secondary.getExpression();
} else if (primaryTypeScore < secondaryTypeScore) {
return primary.getExpression();
} else {
// ERROR:
StringBuilder message = new StringBuilder("Call to operator ")
.append(primary.getResolution().getOperator().getName())
.append("/")
.append(secondary.getResolution().getOperator().getName())
.append(" is ambiguous with: ")
.append("\n - ")
.append(primary.getResolution().getOperator().getName())
.append(primary.getResolution().getOperator().getSignature())
.append("\n - ")
.append(secondary.getResolution().getOperator().getName())
.append(secondary.getResolution().getOperator().getSignature());
throw new IllegalArgumentException(message.toString());
}
}
}

Expand Down
100 changes: 80 additions & 20 deletions Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/OperatorEntry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,50 +68,110 @@ private Signature getInvocationSignature(Signature callSignature, Signature oper
return callSignature;
}

private OperatorResolution getOperatorResolution(
Operator operator,
Signature callSignature,
Signature invocationSignature,
ConversionMap conversionMap,
OperatorMap operatorMap,
boolean allowPromotionAndDemotion,
boolean requireConversions) {
Conversion[] conversions = getConversions(
callSignature, operator.getSignature(), conversionMap, operatorMap, allowPromotionAndDemotion);
OperatorResolution result = new OperatorResolution(
operator, conversions);
if (requireConversions && conversions == null) {
return null;
}
return result;
}

public List<OperatorResolution> resolve(
CallContext callContext, ConversionMap conversionMap, OperatorMap operatorMap) {
List<OperatorResolution> results = null;
Signature invocationSignature = getInvocationSignature(callContext.getSignature(), operator.getSignature());

// Attempt exact match against this signature
if (operator.getSignature().equals(invocationSignature)) {
results = new ArrayList<>();
results.add(new OperatorResolution(operator));
return results;
OperatorResolution result = getOperatorResolution(
operator,
callContext.getSignature(),
invocationSignature,
conversionMap,
operatorMap,
callContext.getAllowPromotionAndDemotion(),
false);
if (result != null) {
results = new ArrayList<>();
results.add(result);
return results;
}
}

// Attempt to resolve against sub signatures
results = subSignatures.resolve(callContext, conversionMap, operatorMap);

// If no subsignatures match, attempt subType match against this signature
if (results == null && operator.getSignature().isSuperTypeOf(invocationSignature)) {
results = new ArrayList<>();
results.add(new OperatorResolution(operator));
OperatorResolution result = getOperatorResolution(
operator,
callContext.getSignature(),
invocationSignature,
conversionMap,
operatorMap,
callContext.getAllowPromotionAndDemotion(),
false);
if (result != null) {
results = new ArrayList<>();
results.add(result);
return results;
}
}

if (results == null && conversionMap != null) {
// Attempt to find a conversion path from the call signature to the target signature
Conversion[] conversions =
new Conversion[operator.getSignature().getSize()];
boolean isConvertible = callContext
.getSignature()
.isConvertibleTo(
operator.getSignature(),
conversionMap,
operatorMap,
callContext.getAllowPromotionAndDemotion(),
conversions);
if (isConvertible) {
OperatorResolution resolution = new OperatorResolution(operator);
resolution.setConversions(conversions);
results = new ArrayList<>();
results.add(resolution);
OperatorResolution result = getOperatorResolution(
operator,
callContext.getSignature(),
invocationSignature,
conversionMap,
operatorMap,
callContext.getAllowPromotionAndDemotion(),
true);
if (result != null) {
if (results == null) {
results = new ArrayList<>();
}
results.add(result);
}
}

return results;
}

private Conversion[] getConversions(
Signature callSignature,
Signature operatorSignature,
ConversionMap conversionMap,
OperatorMap operatorMap,
boolean allowPromotionAndDemotion) {
if (callSignature == null
|| operatorSignature == null
|| callSignature.getSize() != operatorSignature.getSize()) {
return null;
}

Conversion[] conversions = new Conversion[callSignature.getSize()];
boolean isConvertible = callSignature.isConvertibleTo(
operatorSignature, conversionMap, operatorMap, allowPromotionAndDemotion, conversions);

if (isConvertible) {
return conversions;
}

return null;
}

private SignatureNodes subSignatures = new SignatureNodes();

public boolean hasSubSignatures() {
Expand Down
5 changes: 4 additions & 1 deletion Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/OperatorResolution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
public class OperatorResolution {
public OperatorResolution() {}

public OperatorResolution(Operator operator) {
public OperatorResolution(Operator operator, Conversion[] conversions) {
this.operator = operator;
if (conversions != null) {
setConversions(conversions);
}
}

private Operator operator;
Expand Down
77 changes: 77 additions & 0 deletions Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/TranslationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import java.util.stream.Collectors;
import org.cqframework.cql.elm.tracking.TrackBack;
import org.hamcrest.Matchers;
import org.hl7.cql.model.IntervalType;
import org.hl7.cql.model.SimpleType;
import org.hl7.cql_annotations.r1.CqlToElmInfo;
import org.hl7.elm.r1.*;
import org.junit.jupiter.api.Disabled;
Expand Down Expand Up @@ -329,6 +331,81 @@ void multiThreadedTranslation() throws IOException {
CompletableFuture.allOf(cfs).join();
}

@Test
void resolutionProperlyIncludesTests() throws IOException {
final CqlTranslator translator = TestUtils.runSemanticTest("ResolutionTests/ProperlyIncludesTests.cql", 0);
Library compiledLibrary = translator.getTranslatedLibrary().getLibrary();
List<ExpressionDef> statements = compiledLibrary.getStatements().getDef();

assertThat(statements.size(), equalTo(5));

ExpressionDef test = statements.get(0);
assertThat(test.getExpression(), instanceOf(ProperContains.class));
ProperContains properContains = (ProperContains)test.getExpression();
assertThat(properContains.getOperand().get(0), instanceOf(Interval.class));
Interval interval = (Interval)properContains.getOperand().get(0);
assertThat(interval.getResultType(), instanceOf(IntervalType.class));
IntervalType intervalType = (IntervalType)interval.getResultType();
assertThat(intervalType.getPointType(), instanceOf(SimpleType.class));
SimpleType pointType = (SimpleType)intervalType.getPointType();
assertThat(pointType.getName(), equalTo("System.Integer"));
assertThat(properContains.getOperand().get(1), instanceOf(As.class));
As _as = (As)properContains.getOperand().get(1);
assertThat(_as.getAsType().toString(), equalTo("{urn:hl7-org:elm-types:r1}Integer"));
assertThat(_as.getOperand(), instanceOf(Null.class));

test = statements.get(1);
assertThat(test.getExpression(), instanceOf(ProperContains.class));
properContains = (ProperContains)test.getExpression();
assertThat(properContains.getOperand().get(0), instanceOf(Interval.class));
interval = (Interval)properContains.getOperand().get(0);
assertThat(interval.getResultType(), instanceOf(IntervalType.class));
intervalType = (IntervalType)interval.getResultType();
assertThat(intervalType.getPointType(), instanceOf(SimpleType.class));
pointType = (SimpleType)intervalType.getPointType();
assertThat(pointType.getName(), equalTo("System.Integer"));
assertThat(properContains.getOperand().get(1), instanceOf(As.class));
_as = (As)properContains.getOperand().get(1);
assertThat(_as.getAsType().toString(), equalTo("{urn:hl7-org:elm-types:r1}Integer"));
assertThat(_as.getOperand(), instanceOf(Null.class));

test = statements.get(2);
assertThat(test.getExpression(), instanceOf(ProperContains.class));
properContains = (ProperContains)test.getExpression();
assertThat(properContains.getOperand().get(0), instanceOf(Interval.class));
interval = (Interval)properContains.getOperand().get(0);
assertThat(interval.getResultType(), instanceOf(IntervalType.class));
intervalType = (IntervalType)interval.getResultType();
assertThat(intervalType.getPointType(), instanceOf(SimpleType.class));
pointType = (SimpleType)intervalType.getPointType();
assertThat(pointType.getName(), equalTo("System.Any"));
assertThat(properContains.getOperand().get(1), instanceOf(Null.class));

test = statements.get(3);
assertThat(test.getExpression(), instanceOf(ProperContains.class));
properContains = (ProperContains)test.getExpression();
assertThat(properContains.getOperand().get(0), instanceOf(Interval.class));
interval = (Interval)properContains.getOperand().get(0);
assertThat(interval.getResultType(), instanceOf(IntervalType.class));
intervalType = (IntervalType)interval.getResultType();
assertThat(intervalType.getPointType(), instanceOf(SimpleType.class));
pointType = (SimpleType)intervalType.getPointType();
assertThat(pointType.getName(), equalTo("System.Any"));
assertThat(properContains.getOperand().get(1), instanceOf(Null.class));

test = statements.get(4);
assertThat(test.getExpression(), instanceOf(ProperContains.class));
properContains = (ProperContains)test.getExpression();
assertThat(properContains.getOperand().get(0), instanceOf(Interval.class));
interval = (Interval)properContains.getOperand().get(0);
assertThat(interval.getResultType(), instanceOf(IntervalType.class));
intervalType = (IntervalType)interval.getResultType();
assertThat(intervalType.getPointType(), instanceOf(SimpleType.class));
pointType = (SimpleType)intervalType.getPointType();
assertThat(pointType.getName(), equalTo("System.Integer"));
assertThat(properContains.getOperand().get(1), instanceOf(As.class));
}

@Test
void hidingVariousUseCases() throws IOException {
final CqlTranslator translator = TestUtils.runSemanticTest("HidingTests/TestHidingVariousUseCases.cql", 0);
Expand Down
20 changes: 10 additions & 10 deletions Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/fhir/r4/BaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ void fhirTiming() throws IOException {
ExpressionDef def = (ExpressionDef) visitFile("fhir/r4/TestFHIRTiming.cql");
// Query->
// where->
// IncludedIn->
// In->
// left->
// ToInterval()
// As(fhir:Period) ->
// ToDateTime()
// As(fhir:dateTime) ->
// Property(P.performed)
// right-> MeasurementPeriod
Query query = (Query) def.getExpression();
Expand All @@ -69,16 +69,16 @@ void fhirTiming() throws IOException {
Retrieve request = (Retrieve) source.getExpression();
assertThat(request.getDataType(), quickDataType("Procedure"));

// Then check that the where an IncludedIn with a ToInterval as the left operand
// Then check that the where is an In with a ToDateTime as the left operand
Expression where = query.getWhere();
assertThat(where, instanceOf(IncludedIn.class));
IncludedIn includedIn = (IncludedIn) where;
assertThat(includedIn.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) includedIn.getOperand().get(0);
assertThat(functionRef.getName(), is("ToInterval"));
assertThat(where, instanceOf(In.class));
In in = (In) where;
assertThat(in.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) in.getOperand().get(0);
assertThat(functionRef.getName(), is("ToDateTime"));
assertThat(functionRef.getOperand().get(0), instanceOf(As.class));
As asExpression = (As) functionRef.getOperand().get(0);
assertThat(asExpression.getAsType().getLocalPart(), is("Period"));
assertThat(asExpression.getAsType().getLocalPart(), is("dateTime"));
assertThat(asExpression.getOperand(), instanceOf(Property.class));
Property property = (Property) asExpression.getOperand();
assertThat(property.getScope(), is("P"));
Expand Down
20 changes: 10 additions & 10 deletions Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/fhir/r401/BaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ void fhirTiming() throws IOException {
ExpressionDef def = (ExpressionDef) visitFile("fhir/r401/TestFHIRTiming.cql");
// Query->
// where->
// IncludedIn->
// In->
// left->
// ToInterval()
// As(fhir:Period) ->
// ToDateTime()
// As(fhir:dateTime) ->
// Property(P.performed)
// right-> MeasurementPeriod
Query query = (Query) def.getExpression();
Expand All @@ -73,16 +73,16 @@ void fhirTiming() throws IOException {
Retrieve request = (Retrieve) source.getExpression();
assertThat(request.getDataType(), quickDataType("Procedure"));

// Then check that the where an IncludedIn with a ToInterval as the left operand
// Then check that the where is an In with a ToDateTime as the left operand
Expression where = query.getWhere();
assertThat(where, instanceOf(IncludedIn.class));
IncludedIn includedIn = (IncludedIn) where;
assertThat(includedIn.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) includedIn.getOperand().get(0);
assertThat(functionRef.getName(), is("ToInterval"));
assertThat(where, instanceOf(In.class));
In in = (In) where;
assertThat(in.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) in.getOperand().get(0);
assertThat(functionRef.getName(), is("ToDateTime"));
assertThat(functionRef.getOperand().get(0), instanceOf(As.class));
As asExpression = (As) functionRef.getOperand().get(0);
assertThat(asExpression.getAsType().getLocalPart(), is("Period"));
assertThat(asExpression.getAsType().getLocalPart(), is("dateTime"));
assertThat(asExpression.getOperand(), instanceOf(Property.class));
Property property = (Property) asExpression.getOperand();
assertThat(property.getScope(), is("P"));
Expand Down
18 changes: 9 additions & 9 deletions Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/fhir/stu3/BaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ void fhirTiming() throws IOException {
ExpressionDef def = (ExpressionDef) visitFile("fhir/stu3/TestFHIRTiming.cql");
// Query->
// where->
// IncludedIn->
// In->
// left->
// ToInterval()
// As(fhir:Period) ->
// ToDateTime()
// As(fhir:dateTime) ->
// Property(P.performed)
// right-> MeasurementPeriod
Query query = (Query) def.getExpression();
Expand All @@ -67,14 +67,14 @@ void fhirTiming() throws IOException {

// Then check that the where an IncludedIn with a Case as the left operand
Expression where = query.getWhere();
assertThat(where, instanceOf(IncludedIn.class));
IncludedIn includedIn = (IncludedIn) where;
assertThat(includedIn.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) includedIn.getOperand().get(0);
assertThat(functionRef.getName(), is("ToInterval"));
assertThat(where, instanceOf(In.class));
In in = (In) where;
assertThat(in.getOperand().get(0), instanceOf(FunctionRef.class));
FunctionRef functionRef = (FunctionRef) in.getOperand().get(0);
assertThat(functionRef.getName(), is("ToDateTime"));
assertThat(functionRef.getOperand().get(0), instanceOf(As.class));
As asExpression = (As) functionRef.getOperand().get(0);
assertThat(asExpression.getAsType().getLocalPart(), is("Period"));
assertThat(asExpression.getAsType().getLocalPart(), is("dateTime"));
assertThat(asExpression.getOperand(), instanceOf(Property.class));
Property property = (Property) asExpression.getOperand();
assertThat(property.getScope(), is("P"));
Expand Down
Loading

0 comments on commit 49ff33b

Please sign in to comment.