search

FirelyTeam / firely-cql-sdk

BSD 3-Clause "New" or "Revised" License
28 stars 16 forks source link

397 fix forward references in expressionbuilder 3 #468

Closed baseTwo closed 1 week ago

baseTwo commented 3 weeks ago

Fixes for #397 and rework of previous PR #437 with additional bugfixes

Copying description from PR #437 by @ewoutkramer

This PR enables the translation of FunctionRef/ExpressionRef to rely on the ref's resultTypeSpecifier instead of having to look the definition up in the DefinitionDictionary. Not using the DefinitionDictionary avoids running into the situation where the ref is a forwards reference whereby the DefinitionDictionary does not yet contain the translated function we want to call.

This PR uses the recent functionality added to LibrarySet to lookup a symbol, and expands this to be able to lookup functions by signature. Given the function name in the FUnctionRef/ExpressionRef, and, in the case of an overload, the signature, we can now lookup any function defined in any library in the LibrarySet, so we can copy over the resultTypeSpecifier.

Filling out this resultTypeSpecifier is now done in a pre-processing step, happening when we are constructing the LibrarySet. The pre-processing step will find all Refs, and then do this:

  • Try to find the function by name. If it is not an overload, use the resultTypeSpecifier
  • If there is an overload, use the Ref's signature, if available.
  • If there is no signature, use the types from the Ref's parameters and match on those.

Otherwise, we cannot find the overload, and we will raise an error. This ensures that after the pre-processor has run, all Refs > have a resultTypeSpecifier, and we can depend on that fact.

This PR re-uses the OverloadedFunctionDefinition, and there are some updates to the CanCombine function to make sure the pre-processor fix and the CanCombine use the same criteria.

I also added a bunch of useful extension methods for working with the Elm function elements and signatures.

EvanMachusak commented 3 weeks ago

Merging this PR into 425 the results are the same - FHIRHelpers cannot be compiled because its expressions are out of order. Can you point me at the code that claims to fix this? All I see is my commented fix in ExpressionBuilder. Left commented, it's still broken. Uncommented and the eCQMs still don't work.

baseTwo commented 1 week ago

This PR is superceded by #487 and can be closed. The commits by @baseTwo will be included again as part of #503