Repository: FHIR/sushi (ts)

Issue:
Instance serialized with `value[x]`
From Zulip: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/value.5Bx.5D.20confusion.20on.20Instances

In some cases (well, at least _one_ case!), SUSHI exports an instance that uses property `value[x]` instead of a specific type-based property (e.g., `valueMoney`).  E.g., the following sub-extension instance:

```json
{
  "url": "Kostenbetrag",
  "value[x]": {
    "currency": "EUR"
  },
  "valueMoney": {
    "currency": "EUR",
    "value": 5
  }
}
```

We couldn't reproduce this using only FSH, but we were able to reproduce it by creating a FSH project that directly used an extension and profile that were supplied as JSON.  A first step for debugging this would likely be to see how/if those JSON profiles differ from what SUSHI would produce.

Example project attached...
[ValueXBug.zip](https://github.com/FHIR/sushi/files/5982631/ValueXBug.zip)


Pull request description:
Fix sushi 760
This actually fixes two bugs:
* Instances are serialized with choice properties (e.g., `value[x]`) in some cases
* Arrays are serialized as objects (not arrays) in some cases

There are a few possible ways to reproduce the first one:

1. Checkout commit 131b714 and observe the failing test
2. Put the following FSH in a FSH file
```
Profile: EURMoney
Parent: Money
* currency 1..1
* currency = #EUR (exactly)

Extension: InsuranceCost
* extension contains Amount 1..1
* extension[Amount].value[x] 1..1
* extension[Amount].value[x] only EURMoney
* extension[Amount].value[x].value ^short = "Cost"

Profile: ObservationProfile
Parent: Observation
* extension contains InsuranceCost named InsCost 1..1

Instance: TestInstance
InstanceOf: ObservationProfile
* status = #final
* code = #testcode
* extension[InsCost].extension[Amount].valueMoney.value = 5.00
```
3. Run regression on `aehrc/primary-care-data-technical#master` and note the changes in `Observation-aupc-gravidity1.json` and `Observation-aupc-parity1.json`.  For example:
<img width="1269" alt="image" src="https://user-images.githubusercontent.com/2278253/110376172-e25d3480-8020-11eb-85fe-9b9ed9c6e92e.png">

The second bug can also be reproduced via the failing test in 131b714.  Note that `extension` is an object, _not_ an array.  It is not reproduced, however, via the other methods above.

Fixes #760




Patch:
diff --git a/regression/repos-all.txt b/regression/repos-all.txt
index 9a498c7..213b43f 100644
--- a/regression/repos-all.txt
+++ b/regression/repos-all.txt
@@ -66,3 +66,10 @@ dvci/vaccine-yellow-card-ig#main
 # Added Mon Feb 08 2021 11:01:34 GMT-0500 (Eastern Standard Time)
 JohnMoehrke/BasicAudit#main
 medicalhome-aprn/smart-web-messaging#master
+# Added Mon Mar 08 2021 13:27:24 GMT-0500 (Eastern Standard Time)
+HL7/fhir-forfair#master
+standardhealth/fsh-ae#master
+hl7ch/ch-etoc#main
+hl7ch/ch-rad-order#master
+ehealthsuisse/ch-rad-order#master
+HL7NZ/nzf#main
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 48c0479..c432b79 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -519,11 +519,6 @@ export class StructureDefinition {
         }
       }
 
