Repository: FHIR/sushi (ts)

Issue:
SUSHI handles value[x] assignments incorrectly in Instance
If a profile constrains `value[x]` to a single type, and an author assigns `value[x]` in an instance of that profile, then SUSHI incorrectly serialized the property as `value[x]` rather than using its type-specific name.  For example:
```
Profile: BooleanObservation
Parent: Observation
* value[x] only boolean

Instance: MyObservation
InstanceOf: BooleanObservation
* status = #final
* code = http://example#somecode
* value[x] = true
```
produces the following instance:
```json
{
  "resourceType": "Observation",
  "id": "MyObservation",
  "meta": {
    "profile": [
      "http://example.org/StructureDefinition/BooleanObservation"
    ]
  },
  "status": "final",
  "code": {
    "coding": [
      {
        "code": "somecode",
        "system": "http://example"
      }
    ]
  },
  "value[x]": true
}
```
Note that the last property should be `"valueBoolean": true`.

Either SUSHI needs to disallow assignment to `value[x]` in Instances or it needs to handle it correctly. I'd prefer to handle it correctly, but if that proves to be complex, it may be reasonable to disallow it.

Pull request:
Handle choice paths on Instance assignment rules
Fixes #1032 and completes task [CIMPL-910](https://standardhealthrecord.atlassian.net/browse/CIMPL-910).

When assigning values to choice elements on an Instance, type-specific elements should be used in the rules. If the choice element is constrained to a single type, it is possible to change the path to refer to the type-specific element. Warn the user that the path is being changed and that it is preferable to use the type-specific element name in the rule. If the choice element is constrained to multiple types, it is not possible to change the path. Show an error indicating that the value will not be assigned. These warnings and errors may apply regardless of where the choice element appears in the rule path.

Basically: you can't have an `[x]` element on an Instance, so show a warning and fix it in cases where we know the type, and show an error in cases where we don't know the type.

