FHIR / sushi

SUSHI (aka "SUSHI Unshortens Short Hand Inputs") is a reference implementation command-line interpreter/compiler for FHIR Shorthand (FSH).
Apache License 2.0
145 stars 44 forks source link

Don't return instance rules when looking for id #1355

Closed mint-thompson closed 1 year ago

mint-thompson commented 1 year ago

Completes task CIMPL-1169.

When searching for a rule that sets an id, check the rule's isInstance flag. Only return the rule if the flag is not set.

To explain how this was going infinite: an Instance had a rule that looked like this:

* id = procedure-death-certification-ut-example1

Because the value is unquoted, the rule's isInstance flag is set. So, when applying this rule during export, the exporter attempts to fish up an Instance with a matching name or id. When checking the same instance, findIdAssignmentRule would find this rule, meaning that it now wants to assign this Instance. But, the Instance must be exported before it can be assigned. So we start exporting the Instance, attempt to apply this rule, and thus we are stuck until the call stack fills up and the computer drops it all on the ground.

This FSH still results in an error, but a much more helpful one: the fisher reports that it can't find an Instance called procedure-death-certification-ut-example1 to assign for this rule.

mint-thompson commented 1 year ago

@cmoesel I'm glad you asked about url, since that got me more curious about other related scenarios. As it turns out, any situation where an Instance refers to itself leads to a stack overflow. For example, both of these Instances cause stack overflows:

Instance: MyQuestionnaire
InstanceOf: Questionnaire
* url = MyQuestionnaire
* status = #draft

Instance: MyObs
InstanceOf: Observation
Usage: #example
* status = #final
* code = #123
* valueString = MyObs

So, it looks like we may need to do a better job handling this in the more general case of AssignmentRules on Instances.

But to answer your original question: when checking for a url, we use getValueFromFshRules, which doesn't check if the rule has isInstance set, but also just returns the value as a string when a rule with the correct path is found. This implementation is safe as long as isInstance is not set, but either way, it does not cause a stack overflow. Also, it's not until processing the actual CaretValueRule that we get into trouble. For example, using this FSH:

Profile: ObsProfile
Parent: Observation
* ^url = http://hl7.org/some/example/StructureDefinition/ObsProfile

The call to getUrlFromFshDefinition at StructureDefinitionExporter.ts#L309 returns the string http://hl7.org/some/example/StructureDefinition/ObsProfile without any trouble. But when processing the CaretValueRule, we get down to StructureDefinitionExporter.ts#L1000, with this line:

sd.setInstancePropertyByPath(rule.caretPath, fishedValue, this);

We get an error, since fishedValue is an InstanceDefinition that represents ObsProfile, which is not the right type here:

Cannot assign StructureDefinition value: ObsProfile. Value does not match element type: uri

This error is much better than filling up the stack, of course.