Clean-up of TupleType (#1467)

* Clean-up of TupleType

* Fix hashcode

* Fix hashCode for TupleType, make error messages/types consistent, use compact syntax for single-line functions, start using string templates

* Fix exception message

* More cleanup

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/Cql2ElmVisitor.kt

@@ -630,13 +630,13 @@ class Cql2ElmVisitor(
 
     override fun visitTupleSelector(ctx: TupleSelectorContext): Any? {
         val tuple = of.createTuple()
-        val tupleType = TupleType()
+        val elements = mutableListOf<TupleTypeElement>()
         for (elementContext in ctx.tupleElementSelector()) {
             val element = visit(elementContext) as TupleElement
-            tupleType.addElement(TupleTypeElement(element.name, element.resultType!!))
+            elements.add(TupleTypeElement(element.name, element.resultType!!))
             tuple.element.add(element)
         }
-        tuple.resultType = tupleType
+        tuple.resultType = TupleType(elements)
         return tuple
     }
 
@@ -3522,7 +3522,7 @@ class Cql2ElmVisitor(
                 if (agg == null && ret == null && sources.size > 1) {
                     ret = of.createReturnClause().withDistinct(true)
                     val returnExpression = of.createTuple()
-                    val returnType = TupleType()
+                    val elements = mutableListOf<TupleTypeElement>()
                     for (aqs: AliasedQuerySource in sources) {
                         val element =
                             of.createTupleElement()
@@ -3534,9 +3534,10 @@ class Cql2ElmVisitor(
                         element.value.resultType =
                             sourceType // Doesn't use the fluent API to avoid casting
                         element.resultType = element.value.resultType
-                        returnType.addElement(TupleTypeElement(element.name, element.resultType!!))
+                        elements.add(TupleTypeElement(element.name, element.resultType!!))
                         returnExpression.element.add(element)
                     }
+                    val returnType = TupleType(elements)
                     returnExpression.resultType =
                         if (queryContext.isSingular) returnType else ListType(returnType)
                     ret.expression = returnExpression

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

@@ -227,15 +227,16 @@ class ModelImporter(val modelInfo: ModelInfo, val modelManager: ModelManager?) {
         }
 
         if (typeSpecifier is TupleTypeSpecifier) {
-            val tupleType = TupleType()
+            val elements = mutableListOf<TupleTypeElement>()
             for (specifierElement in typeSpecifier.element) {
                 val element =
                     TupleTypeElement(
                         specifierElement.name,
                         resolveTypeSpecifier(specifierElement.elementType)!!
                     )
-                tupleType.addElement(element)
+                elements.add(element)
             }
+            return TupleType(elements)
         }
 
         if (typeSpecifier is ChoiceTypeSpecifier) {

Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/preprocessor/CqlPreprocessorElmCommonVisitor.kt

@@ -151,14 +151,14 @@ abstract class CqlPreprocessorElmCommonVisitor(
     }
 
     override fun visitTupleTypeSpecifier(ctx: TupleTypeSpecifierContext): Any {
-        val resultType = TupleType()
+        val elements = mutableListOf<TupleTypeElement>()
         val typeSpecifier = of.createTupleTypeSpecifier()
         for (definitionContext in ctx.tupleElementDefinition()) {
             val element = visit(definitionContext) as TupleElementDefinition
-            resultType.addElement(TupleTypeElement(element.name, element.elementType.resultType!!))
+            elements.add(TupleTypeElement(element.name, element.elementType.resultType!!))
             typeSpecifier.element.add(element)
         }
-        typeSpecifier.resultType = resultType
+        typeSpecifier.resultType = TupleType(elements)
         return typeSpecifier
     }
 

Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/operators/QueryTest.java

@@ -76,7 +76,7 @@ void mixedMultipleSourceQuery() {
                 def,
                 hasTypeAndResult(
                         Query.class,
-                        "list<tuple{S:tuple{id:System.Integer,name:System.String},P:tuple{id:System.Integer,name:System.String}}>"));
+                        "list<tuple{P:tuple{id:System.Integer,name:System.String},S:tuple{id:System.Integer,name:System.String}}>"));
     }
 
     @Test

Src/java/elm-fhir/src/main/java/org/cqframework/cql/elm/requirements/TypeResolver.java

@@ -133,13 +133,13 @@ private DataType resolveNamedTypeSpecifier(NamedTypeSpecifier typeSpecifier) {
     }
 
     private DataType resolveTupleTypeSpecifier(TupleTypeSpecifier typeSpecifier) {
-        TupleType tupleType = new TupleType();
+        var elements = new ArrayList<TupleTypeElement>();
         for (TupleElementDefinition element : typeSpecifier.getElement()) {
             TupleTypeElement tupleElement =
                     new TupleTypeElement(element.getName(), resolveTypeSpecifier(element.getElementType()), false);
-            tupleType.addElement(tupleElement);
+            elements.add(tupleElement);
         }
-        return tupleType;
+        return new TupleType(elements);
     }
 
     private DataType resolveIntervalTypeSpecifier(IntervalTypeSpecifier typeSpecifier) {

Src/java/model/src/main/java/org/hl7/cql/model/BaseDataType.kt

@@ -3,9 +3,7 @@ package org.hl7.cql.model
 abstract class BaseDataType protected constructor(baseType: DataType? = DataType.ANY) : DataType {
     override val baseType: DataType = baseType ?: DataType.ANY
 
-    override fun toLabel(): String {
-        return toString()
-    }
+    override fun toLabel(): String = toString()
 
     override fun isSubTypeOf(other: DataType): Boolean {
         var currentType: DataType = this
@@ -51,11 +49,11 @@ abstract class BaseDataType protected constructor(baseType: DataType? = DataType
     // type compatibility is used to support implicit casting, such as casting a "null"
     // literal to any other type, or casting a class to an equivalent tuple.
     override fun isCompatibleWith(other: DataType): Boolean {
-        return when {
+        return when (other) {
             // Any data type is compatible with itself
-            this == other -> true
+            this -> true
             // A type is compatible with a choice type if it is a subtype of one of the choice types
-            other is ChoiceType -> other.types.any { this.isSubTypeOf(it) }
+            is ChoiceType -> other.types.any { this.isSubTypeOf(it) }
             else -> false
         }
     }

Src/java/model/src/main/java/org/hl7/cql/model/ChoiceType.kt

@@ -16,14 +16,11 @@ data class ChoiceType private constructor(val types: Set<DataType>) : BaseDataTy
         }
     }
 
-    fun isSubSetOf(other: ChoiceType): Boolean {
-        // every type in this choice is a subtype of some type in the other choice
-        return types.all { x -> other.types.any { x.isSubTypeOf(it) } }
-    }
+    // every type in this choice is a subtype of some type in the other choice
+    fun isSubSetOf(other: ChoiceType): Boolean =
+        types.all { x -> other.types.any { x.isSubTypeOf(it) } }
 
-    fun isSuperSetOf(other: ChoiceType): Boolean {
-        return other.isSubSetOf(this)
-    }
+    fun isSuperSetOf(other: ChoiceType): Boolean = other.isSubSetOf(this)
 
     override fun isCompatibleWith(other: DataType): Boolean {
         // This type is compatible with the other type if
@@ -38,9 +35,7 @@ data class ChoiceType private constructor(val types: Set<DataType>) : BaseDataTy
         }
     }
 
-    override fun toString(): String {
-        return types.joinToString(",", "choice<", ">")
-    }
+    override fun toString(): String = types.joinToString(",", "choice<", ">")
 
     override val isGeneric: Boolean = types.any { it.isGeneric }
 

Src/java/model/src/main/java/org/hl7/cql/model/ClassType.kt

@@ -19,7 +19,7 @@ constructor(
     var genericParameters: MutableList<TypeParameter> = mutableListOf()
 ) : BaseDataType(baseType), NamedType {
     init {
-        require(name.isNotEmpty()) { "A class type must have a name." }
+        require(name.isNotEmpty()) { "name can not be empty" }
     }
 
     override val namespace: String
@@ -80,7 +80,7 @@ constructor(
     }
 
     fun findSearch(searchPath: String): SearchType? {
-        return searches.firstOrNull() { it.name == searchPath }
+        return searches.firstOrNull { it.name == searchPath }
     }
 
     /**
@@ -142,7 +142,7 @@ constructor(
     }
 
     val sortedElements: List<ClassTypeElement>
-        get() = elements.sortedWith { o1, o2 -> o1.name.compareTo(o2.name) }
+        get() = elements.sortedWith(compareBy { it.name })
 
     private var baseElementMap: LinkedHashMap<String, ClassTypeElement>? = null
         get() {
@@ -230,8 +230,7 @@ constructor(
 
     override fun toLabel(): String = label ?: name
 
-    val tupleType: TupleType
-        get() = buildTupleType()
+    val tupleType: TupleType by lazy { buildTupleType() }
 
     private fun addTupleElements(
         classType: ClassType,
@@ -253,10 +252,8 @@ constructor(
 
     private fun buildTupleType(): TupleType {
         val tupleElements = LinkedHashMap<String, TupleTypeElement>()
-
         addTupleElements(this, tupleElements)
-
-        return TupleType(tupleElements.values.toMutableList())
+        return TupleType(tupleElements.values)
     }
 
     override fun isCompatibleWith(other: DataType): Boolean {

Src/java/model/src/main/java/org/hl7/cql/model/ClassTypeElement.kt

@@ -11,26 +11,22 @@ data class ClassTypeElement(
 ) {
 
     init {
-        require(name.isNotEmpty()) { "A class type element must have a name." }
+        require(name.isNotEmpty()) { "name can not be empty" }
     }
 
-    fun isSubTypeOf(that: ClassTypeElement): Boolean {
-        return this.name == that.name && type.isSubTypeOf(that.type)
-    }
+    fun isSubTypeOf(that: ClassTypeElement): Boolean =
+        this.name == that.name && type.isSubTypeOf(that.type)
 
-    fun isSuperTypeOf(that: ClassTypeElement): Boolean {
-        return this.name == that.name && type.isSuperTypeOf(that.type)
-    }
+    fun isSuperTypeOf(that: ClassTypeElement): Boolean =
+        this.name == that.name && type.isSuperTypeOf(that.type)
 
     override fun toString(): String {
-        return String.format(
-            Locale.US,
-            "%s:%s%s%s%s",
-            this.name,
-            type.toString(),
-            if (this.prohibited) " (prohibited)" else "",
-            if (this.oneBased) " (one-based)" else "",
-            if (this.target != null) " (target: " + this.target + ")" else ""
-        )
+        return "$name:$type$%s%s%s"
+            .format(
+                Locale.US,
+                if (this.prohibited) " (prohibited)" else "",
+                if (this.oneBased) " (one-based)" else "",
+                if (this.target != null) " (target: " + this.target + ")" else ""
+            )
     }
 }

Src/java/model/src/main/java/org/hl7/cql/model/GenericClassSignatureParser.kt

@@ -93,7 +93,7 @@ class GenericClassSignatureParser(
     private fun handleParameterDeclaration(parameterString: String): TypeParameter {
         val paramComponents = parameterString.split("\\s+".toRegex()).dropLastWhile { it.isEmpty() }
         return if (paramComponents.size == 1) {
-            TypeParameter(parameterString.trim(), TypeParameter.TypeParameterConstraint.NONE, null)
+            TypeParameter(parameterString.trim(), TypeParameter.TypeParameterConstraint.NONE)
         } else if (paramComponents.size == 3) {
             if (paramComponents[1].equals(EXTENDS, ignoreCase = true)) {
                 TypeParameter(
@@ -138,49 +138,53 @@ class GenericClassSignatureParser(
     private fun handleBoundType(boundGenericSignature: String): DataType {
         var resolvedType =
             resolvedTypes[escapeNestedAngleBrackets(boundGenericSignature)] as? ClassType
-        if (resolvedType != null) {
-            return resolvedType
-        } else {
-            val genericTypeName =
-                boundGenericSignature.substring(0, boundGenericSignature.indexOf('<'))
-            resolvedType = resolveType(genericTypeName) as ClassType
-            val newType = ClassType(escapeNestedAngleBrackets(boundGenericSignature), resolvedType)
-            val parameters =
-                boundGenericSignature.substring(
-                    boundGenericSignature.indexOf('<') + 1,
-                    boundGenericSignature.lastIndexOf('>')
-                )
-            val params =
-                escapeNestedCommas(parameters).split(",".toRegex()).dropLastWhile { it.isEmpty() }
-            for ((index, param) in params.withIndex()) {
-                var boundParam: DataType?
-                val unescaped = unescapeNestedCommas(param)
-                boundParam =
-                    if (isValidGenericSignature(unescaped)) {
-                        handleBoundType(unescaped)
-                    } else {
-                        resolveType(unescaped)
+        return when {
+            resolvedType != null -> resolvedType
+            else -> {
+                val genericTypeName =
+                    boundGenericSignature.substring(0, boundGenericSignature.indexOf('<'))
+                resolvedType = resolveType(genericTypeName) as ClassType
+                val newType =
+                    ClassType(escapeNestedAngleBrackets(boundGenericSignature), resolvedType)
+                val parameters =
+                    boundGenericSignature.substring(
+                        boundGenericSignature.indexOf('<') + 1,
+                        boundGenericSignature.lastIndexOf('>')
+                    )
+                val params =
+                    escapeNestedCommas(parameters).split(",".toRegex()).dropLastWhile {
+                        it.isEmpty()
                     }
-                val typeParameter = resolvedType.genericParameters[index]
-                for ((name, type) in resolvedType.elements) {
-                    if (
-                        type is TypeParameter &&
-                            type.identifier.equals(typeParameter.identifier, ignoreCase = true)
-                    ) {
-                        val newElement =
-                            ClassTypeElement(
-                                name,
-                                boundParam,
-                                prohibited = false,
-                                oneBased = false,
-                                target = null
-                            )
-                        newType.addElement(newElement)
+                for ((index, param) in params.withIndex()) {
+                    var boundParam: DataType?
+                    val unescaped = unescapeNestedCommas(param)
+                    boundParam =
+                        if (isValidGenericSignature(unescaped)) {
+                            handleBoundType(unescaped)
+                        } else {
+                            resolveType(unescaped)
+                        }
+                    val typeParameter = resolvedType.genericParameters[index]
+                    for ((name, type) in resolvedType.elements) {
+                        if (
+                            type is TypeParameter &&
+                                type.identifier.equals(typeParameter.identifier, ignoreCase = true)
+                        ) {
+                            val newElement =
+                                ClassTypeElement(
+                                    name,
+                                    boundParam,
+                                    prohibited = false,
+                                    oneBased = false,
+                                    target = null
+                                )
+                            newType.addElement(newElement)
+                        }
                     }
                 }
+                resolvedTypes[newType.name] = newType
+                newType
             }
-            resolvedTypes[newType.name] = newType
-            return newType
         }
     }
 
@@ -260,12 +264,10 @@ class GenericClassSignatureParser(
         var openBracketCount = 0
         for (index in signatureCharArray.indices) {
             val c = signatureCharArray[index]
-            if (c == '<') {
-                openBracketCount++
-            } else if (c == '>') {
-                openBracketCount--
-            } else if (c == ',' && openBracketCount > 0) {
-                signatureCharArray[index] = '|'
+            when {
+                c == '<' -> openBracketCount++
+                c == '>' -> openBracketCount--
+                c == ',' && openBracketCount > 0 -> signatureCharArray[index] = '|'
             }
         }
         return String(signatureCharArray)