Fix operator resolution involving generics and supertypes

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/GenericOperator.java

@@ -54,15 +54,15 @@ public InstantiationResult instantiate(
         InstantiationContextImpl context =
                 new InstantiationContextImpl(typeMap, operatorMap, conversionMap, allowPromotionAndDemotion);
 
-        Boolean instantiable = getSignature().isInstantiable(callSignature, context);
-        if (instantiable) {
-            Operator result = new Operator(
-                    getName(),
-                    getSignature().instantiate(context),
-                    getResultType().instantiate(context));
-            result.setAccessLevel(getAccessLevel());
-            result.setLibraryName(getLibraryName());
-            return new InstantiationResult(this, result, context.getConversionScore());
+        var signatureInstantiated = getSignature().instantiate(callSignature, context);
+        if (signatureInstantiated != null) {
+            var resultTypeInstantiated = getResultType().instantiate(null, context);
+            if (resultTypeInstantiated != null) {
+                Operator result = new Operator(getName(), signatureInstantiated, resultTypeInstantiated);
+                result.setAccessLevel(getAccessLevel());
+                result.setLibraryName(getLibraryName());
+                return new InstantiationResult(this, result, context.getConversionScore());
+            }
         }
 
         return new InstantiationResult(this, null, context.getConversionScore());

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/InstantiationContextImpl.java

@@ -39,28 +39,33 @@ public int getConversionScore() {
     }
 
     @Override
-    public boolean isInstantiable(TypeParameter parameter, DataType callType) {
+    public DataType instantiate(TypeParameter parameter, DataType callType) {
         // If the type is not yet bound, bind it to the call type.
         DataType boundType = typeMap.get(parameter);
+
+        if (callType == null) {
+            return boundType;
+        }
+
         if (boundType == null) {
             if (parameter.canBind(callType)) {
                 typeMap.put(parameter, callType);
-                return true;
+                return callType;
             } else {
-                return false;
+                return null;
             }
         } else {
             // If the type is bound, and is a super type of the call type, return true;
             if (boundType.isSuperTypeOf(callType) || callType.isCompatibleWith(boundType)) {
-                return true;
+                return callType;
             } else if (callType.isSuperTypeOf(boundType) || boundType.isCompatibleWith(callType)) {
                 // If the call type is a super type of the bound type, switch the bound type for this parameter to the
                 // call type
                 if (parameter.canBind(callType)) {
                     typeMap.put(parameter, callType);
-                    return true;
+                    return callType;
                 } else {
-                    return false;
+                    return null;
                 }
             } else {
                 // If there is an implicit conversion path from the call type to the bound type, return true
@@ -77,13 +82,13 @@ public boolean isInstantiable(TypeParameter parameter, DataType callType) {
                                 conversionScore -=
                                         ConversionMap.ConversionScore.ListPromotion
                                                 .score(); // This removes the list promotion
-                                return true;
+                                return callType;
                             } else {
-                                return false;
+                                return null;
                             }
                         }
                     }
-                    return true;
+                    return callType;
                 }
 
                 // If there is an implicit conversion path from the bound type to the call type
@@ -97,9 +102,9 @@ public boolean isInstantiable(TypeParameter parameter, DataType callType) {
                                 ? ConversionMap.ConversionScore.SimpleConversion.score()
                                 : ConversionMap.ConversionScore.ComplexConversion
                                         .score()); // This removes the conversion from the instantiation
-                        return true;
+                        return callType;
                     } else {
-                        return false;
+                        return null;
                     }
                 }
 
@@ -132,18 +137,7 @@ public boolean isInstantiable(TypeParameter parameter, DataType callType) {
             }
         }
 
-        return false;
-    }
-
-    @Override
-    public DataType instantiate(TypeParameter parameter) {
-        DataType result = typeMap.get(parameter);
-        if (result == null) {
-            throw new IllegalArgumentException(
-                    String.format("Could not resolve type parameter %s.", parameter.getIdentifier()));
-        }
-
-        return result;
+        return null;
     }
 
     @Override
@@ -233,62 +227,4 @@ public Iterable<ListType> getListConversionTargets(DataType callType) {
 
         return results;
     }
