Skip to content

Commit 654e79e

Browse files
committed
Fix TupleAndClassConversions parity: model-aware type resolution in TypeResolver and ElmEmitter
Resolve model types (QUICK Element, Extension, etc.) through loaded models instead of only the system type namespace. This completes parity for the TupleAndClassConversions test, bringing results to 31/32 (1 intentional skip: Aggregate legacy bug #1710). TypeResolver: - Add resolveNamedType() and resolveModelType() for system+model lookup - Update resolveTypeSpecifier to handle qualified and unqualified model types - Update inferInstanceLiteralType to delegate to resolveTypeSpecifier EmissionContext: - Add resolveTypeQName() for name-to-QName with correct model namespace - Add dataTypeToQName() and dataTypeToTypeSpecifier() that bypass TypeBuilder's system-only ModelResolver for model types - Replace all operatorRegistry.typeBuilder.dataTypeToQName/TypeSpecifier calls across codegen package (EmissionContext, CollectionOperatorEmission, IntervalOperatorEmission) TypeOperatorEmission: - Update emitTypeSpecifier, emitImplicitCastExpression to use resolveTypeQName - Fix emitConversionExpression for class/tuple conversions: emit As with asType (QName) for named types, asTypeSpecifier for complex types, matching legacy buildAs behavior FullParityTest: - Register QuickModelInfoProvider in test model manager - Update results documentation
1 parent 8cd37a4 commit 654e79e

15 files changed

Lines changed: 513 additions & 123 deletions

File tree

Src/java/cql-to-elm/src/commonMain/kotlin/org/cqframework/cql/cql2elm/analysis/ExpressionLowering.kt

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,72 @@ class ExpressionLowering(
230230
target: Expression?,
231231
arguments: List<Expression>,
232232
): Expression {
233+
// CalculateAgeIn*At with mixed DateTime/Date args: enforce compatibility by
234+
// converting the DateTime operand to Date, matching the legacy translator's
235+
// behavior (SystemFunctionResolver.enforceCompatible). This ensures the (Date,Date)
236+
// overload is used rather than promoting Date→DateTime.
237+
val lowered = lowerCalculateAgeAtCompatibility(expr, arguments)
238+
if (lowered != null) return lowered
239+
233240
val argsChanged = arguments.indices.any { arguments[it] !== expr.arguments[it] }
234241
val targetChanged = target !== expr.target
235242
return if (!targetChanged && !argsChanged) expr
236243
else expr.copy(target = target, arguments = arguments)
237244
}
238245

246+
/**
247+
* For CalculateAgeIn{Years,Months,Weeks,Days}At calls where one arg is DateTime and the other
248+
* is Date, wrap the DateTime arg in ConversionExpression(ToDate) so both operands are Date.
249+
* This matches the CQL spec behavior: age calculations at day precision or coarser should
250+
* operate on Date, not DateTime.
251+
*
252+
* Returns null if no lowering is needed.
253+
*/
254+
private fun lowerCalculateAgeAtCompatibility(
255+
expr: FunctionCallExpression,
256+
arguments: List<Expression>,
257+
): Expression? {
258+
if (arguments.size != 2) return null
259+
val name = expr.function.value
260+
// Only apply to day-or-coarser precisions (Years, Months, Weeks, Days).
261+
// Hours, Minutes, Seconds require DateTime precision — no conversion needed.
262+
val isAgeAtDayOrCoarser =
263+
name.startsWith("CalculateAgeIn") &&
264+
name.endsWith("At") &&
265+
!name.contains("Hours") &&
266+
!name.contains("Minutes") &&
267+
!name.contains("Seconds")
268+
if (!isAgeAtDayOrCoarser) return null
269+
270+
val type0 = semanticModel[expr.arguments[0]]?.toString()
271+
val type1 = semanticModel[expr.arguments[1]]?.toString()
272+
if (type0 == null || type1 == null) return null
273+
274+
// If one is DateTime and the other is Date, convert the DateTime one to Date.
275+
val newArgs = arguments.toMutableList()
276+
if (type0 == "System.DateTime" && type1 == "System.Date") {
277+
newArgs[0] = wrapInToDate(arguments[0], expr)
278+
} else if (type0 == "System.Date" && type1 == "System.DateTime") {
279+
newArgs[1] = wrapInToDate(arguments[1], expr)
280+
} else {
281+
return null
282+
}
283+
// Don't use rewrite() here — we don't want to transfer the old synthetics
284+
// (e.g., Date→DateTime on arg[1]) since the new args are both Date and the
285+
// (Date,Date) overload matches exactly with no conversions needed.
286+
return expr.copy(arguments = newArgs)
287+
}
288+
289+
private fun wrapInToDate(operand: Expression, context: Expression): ConversionExpression =
290+
ConversionExpression(
291+
operand = operand,
292+
destinationType =
293+
org.hl7.cql.ast.NamedTypeSpecifier(
294+
name = org.hl7.cql.ast.QualifiedIdentifier(listOf("Date"))
295+
),
296+
locator = context.locator,
297+
)
298+
239299
override fun onPropertyAccess(expr: PropertyAccessExpression, target: Expression) =
240300
if (target === expr.target) expr else expr.copy(target = target)
241301

@@ -428,19 +488,24 @@ class ExpressionLowering(
428488
left: Expression,
429489
right: Expression,
430490
): Expression {
431-
// Interval<Any> expansion: when one operand is Interval<Any> (non-literal) and the
432-
// other has a concrete point type, expand using PropertyAccessExpression + AsExpression.
491+
// Interval type promotion: when both operands are non-literal intervals with
492+
// different point types, expand the one needing conversion into bound access +
493+
// conversion + reconstruction. This handles Interval<Date> → Interval<DateTime>
494+
// for operators like IncludedIn/Includes. The IntervalConversion synthetic tells
495+
// us which slot needs conversion and what operator to use.
433496
var loweredLeft = left
434497
var loweredRight = right
435498
val leftType = semanticModel[expr.left]
436499
val rightType = semanticModel[expr.right]
500+
501+
// Interval<Any> expansion: when one operand is Interval<Any> (non-literal) and the
502+
// other has a concrete point type, expand using PropertyAccessExpression + AsExpression.
437503
if (
438504
leftType is IntervalType &&
439505
rightType is IntervalType &&
440506
rightType.pointType.toString() == "System.Any" &&
441507
leftType.pointType.toString() != "System.Any" &&
442-
!(expr.right is LiteralExpression &&
443-
(expr.right as LiteralExpression).literal is org.hl7.cql.ast.IntervalLiteral)
508+
!isIntervalLiteral(expr.right)
444509
) {
445510
loweredRight = expandIntervalToType(right, leftType.pointType)
446511
}
@@ -853,6 +918,9 @@ class ExpressionLowering(
853918
)
854919
}
855920

921+
private fun isIntervalLiteral(expr: Expression): Boolean =
922+
expr is LiteralExpression && expr.literal is org.hl7.cql.ast.IntervalLiteral
923+
856924
override fun onQuery(expr: QueryExpression, children: QueryChildren<Expression>) = expr
857925

858926
override fun onRetrieve(expr: RetrieveExpression) = expr

Src/java/cql-to-elm/src/commonMain/kotlin/org/cqframework/cql/cql2elm/analysis/SemanticAnalyzer.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class SemanticAnalyzer(
5757
val cumulativeTypeTable = TypeTable()
5858
var totalConversions = 0
5959
val conversionsPerIteration = mutableListOf<Int>()
60-
val maxIterations = 3
60+
val maxIterations = 2
6161
val syntheticTable = SyntheticTable()
6262

6363
for (iteration in 1..maxIterations) {

Src/java/cql-to-elm/src/commonMain/kotlin/org/cqframework/cql/cql2elm/analysis/SemanticValidator.kt

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,16 @@ private class ExpressionChecker(
231231

232232
override fun onExternalConstant(expr: ExternalConstantExpression) {} // leaf
233233

234-
override fun onBinaryOperator(expr: OperatorBinaryExpression, left: Unit, right: Unit) {}
234+
override fun onBinaryOperator(expr: OperatorBinaryExpression, left: Unit, right: Unit) {
235+
// If both operands have types but the operator couldn't resolve, it's a type error
236+
// (e.g., Equal(Concept, List<Code>) — no implicit conversion exists).
237+
if (model[expr] != null) return
238+
val leftType = model[expr.left]
239+
val rightType = model[expr.right]
240+
if (leftType != null && rightType != null) {
241+
model.addError(expr)
242+
}
243+
}
235244

236245
override fun onUnaryOperator(expr: OperatorUnaryExpression, operand: Unit) {}
237246

@@ -298,6 +307,12 @@ private class ExpressionChecker(
298307
// Flag if interval relation has no resolved type (e.g., Includes on non-list/interval)
299308
if (model[expr] == null) {
300309
model.addError(expr)
310+
return
311+
}
312+
// Propagate errors from children: if an operand has an error (e.g., unresolved
313+
// identifier B in "B.relevantPeriod during ..."), the interval relation is also invalid.
314+
if (hasNestedError(expr.left) || hasNestedError(expr.right)) {
315+
model.addError(expr)
301316
}
302317
}
303318

Src/java/cql-to-elm/src/commonMain/kotlin/org/cqframework/cql/cql2elm/analysis/TemporalTypeInference.kt

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,23 +119,67 @@ internal fun TypeResolver.inferExpandCollapseType(
119119
return operandType
120120
}
121121

122+
@Suppress("ReturnCount", "CyclomaticComplexMethod")
122123
internal fun TypeResolver.inferIntervalRelationType(
123124
expression: IntervalRelationExpression
124125
): DataType? {
125126
// left, right pre-folded by catamorphism
126-
// Basic validation: includes/includedIn require at least one list/interval operand
127+
val leftType = typeTable[expression.left]
128+
val rightType = typeTable[expression.right]
129+
130+
// Resolve through the OperatorMap ONLY when both operands are intervals with different
131+
// point types (interval type promotion, e.g., Interval<Date> → Interval<DateTime>).
132+
// For other cases (scalar/null operands, same-type intervals), the existing lowering +
133+
// ConversionAnalyzer path handles everything correctly.
134+
val opName = intervalPhraseToOperatorName(expression.phrase)
135+
if (
136+
opName != null &&
137+
leftType is org.hl7.cql.model.IntervalType &&
138+
rightType is org.hl7.cql.model.IntervalType &&
139+
leftType.pointType != rightType.pointType
140+
) {
141+
val effectiveLeft =
142+
syntheticTable?.effectiveType(expression, Slot.Left, leftType, operatorRegistry)
143+
?: leftType
144+
val effectiveRight =
145+
syntheticTable?.effectiveType(expression, Slot.Right, rightType, operatorRegistry)
146+
?: rightType
147+
val resolution = operatorRegistry.resolve(opName, listOf(effectiveLeft, effectiveRight))
148+
if (resolution != null) {
149+
typeTable.setOperatorResolution(expression, resolution)
150+
return resolution.operator.resultType
151+
}
152+
}
153+
154+
// Fallback: basic validation for includes/includedIn (at least one collection operand required)
127155
val phrase = expression.phrase
128156
if (
129157
phrase is org.hl7.cql.ast.IncludesIntervalPhrase ||
130158
phrase is org.hl7.cql.ast.IncludedInIntervalPhrase
131159
) {
132-
val leftType = typeTable[expression.left]
133-
val rightType = typeTable[expression.right]
134160
val leftIsCollection =
135161
leftType is org.hl7.cql.model.ListType || leftType is org.hl7.cql.model.IntervalType
136162
val rightIsCollection =
137163
rightType is org.hl7.cql.model.ListType || rightType is org.hl7.cql.model.IntervalType
138164
if (!leftIsCollection && !rightIsCollection) return null
139165
}
166+
140167
return type("Boolean")
141168
}
169+
170+
/**
171+
* Map an interval phrase to the system operator name used for resolution. Returns null if the
172+
* phrase type doesn't have a direct operator mapping (e.g., complex phrases that are lowered into
173+
* operator trees before emission).
174+
*/
175+
private fun intervalPhraseToOperatorName(phrase: org.hl7.cql.ast.IntervalOperatorPhrase): String? =
176+
when (phrase) {
177+
is org.hl7.cql.ast.IncludedInIntervalPhrase ->
178+
if (phrase.proper) "ProperIncludedIn" else "IncludedIn"
179+
is org.hl7.cql.ast.IncludesIntervalPhrase ->
180+
if (phrase.proper) "ProperIncludes" else "Includes"
181+
is org.hl7.cql.ast.ConcurrentIntervalPhrase -> null // lowered to boundary comparisons
182+
is org.hl7.cql.ast.BeforeOrAfterIntervalPhrase -> null // lowered by ExpressionLowering
183+
is org.hl7.cql.ast.WithinIntervalPhrase -> null // lowered by ExpressionLowering
184+
else -> null
185+
}

0 commit comments

Comments
 (0)