Repository: FHIR/sushi
Pull request #243 (2020-02-19 18:13:46)

Error message line number
Fixes https://github.com/FHIR/sushi/issues/194.

The error being emitted didn't have a line number because the error itself was incorrect. SUSHI was incorrectly thinking that a code value from a Structure Definition couldn't be fixed to a code on an instance, because the code on the Structure Definition gets serialized as a string. So to SUSHI, it seemed like we were trying to fix a string to a code. 

This is fixed by not validating values that are set on an Instance from a Structure Definition. We can do this, because the values have already been set on a Structure Definition, so we know they are valid to set on an Instance.

ngfreiter: In my investigation of if this was a good approach, I realized that in `setFixedValuesFromStructureDefinition`, the `fixedByDirectParent` function was not actually doing anything. Since `setFixedValuesFromStructureDefinition` always gets called on the parent of an element before getting called on the element itself, there is no need to check for `pattern` on a parent, since that `pattern` would have already been accounted for when processing the parent. I'm now comfortable with this approach, but before merging I still want @jafeltra to confirm she is comfortable too.
jafeltra: I took a look at this and had a question for Nick about using `fixedValue` vs `foundFixedValue`. I talked to him in person a bit and it sounded like he had additional thoughts while I was in a meeting that I'll circle back on early tomorrow morning. Pending that, I'm good to merge this.

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 3fa867c..e6fa3bb 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -5,7 +5,7 @@ import { logger, Fishable, Type } from '../utils';
 import { setPropertyOnInstance, replaceReferences, replaceField } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { Package } from '.';
-import isEmpty from 'lodash/isEmpty';
+import { isEmpty, cloneDeep } from 'lodash';
 
 export class InstanceExporter {
   constructor(
@@ -85,6 +85,7 @@ export class InstanceExporter {
   /**
    * Given an ElementDefinition, set fixed values for the direct children of that element
    * according to the ElementDefinitions of the children
+   * This function assumes that when it is called on an element, it has alreday been called on parents of that element
    * @param {ElementDefinition} element - The element whose children we will fix
    * @param {string} existingPath - The path to the element whose children we will fix
    * @param {InstanceDefinition} instanceDef - The InstanceDefinition to fix values on
@@ -104,9 +105,7 @@ export class InstanceExporter {
         k => k.startsWith('fixed') || k.startsWith('pattern')
       );
       // Fixed value can come from fixed[x] or pattern[x] directly on element, or via pattern[x] on parent
-      const foundFixedValue =
-        fixableElement[fixedValueKey as keyof ElementDefinition] ??
-        fixableElement.fixedByDirectParent();
+      const foundFixedValue = cloneDeep(fixableElement[fixedValueKey as keyof ElementDefinition]);
       // We only fix the value if the element is the original element, or it is a direct child with card 1..n
       if (foundFixedValue && (fixableElement.id === element.id || fixableElement.min > 0)) {
         // Get the end of the path, this is the part that differs from existingPath
@@ -124,14 +123,16 @@ export class InstanceExporter {
         });
         // Fix the value if we validly can
         try {
-          const { fixedValue, pathParts } = instanceOfStructureDefinition.validateValueAtPath(
+          const { pathParts } = instanceOfStructureDefinition.validateValueAtPath(
             // fshElementPath is '' when the fixableElement is the original element, trailing '.' on this path must be removed
             // Otherwise add the child path to existing path
             fshElementPath === '' ? existingPath.slice(0, -1) : existingPath + fshElementPath,
-            foundFixedValue,
+            // We don't actually want to validate the value, we only want to validate the path, so pass null
+            // we already know the value is valid, since we got it from the Structure Definition
+            null,
             this.fisher
           );
-          setPropertyOnInstance(instanceDef, pathParts, fixedValue);
+          setPropertyOnInstance(instanceDef, pathParts, foundFixedValue);
         } catch (e) {
           logger.error(e.message);
         }


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 59b0414..54d146c 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -221,6 +221,17 @@ describe('InstanceExporter', () => {
       expect(exported.active).toEqual(true);
     });
 
+    it('should fix top level codes that are fixed on the Structure Definition', () => {
+      const cardRule = new CardRule('gender');
+      cardRule.min = 1;
+      patient.rules.push(cardRule);
+      const fixedValRule = new FixedValueRule('gender');
+      fixedValRule.fixedValue = new FshCode('F');
+      patient.rules.push(fixedValRule);
+      const exported = exportInstance(patientInstance);
+      expect(exported.gender).toBe('F');
+    });
+
     it('should not fix optional elements that are fixed on the Structure Definition', () => {
       const fixedValRule = new FixedValueRule('active');
       fixedValRule.fixedValue = true;