-      // If the element has a base.max that is greater than 1, but the element has been constrained, still set properties in an array
-      const nonArrayElementIsBasedOnArray =
-        currentElement?.base?.max !== '0' &&
-        currentElement?.base?.max !== '1' &&
-        (currentElement?.max === '0' || currentElement?.max === '1');
       if (
         !currentElement ||
         currentElement.max === '0' ||
@@ -534,17 +529,24 @@ export class StructureDefinition {
         // We throw an error if the currentElement doesn't exist, has been zeroed out,
         // or is being incorrectly accessed as an array
         throw new CannotResolvePathError(path);
-      } else if (
-        (arrayIndex == null &&
-          currentElement.max != null &&
-          currentElement.max !== '0' &&
-          currentElement.max !== '1') ||
-        nonArrayElementIsBasedOnArray
-      ) {
-        // Modify the path to have 0 indices
+      }
+
+      // Determine if base and/or current are arrays. Note that this is not perfect (if base or current max is missing),
+      // but in practice, it appears to be sufficient. We could walk the inheritance tree to get the base and current values
+      // when they are missing, but this comes at a cost, and as noted above, the current approach works (likely due to
+      // how the publisher populates base in snapshots and how previous code processes the current element).
+      const baseIsArray =
+        currentElement?.base?.max != null &&
+        currentElement.base.max !== '0' &&
+        currentElement.base.max !== '1';
+      const currentIsArray =
+        currentElement?.max != null && currentElement.max !== '0' && currentElement.max !== '1';
+      // If the base is an array and we don't yet have an index or the currentElement is singular, make this index 0.
+      if (baseIsArray && (arrayIndex == null || !currentIsArray)) {
         if (!pathPart.brackets) pathPart.brackets = [];
         pathPart.brackets.push('0');
       }
+
       // Primitive and only primitives have a lower case first letter
       if (
         currentElement.type?.length === 1 &&
diff --git a/src/fhirtypes/common.ts b/src/fhirtypes/common.ts
index 7c9c4e6..2f02147 100644
--- a/src/fhirtypes/common.ts
+++ b/src/fhirtypes/common.ts
@@ -1,4 +1,4 @@
-import { isEmpty, cloneDeep } from 'lodash';
+import { isEmpty, cloneDeep, upperFirst } from 'lodash';
 import {
   StructureDefinition,
   PathPart,
@@ -71,7 +71,7 @@ export function setImpliedPropertiesOnInstance(
     const element = instanceOfStructureDefinition.findElementByPath(nonNumericPath, fisher);
     if (element) {
       // If an element is pointed to by a path, that means we must assign values on its parents, its own
-      // assignable descendents, and assignable descenendents of its parents. An assignable descendent is a 1..n direct child
+      // assignable descendents, and assignable descendents of its parents. An assignable descendent is a 1..n direct child
       // or an assignable descendent of such a child
       const parents = element.getAllParents();
       const associatedElements = [...element.getAssignableDescendents(), ...parents];
@@ -89,11 +89,16 @@ export function setImpliedPropertiesOnInstance(
           let overlapIdx = 0;
           const elParts = element.id.split('.');
           const assocElParts = associatedEl.id.split('.');
-          for (
-            ;
-            overlapIdx < elParts.length && elParts[overlapIdx] === assocElParts[overlapIdx];
-            overlapIdx++
-          );
+          for (; overlapIdx < elParts.length && overlapIdx < assocElParts.length; overlapIdx++) {
+            // If an associated path applies to all choices (e.g., value[x]), and the element path
+            // is a specific choice (e.g., value[x]:valueQuantity), then treat it as a match
+            const [elPart, assocElPart] = [elParts[overlapIdx], assocElParts[overlapIdx]];
+            if (assocElPart.endsWith('[x]') && elPart.startsWith(`${assocElPart}:`)) {
+              continue;
+            } else if (elPart !== assocElPart) {
+              break;
+            }
+          }
           // We must keep the relevant portion of the beginning of path to preserve sliceNames
           // and combine this with portion of the associatedEl's path that is not overlapped
           const pathStart = splitOnPathPeriods(path).slice(0, overlapIdx - 1);
@@ -109,7 +114,31 @@ export function setImpliedPropertiesOnInstance(
             // Implied paths are found via the index free path
             finalPath.replace(/\[[-+]?\d+\]$/g, '')
           );
-          [finalPath, ...impliedPaths].forEach(ip => sdRuleMap.set(ip, foundAssignedValue));
+          [finalPath, ...impliedPaths].forEach(ip => {
+            // Don't let any non-constrained choice paths (e.g., value[x]) through since instances
+            // must specify a particular choice (i.e., value[x] is not a valid path in an instance)
+            if (/\[x]/.test(ip)) {
+              // Fix any single-type choices to be type-specific (e.g., value[x] -> valueString)
+              const parts = splitOnPathPeriods(ip);
+              for (let i = 0; i < parts.length; i++) {
+                if (parts[i].endsWith('[x]')) {
+                  const partEl = instanceOfStructureDefinition.findElementByPath(
+                    parts.slice(0, i + 1).join('.'),
+                    fisher
+                  );
+                  if (partEl?.type?.length === 1) {
+                    parts[i] = parts[i].replace('[x]', upperFirst(partEl.type[0].code));
+                  }
+                }
+              }
+              ip = parts.join('.');
+              // If there is still a [x], we couldn't fix it, so skip it
+              if (/\[x]/.test(ip)) {
+                return; // out of the forEach of implied paths
+              }
+            }
+            sdRuleMap.set(ip, foundAssignedValue);
+          });
         }
       }
     }


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 2268b7d..8d5dc1d 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -583,6 +583,264 @@ describe('InstanceExporter', () => {
       expect(exported.deceasedBoolean).toBe(true);
     });
 
+    it('should not assign fixed values from value[x] children when a specific choice has not been chosen', () => {
+      // Profile: ObservationProfile
+      // Parent: Observation
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      // * value[x] 1..1
+      const valueCardRequired = new CardRule('value[x]');
+      valueCardRequired.min = 1;
+      valueCardRequired.max = '1';
+      observationProfile.rules.push(valueCardRequired);
+      // * value[x].id 1..1
+      const valueIdCardRequired = new CardRule('value[x].id');
+      valueIdCardRequired.min = 1;
+      valueIdCardRequired.max = '1';
+      observationProfile.rules.push(valueIdCardRequired);
+      // * value[x].id = "some-required-id"
+      const valueIdAssignment = new AssignmentRule('value[x].id');
+      valueIdAssignment.value = 'Hello World';
+      observationProfile.rules.push(valueIdAssignment);
+      doc.profiles.set(observationProfile.name, observationProfile);
+
+      // Instance: TestInstance
+      // InstanceOf: ObservationProfile
+      const testInstance = new Instance('TestInstance');
+      testInstance.instanceOf = 'ObservationProfile';
+      // * status = #final
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      testInstance.rules.push(statusFinal);
+      // * code = #testcode
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      testInstance.rules.push(codeTestCode);
+      doc.instances.set(testInstance.name, testInstance);
+
+      const exported = exportInstance(testInstance);
+      expect(exported.toJSON()).toEqual({
+        resourceType: 'Observation',
+        id: 'TestInstance',
+        meta: {
+          profile: ['http://hl7.org/fhir/us/minimal/StructureDefinition/ObservationProfile']
+        },
+        status: 'final',
+        code: { coding: [{ code: 'testcode' }] }
+      });
+    });
+
+    it('should assign fixed values from value[x] children using the correct specific choice property name', () => {
+      // Profile: ObservationProfile
+      // Parent: Observation
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      // * value[x] 1..1
+      const valueCardRequired = new CardRule('value[x]');
+      valueCardRequired.min = 1;
+      valueCardRequired.max = '1';
+      observationProfile.rules.push(valueCardRequired);
+      // * value[x].id 1..1
+      const valueIdCardRequired = new CardRule('value[x].id');
+      valueIdCardRequired.min = 1;
+      valueIdCardRequired.max = '1';
+      observationProfile.rules.push(valueIdCardRequired);
+      // * value[x].id = "some-required-id"
+      const valueIdAssignment = new AssignmentRule('value[x].id');
+      valueIdAssignment.value = 'some-id';
+      observationProfile.rules.push(valueIdAssignment);
+      doc.profiles.set(observationProfile.name, observationProfile);
+
+      // Instance: TestInstance
+      // InstanceOf: ObservationProfile
+      const testInstance = new Instance('TestInstance');
+      testInstance.instanceOf = 'ObservationProfile';
+      // * status = #final
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      testInstance.rules.push(statusFinal);
+      // * code = #testcode
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      testInstance.rules.push(codeTestCode);
+      // * valueQuantity = 5 'mm'
+      const valueQuantityAssignment = new AssignmentRule('valueQuantity');
+      valueQuantityAssignment.value = new FshQuantity(
+        5,
+        new FshCode('mm', 'http://unitsofmeasure.org')
+      );
+      testInstance.rules.push(valueQuantityAssignment);
+      doc.instances.set(testInstance.name, testInstance);
+
+      const exported = exportInstance(testInstance);
+      expect(exported.toJSON()).toEqual({
+        resourceType: 'Observation',
+        id: 'TestInstance',
+        meta: {
+          profile: ['http://hl7.org/fhir/us/minimal/StructureDefinition/ObservationProfile']
+        },
+        status: 'final',
+        code: { coding: [{ code: 'testcode' }] },
+        valueQuantity: {
+          id: 'some-id',
+          value: 5,
+          system: 'http://unitsofmeasure.org',
+          code: 'mm'
+        }
+      });
+    });
+
+    it('should assign fixed values from value[x] children using the correct specific choice property name (primitive edition)', () => {
+      // Profile: ObservationProfile
+      // Parent: Observation
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      // * value[x] 1..1
+      const valueCardRequired = new CardRule('value[x]');
+      valueCardRequired.min = 1;
+      valueCardRequired.max = '1';
+      observationProfile.rules.push(valueCardRequired);
+      // * value[x].id 1..1
+      const valueIdCardRequired = new CardRule('value[x].id');
+      valueIdCardRequired.min = 1;
+      valueIdCardRequired.max = '1';
+      observationProfile.rules.push(valueIdCardRequired);
+      // * value[x].id = "some-required-id"
+      const valueIdAssignment = new AssignmentRule('value[x].id');
+      valueIdAssignment.value = 'some-id';
+      observationProfile.rules.push(valueIdAssignment);
+      doc.profiles.set(observationProfile.name, observationProfile);
+
+      // Instance: TestInstance
+      // InstanceOf: ObservationProfile
+      const testInstance = new Instance('TestInstance');
+      testInstance.instanceOf = 'ObservationProfile';
+      // * status = #final
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      testInstance.rules.push(statusFinal);
+      // * code = #testcode
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      testInstance.rules.push(codeTestCode);
+      // * valueString = 'Hello World'
+      const valueStringAssignment = new AssignmentRule('valueString');
+      valueStringAssignment.value = 'Hello World';
+      testInstance.rules.push(valueStringAssignment);
+      doc.instances.set(testInstance.name, testInstance);
+
+      const exported = exportInstance(testInstance);
+      expect(exported.toJSON()).toEqual({
+        resourceType: 'Observation',
+        id: 'TestInstance',
+        meta: {
+          profile: ['http://hl7.org/fhir/us/minimal/StructureDefinition/ObservationProfile']
+        },
+        status: 'final',
+        code: { coding: [{ code: 'testcode' }] },
+        valueString: 'Hello World',
+        _valueString: { id: 'some-id' }
+      });
+    });
+
+    it('should assign fixed value[x] correctly even in weird situations (SUSHI #760)', () => {
+      // See https://github.com/FHIR/sushi/issues/760
+
+      // Profile: EURMoney
+      // Parent: Money
+      const eurMoney = new Profile('EURMoney');
+      eurMoney.parent = 'Money';
+      // * currency 1..1
+      const currencyRequired = new CardRule('currency');
+      currencyRequired.min = 1;
+      eurMoney.rules.push(currencyRequired);
+      // * currency = #EUR (exactly)
+      const currencyFixed = new AssignmentRule('currency');
+      currencyFixed.value = new FshCode('EUR');
+      currencyFixed.exactly = true;
+      eurMoney.rules.push(currencyFixed);
+      doc.profiles.set(eurMoney.name, eurMoney);
+
+      // Extension: InsuranceCost
+      const insCost = new Extension('InsuranceCost');
+      // * extension contains Amount 1..1
+      const containsAmount = new ContainsRule('extension');
+      containsAmount.items = [{ name: 'Amount' }];
+      insCost.rules.push(containsAmount);
+      const amountRequired = new CardRule('extension[Amount]');
+      amountRequired.min = 1;
+      insCost.rules.push(amountRequired);
+      const noValue = new CardRule('value[x]');
+      noValue.max = '0';
+      insCost.rules.push(noValue);
+      // * extension[Amount].value[x] 1..1
+      const amountValueRequired = new CardRule('extension[Amount].value[x]');
+      amountValueRequired.min = 1;
+      insCost.rules.push(amountValueRequired);
+      const noExtension = new CardRule('extension[Amount].extension');
+      noExtension.max = '0';
+      insCost.rules.push(noExtension);
+      // * extension[Amount].value[x] only EURMoney
+      const amountOnlyEurMoney = new OnlyRule('extension[Amount].value[x]');
+      amountOnlyEurMoney.types = [{ type: 'EURMoney' }];
+      insCost.rules.push(amountOnlyEurMoney);
+      // * extension[Amount].value[x].value ^short = "Cost"
+      const amountMoneyValueShort = new CaretValueRule('extension[Amount].value[x].value');
+      amountMoneyValueShort.caretPath = 'short';
+      amountMoneyValueShort.value = 'Cost';
+      insCost.rules.push(amountMoneyValueShort);
+      doc.extensions.set(insCost.name, insCost);
+
+      // Profile: ObservationProfile
+      // Parent: Observation
+      const observationProfile = new Profile('ObservationProfile');
+      observationProfile.parent = 'Observation';
+      // * extension contains InsuranceCost named InsCost 1..1
+      const containsInsCost = new ContainsRule('extension');
+      containsInsCost.items = [{ name: 'InsCost', type: 'InsuranceCost' }];
+      observationProfile.rules.push(containsInsCost);
+      const insCostRequired = new CardRule('extension[InsCost]');
+      insCostRequired.min = 1;
+      observationProfile.rules.push(insCostRequired);
+      doc.profiles.set(observationProfile.name, observationProfile);
+
+      // Instance: TestInstance
+      // InstanceOf: ObservationProfile
+      const testInstance = new Instance('TestInstance');
+      testInstance.instanceOf = 'ObservationProfile';
+      // * status = #final
+      const statusFinal = new AssignmentRule('status');
+      statusFinal.value = new FshCode('final');
+      testInstance.rules.push(statusFinal);
+      // * code = #testcode
+      const codeTestCode = new AssignmentRule('code');
+      codeTestCode.value = new FshCode('testcode');
+      testInstance.rules.push(codeTestCode);
+      // * extension[InsCost].extension[Amount].valueMoney.value = 5.00
+      const moneyValueFive = new AssignmentRule(
+        'extension[InsCost].extension[Amount].valueMoney.value'
+      );
+      moneyValueFive.value = 5;
+      testInstance.rules.push(moneyValueFive);
+      doc.instances.set(testInstance.name, testInstance);
+
+      const exported = exportInstance(testInstance);
+      expect(exported.extension).toEqual([
+        {
+          url: 'http://hl7.org/fhir/us/minimal/StructureDefinition/InsuranceCost',
+          extension: [
+            {
+              url: 'Amount',
+              valueMoney: {
+                currency: 'EUR',
+                value: 5
+              }
+            }
+          ]
+        }
+      ]);
+    });
+
     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;