-
-    @Override
-    public Iterable<SimpleType> getSimpleConversionTargets(DataType callType) {
-        ArrayList<SimpleType> results = new ArrayList<SimpleType>();
-        for (Conversion c : conversionMap.getConversions(callType)) {
-            if (c.getToType() instanceof SimpleType) {
-                results.add((SimpleType) c.getToType());
-                conversionScore += ConversionMap.ConversionScore.SimpleConversion.score();
-            }
-        }
-
-        if (results.isEmpty()) {
-            for (Conversion c : conversionMap.getGenericConversions()) {
-                if (c.getOperator() != null) {
-                    if (c.getToType() instanceof SimpleType) {
-                        InstantiationResult instantiationResult = ((GenericOperator) c.getOperator())
-                                .instantiate(new Signature(callType), operatorMap, conversionMap, false);
-                        Operator operator = instantiationResult.getOperator();
-                        // TODO: Consider impact of conversion score of the generic instantiation on this conversion
-                        // score
-                        if (operator != null) {
-                            operatorMap.addOperator(operator);
-                            Conversion conversion = new Conversion(operator, true);
-                            conversionMap.add(conversion);
-                            results.add((SimpleType) conversion.getToType());
-                        }
-                    }
-                }
-            }
-        }
-
-        // Add interval demotion if no other conversion is found
-        if (results.isEmpty()) {
-            if (callType instanceof IntervalType) {
-                IntervalType callIntervalType = (IntervalType) callType;
-                if (callIntervalType.getPointType() instanceof SimpleType
-                        && (allowPromotionAndDemotion || conversionMap.isIntervalDemotionEnabled())) {
-                    results.add((SimpleType) callIntervalType.getPointType());
-                    conversionScore += ConversionMap.ConversionScore.IntervalDemotion.score();
-                }
-            }
-        }
-
-        // NOTE: FHIRPath Support
-        // Add list demotion if no other conversion is found
-        if (results.isEmpty()) {
-            if (callType instanceof ListType) {
-                ListType callListType = (ListType) callType;
-                if (callListType.getElementType() instanceof SimpleType
-                        && (allowPromotionAndDemotion || conversionMap.isListDemotionEnabled())) {
-                    results.add((SimpleType) callListType.getElementType());
-                    conversionScore += ConversionMap.ConversionScore.ListDemotion.score();
-                }
-            }
-        }
-
-        return results;
-    }
 }

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/OperatorEntry.java

@@ -277,39 +277,26 @@ public List<OperatorResolution> resolve(
             throw new IllegalArgumentException("callContext is null");
         }
 