See also  http://hl7.org/fhir/R4/formats.html#choice

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 91d9e9a..f80bb82 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -104,6 +104,17 @@ export class InstanceExporter implements Fishable {
         matchingInlineResourcePaths.forEach(match => {
           inlineResourceTypes[splitOnPathPeriods(match.path).length - 1] = match.instanceOf;
         });
+        // if a rule has choice elements in the path that are constrained to a single type,
+        // change the path to reflect the available type for each choice element.
+        // if the path changes, warn the user that it is a better idea to use the more specific element name
+        const updatedPath = instanceOfStructureDefinition.updatePathWithChoices(rule.path);
+        if (rule.path !== updatedPath) {
+          logger.warn(
+            `When assigning values on instances, use the choice element's type. Rule path changed from '${rule.path}' to '${updatedPath}'.`,
+            rule.sourceInfo
+          );
+          rule.path = updatedPath;
+        }
         const validatedRule = instanceOfStructureDefinition.validateValueAtPath(
           rule.path,
           rule.value,
@@ -111,11 +122,20 @@ export class InstanceExporter implements Fishable {
           inlineResourceTypes
         );
         // Record each valid rule in a map
-        ruleMap.set(rule.path, {
-          pathParts: validatedRule.pathParts,
-          assignedValue: validatedRule.assignedValue,
-          sourceInfo: rule.sourceInfo
-        });
+        // Choice elements on an instance must use a specific type, so if the path still has an unchosen choice element,
+        // the rule can't be used. See http://hl7.org/fhir/R4/formats.html#choice
+        if (validatedRule.pathParts.some(part => part.base.endsWith('[x]'))) {
+          logger.error(
+            `Unable to assign value at ${rule.path}: choice elements on an instance must use a specific type`,
+            rule.sourceInfo
+          );
+        } else {
+          ruleMap.set(rule.path, {
+            pathParts: validatedRule.pathParts,
+            assignedValue: validatedRule.assignedValue,
+            sourceInfo: rule.sourceInfo
+          });
+        }
       } catch (e) {
         logger.error(e.message, rule.sourceInfo);
       }
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 35966bc..c47818e 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -316,6 +316,31 @@ export class StructureDefinition {
     return matchingElements.length === 1 ? matchingElements[0] : undefined;
   }
 
+  /**
+   * Change a path's choice elements that are given as [x], but only have one type.
+   * If the choice element has more than one type, do not change it.
+   * For example, value[x] constrained to only "string" would become valueString.
+   * These paths are useful when assigning values on an instance.
+   * @param {string} path - The FSH path to operate on
+   * @returns {string} - a FSH path with type-specific choices
+   */
+  updatePathWithChoices(path: string): string {
+    // if there are no [x] elements, there's nothing to update
+    if (!/\[x\]/.test(path)) {
+      return path;
+    }
+    const parsedPath = parseFSHPath(path);
+    let buildPath = this.pathType;
+    for (const pathPart of parsedPath) {
+      buildPath += `.${pathPart.base}`;
+      const matchingElement = this.findElement(buildPath);
+      if (pathPart.base.endsWith('[x]') && matchingElement?.type.length === 1) {
+        pathPart.base = pathPart.base.replace('[x]', upperFirst(matchingElement.type[0].code));
+      }
+    }
+    return assembleFSHPath(parsedPath);
+  }
+
   /**
    * This function sets an instance property of an SD if possible
    * @param {string} path - The path to the ElementDefinition to assign


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 8a1962d..f980ab1 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1377,6 +1377,75 @@ describe('InstanceExporter', () => {
       ]);
     });
 
+    it('should assign value[x] to the correct path when the rule on the instance refers to value[x], and value[x] is constrained to one type', () => {
+      // Profile: ObservationProfile
+      // Parent: Observation
+      // * value[x] 1..1
+      // * value[x] only string
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      const valueCard = new CardRule('value[x]');
+      valueCard.min = 1;
+      valueCard.max = '1';
+      const valueOnly = new OnlyRule('value[x]');
+      valueOnly.types = [{ type: 'string' }];
+      observationProfile.rules.push(valueCard, valueOnly);
+      doc.profiles.set(observationProfile.name, observationProfile);
+      // Instance: MyObservation
+      // InstanceOf: ObservationProfile
+      // * status = #final
+      // * code = #testcode
+      // * value[x] = "some value"
+      const testInstance = new Instance('MyObservation');
+      testInstance.instanceOf = 'ObservationProfile';
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      const valueSomeValue = new AssignmentRule('value[x]');
+      valueSomeValue.value = 'some value';
+      testInstance.rules.push(statusFinal, codeTestCode, valueSomeValue);
+      const exported = exportInstance(testInstance);
+      expect(exported.valueString).toBe('some value');
+      expect(exported['value[x]']).toBeUndefined();
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getLastMessage('warn')).toMatch(
+        "When assigning values on instances, use the choice element's type. Rule path changed from 'value[x]' to 'valueString'."
+      );
+    });
+
+    it('should log an error and not assign to a descendant of a choice element when that choice element has more than one type', () => {
+      // Profile: ObservationProfile
+      // Parent: Observation
+      // * value[x] only string or boolean
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      const valueOnly = new OnlyRule('value[x]');
+      valueOnly.types = [{ type: 'string' }, { type: 'boolean' }];
+      observationProfile.rules.push(valueOnly);
+      doc.profiles.set(observationProfile.name, observationProfile);
+      // Instance: MyObservation
+      // InstanceOf: ObservationProfile
+      // * status = #final
+      // * code = #testcode
+      // * value[x].extension.url = "http://example.org/SomeExtension"
+      const testInstance = new Instance('MyObservation');
+      testInstance.instanceOf = 'ObservationProfile';
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      const extensionUrl = new AssignmentRule('value[x].extension.url');
+      extensionUrl.value = 'http://example.org/SomeExtension';
+      testInstance.rules.push(statusFinal, codeTestCode, extensionUrl);
+      const exported = exportInstance(testInstance);
+      expect(exported['value[x]']).toBeUndefined();
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+      expect(loggerSpy.getLastMessage('error')).toMatch(
+        'Unable to assign value at value[x].extension.url: choice elements on an instance must use a specific type'
+      );
+    });
+
     it('should assign an element to a value the same as the assigned value on the Structure Definition', () => {
       const assignedValRule = new AssignmentRule('active');
       assignedValRule.value = true;