-        List<OperatorResolution> results = signatures.resolve(callContext, conversionMap, operatorMap);
-
-        // If there is no resolution, or all resolutions require conversion, attempt to instantiate a generic signature
-        if (results == null || allResultsUseConversion(results)) {
-            // If the callContext signature contains choices, attempt instantiation with all possible combinations of
-            // the call signature (ouch, this could really hurt...)
-            boolean signaturesInstantiated = false;
-            List<Signature> callSignatures = expandChoices(callContext.getSignature());
-            for (Signature callSignature : callSignatures) {
-                Operator result = instantiate(
-                        callSignature, operatorMap, conversionMap, callContext.getAllowPromotionAndDemotion());
-                if (result != null && !signatures.contains(result)) {
-                    // If the generic signature was instantiated, store it as an actual signature.
-                    signatures.add(new SignatureNode(result));
-                    signaturesInstantiated = true;
-                }
-            }
-
-            // re-attempt the resolution with the instantiated signature registered
-            if (signaturesInstantiated) {
-                results = signatures.resolve(callContext, conversionMap, operatorMap);
+        // If the callContext signature contains choices, attempt instantiation with all possible combinations of
+        // the call signature (ouch, this could really hurt...)
+        List<Signature> callSignatures = expandChoices(callContext.getSignature());
+        for (Signature callSignature : callSignatures) {
+            Operator result =
+                    instantiate(callSignature, operatorMap, conversionMap, callContext.getAllowPromotionAndDemotion());
+            if (result != null && !signatures.contains(result)) {
+                // If the generic signature was instantiated, store it as an actual signature.
+                signatures.add(new SignatureNode(result));
             }
         }
 
-        return results;
+        return signatures.resolve(callContext, conversionMap, operatorMap);
     }
 
     private Operator instantiate(
             Signature signature,
             OperatorMap operatorMap,
             ConversionMap conversionMap,
             boolean allowPromotionAndDemotion) {
-        List<Operator> instantiations = new ArrayList<Operator>();
         int lowestConversionScore = Integer.MAX_VALUE;
         Operator instantiation = null;
         for (GenericOperator genericOperator : genericOperators.values()) {

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/model/Signature.java

@@ -80,27 +80,22 @@ public boolean isSubTypeOf(Signature other) {
         return false;
     }
 
-    public boolean isInstantiable(Signature callSignature, InstantiationContext context) {
+    public Signature instantiate(Signature callSignature, InstantiationContext context) {
         if (operandTypes.size() == callSignature.operandTypes.size()) {
+            DataType[] result = new DataType[operandTypes.size()];
             for (int i = 0; i < operandTypes.size(); i++) {
-                if (!operandTypes.get(i).isInstantiable(callSignature.operandTypes.get(i), context)) {
-                    return false;
+                var operandTypeInstantiated =
+                        operandTypes.get(i).instantiate(callSignature.operandTypes.get(i), context);
+                if (operandTypeInstantiated == null) {
+                    return null;
                 }
+                result[i] = operandTypeInstantiated;
             }
 
-            return true;
-        }
-
-        return false;
-    }
-
-    public Signature instantiate(InstantiationContext context) {
-        DataType[] result = new DataType[operandTypes.size()];
-        for (int i = 0; i < operandTypes.size(); i++) {
-            result[i] = operandTypes.get(i).instantiate(context);
+            return new Signature(result);
         }
 
-        return new Signature(result);
+        return null;
     }
 
     public boolean isConvertibleTo(

Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/model/OperatorMapTest.java

@@ -0,0 +1,124 @@
+package org.cqframework.cql.cql2elm.model;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.cqframework.cql.cql2elm.ModelManager;
+import org.hl7.cql.model.TypeParameter;
+import org.junit.jupiter.api.Test;
+
+class OperatorMapTest {
+    private final ModelManager modelManager = new ModelManager();
+    private final Model systemModel = modelManager.resolveModel("System");
+    private final Model fhirModel = modelManager.resolveModel("FHIR", "4.0.1");
+    private final String libraryName = "TestLibrary";
+
+    @Test
+    void resolveOperator() {
+        var systemAny = systemModel.resolveTypeName("Any");
+        var systemBoolean = systemModel.resolveTypeName("Boolean");
+        var systemInteger = systemModel.resolveTypeName("Integer");
+        var fhirQuantity = fhirModel.resolveTypeName("Quantity");
+        var fhirDistance = fhirModel.resolveTypeName("Distance"); // Subtype of FHIR.Quantity
+
+        var operatorMap = new OperatorMap();
+        var conversionMap = new ConversionMap();
+
+        // Define operators:
+        // OperatorA : (System.Integer, System.Integer) -> System.Boolean
+        // OperatorB : (T, System.Integer) -> T
+        operatorMap.addOperator(createOperatorA());
+        operatorMap.addOperator(createOperatorB());
+
+        // When passed in (System.Integer, System.Integer), OperatorA should resolve with the
+        // (System.Integer, System.Integer) -> System.Boolean signature.
+        var resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorA", false, false, false, systemInteger, systemInteger),
+                conversionMap);
+        assertNotNull(resolution);
+        assertEquals(
+                new Signature(systemInteger, systemInteger),
+                resolution.getOperator().getSignature());
+        assertEquals(systemBoolean, resolution.getOperator().getResultType());
+
+        // When passed in (System.Boolean, System.Boolean), OperatorA should not resolve.
+        resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorA", false, false, false, systemBoolean, systemBoolean),
+                conversionMap);
+        assertNull(resolution);
+
+        // When passed in (System.Boolean, System.Any), OperatorB should resolve with the
+        // (System.Boolean, System.Integer) -> System.Boolean signature.
+        // System.Integer must be instantiable from System.Any for this to work.
+        resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorB", false, false, false, systemBoolean, systemAny),
+                conversionMap);
+        assertNotNull(resolution);
+        assertEquals(
+                new Signature(systemBoolean, systemInteger),
+                resolution.getOperator().getSignature());
+        assertEquals(systemBoolean, resolution.getOperator().getResultType());
+
+        // When later passed in (System.Boolean, System.Integer), OperatorB should resolve with the
+        // (System.Boolean, System.Integer) -> System.Boolean signature.
+        resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorB", false, false, false, systemBoolean, systemInteger),
+                conversionMap);
+        assertNotNull(resolution);
+        assertEquals(
+                new Signature(systemBoolean, systemInteger),
+                resolution.getOperator().getSignature());
+        assertEquals(systemBoolean, resolution.getOperator().getResultType());
+
+        // When passed in (FHIR.Quantity, System.Integer), OperatorB should resolve with the
+        // (FHIR.Quantity, System.Integer) -> FHIR.Quantity signature.
+        resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorB", false, false, false, fhirQuantity, systemInteger),
+                conversionMap);
+        assertNotNull(resolution);
+        assertEquals(
+                new Signature(fhirQuantity, systemInteger),
+                resolution.getOperator().getSignature());
+        assertEquals(fhirQuantity, resolution.getOperator().getResultType());
+
+        // When later passed in (FHIR.Distance, System.Integer), OperatorB should resolve with the
+        // (FHIR.Distance, System.Integer) -> FHIR.Distance signature.
+        // Previous instantiation of (FHIR.Quantity, System.Integer) for OperatorB must not affect this resolution.
+        resolution = operatorMap.resolveOperator(
+                new CallContext(libraryName, "OperatorB", false, false, false, fhirDistance, systemInteger),
+                conversionMap);
+        assertNotNull(resolution);
+        assertEquals(
+                new Signature(fhirDistance, systemInteger),
+                resolution.getOperator().getSignature());
+        assertEquals(fhirDistance, resolution.getOperator().getResultType());
+    }
+
+    /**
+     * Create OperatorA : (Integer, Integer) -> Boolean
+     */
+    private Operator createOperatorA() {
+        var systemBoolean = systemModel.resolveTypeName("Boolean");
+        var systemInteger = systemModel.resolveTypeName("Integer");
+
+        var operator = new Operator("OperatorA", new Signature(systemInteger, systemInteger), systemBoolean);
+        operator.setLibraryName(libraryName);
+
+        return operator;
+    }
+
+    /**
+     * Create OperatorB : (T, Integer) -> T
+     */
+    private GenericOperator createOperatorB() {
+        var systemInteger = systemModel.resolveTypeName("Integer");
+
+        var operator = new GenericOperator(
+                "OperatorB",
+                new Signature(new TypeParameter("T"), systemInteger),
+                new TypeParameter("T"),
+                new TypeParameter("T"));
+        operator.setLibraryName(libraryName);
+
+        return operator;
+    }
+}