Don't slice single-type choices (unless already sliced) (#1116)

If a user refers to a specific choice type (e.g., valueQuantity), but the choice (e.g., value[x]) only allows one type anyway, then don't create a type slicing and slice, because they're not necessary.

On the other hand, if the choice has already been sliced, then go ahead and create the slice for consistency.

src/export/InstanceExporter.ts

@@ -16,7 +16,7 @@ import {
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { InstanceOfLogicalProfileError } from '../errors/InstanceOfLogicalProfileError';
 import { Package } from '.';
-import { cloneDeep, merge, padEnd, uniq } from 'lodash';
+import { cloneDeep, merge, padEnd, uniq, upperFirst } from 'lodash';
 import { AssignmentRule } from '../fshtypes/rules';
 import chalk from 'chalk';
 
@@ -319,6 +319,17 @@ export class InstanceExporter implements Fishable {
             }
           }
         }
+        // If we still haven't found it, it's possible that a type slice just wasn't created. In that case, there would
+        // be a type in the choice element's type array that would be a match if it were type-sliced.
+        if (instanceChild == null) {
+          for (const type of child.type) {
+            const name = childPathEnd.replace(/\[x\]$/, upperFirst(type.code));
+            instanceChild = instance[`_${name}`] ?? instance[name];
+            if (instanceChild != null) {
+              break;
+            }
+          }
+        }
       }
       // Recursively validate children of the current element
       if (Array.isArray(instanceChild)) {

src/fhirtypes/StructureDefinition.ts

@@ -721,17 +721,29 @@ export class StructureDefinition {
    */
   private sliceMatchingValueX(fhirPath: string, elements: ElementDefinition[]): ElementDefinition {
     let matchingType: ElementDefinitionType;
-    const matchingXElements = elements.filter(e => {
-      if (e.path.endsWith('[x]')) {
-        for (const t of e.type ?? []) {
-          if (`${e.path.slice(0, -3)}${upperFirst(t.code)}` === fhirPath) {
-            matchingType = t;
-            return true;
-          }
+    const xElements = elements.filter(e => e.path.endsWith('[x]'));
+    const matchingXElements = xElements.filter(e => {
+      for (const t of e.type ?? []) {
+        if (`${e.path.slice(0, -3)}${upperFirst(t.code)}` === fhirPath) {
+          matchingType = t;
+          return true;
         }
       }
     });
-    if (matchingXElements.length > 0) {
+    // If the only match is the choice[x] element itself, and it's already been restricted
+    // to just a single type, and there are no existing slices (for this type or otherwise),
+    // just return that instead of creating an unnecessary slice.
+    // See: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/Type.20Slices.20on.20Choices.20w.2F.20a.20Single.20Type/near/282241129
+    if (
+      matchingXElements.length === 1 &&
+      matchingXElements[0].sliceName == null &&
+      matchingXElements[0].type?.length === 1 &&
+      xElements.filter(e => e.path === matchingXElements[0].path).length === 1
+    ) {
+      return matchingXElements[0];
+    }
+    // Otherwise we want a slice representing the specific type
+    else if (matchingXElements.length > 0) {
       const sliceName = fhirPath.slice(fhirPath.lastIndexOf('.') + 1);
       const matchingSlice = matchingXElements.find(c => c.sliceName === sliceName);
       // if we have already have a matching slice, we want to return it

test/fhirtypes/StructureDefinition.test.ts

@@ -1009,12 +1009,12 @@ describe('StructureDefinition', () => {
     it('should find a child of a sliced element by path with URL', () => {
       const originalLength = clinicalDocument.elements.length;
       const valueString = clinicalDocument.findElementByPath(
-        'extension[http://hl7.org/fhir/StructureDefinition/composition-clinicaldocument-versionNumber].valueString',
+        'extension[http://hl7.org/fhir/StructureDefinition/composition-clinicaldocument-versionNumber].value[x]',
         fisher
       );
       expect(valueString).toBeDefined();
-      expect(valueString.id).toBe('Composition.extension:versionNumber.value[x]:valueString');
-      expect(clinicalDocument.elements.length).toBe(originalLength + 5);
+      expect(valueString.id).toBe('Composition.extension:versionNumber.value[x]');
+      expect(clinicalDocument.elements.length).toBe(originalLength + 4);
     });
 
     // Choices
@@ -1088,6 +1088,142 @@ describe('StructureDefinition', () => {
       expect(respRate.elements.length).toBe(originalLength);
     });
 
+    it('should not create a slicing or slice for single-choice elements when finding it by a type-specific path ', () => {
+      const originalLength = observation.elements.length;
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      // Since there is only one possible choice, it should return the choice element directly
+      const valueQuantity = observation.findElementByPath('valueQuantity', fisher);
+      expect(valueX.slicing).toBeUndefined();
+      expect(valueQuantity).toBe(valueX);
+      expect(valueQuantity.id).toBe('Observation.value[x]');
+      expect(observation.elements.length).toBe(originalLength);
+    });
+
+    it('should not create a slicing or slice for single-choice elements when finding it by a type-specific path to a child element ', () => {
+      const originalLength = observation.elements.length;
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      // Since there is only one possible choice, it should return the choice element directly
+      const valueQuantityUnit = observation.findElementByPath('valueQuantity.unit', fisher);
+      expect(valueX.slicing).toBeUndefined();
+      expect(valueQuantityUnit).toBeDefined();
+      expect(valueQuantityUnit.parent()).toBe(valueX);
+      expect(valueQuantityUnit.id).toBe('Observation.value[x].unit');
+      expect(observation.elements.length).toBe(originalLength + 7); // Plus 7 for unfolded Quantity elements
+    });
+
+    it('should create a slicing and slice for two-choice elements when finding it by a type-specific path', () => {
+      const originalLength = observation.elements.length;
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.type = [
+        new ElementDefinitionType('CodeableConcept'),
+        new ElementDefinitionType('Quantity')
+      ];
+      // Since there is more than one choice, it should slice to distinguish between them
+      const valueQuantity = observation.findElementByPath('valueQuantity', fisher);
+      expect(valueX.slicing).toBeDefined();
+      expect(valueX.slicing.discriminator[0]).toEqual({ type: 'type', path: '$this' });
+      expect(valueX.slicing.ordered).toBe(false);
+      expect(valueX.slicing.rules).toBe('open');
+      expect(valueQuantity).toBeDefined();
+      expect(valueQuantity.id).toBe('Observation.value[x]:valueQuantity');
+      expect(valueQuantity.slicing).toBeUndefined();
+      expect(valueQuantity.sliceName).toBe('valueQuantity');
+      expect(valueQuantity.path).toBe('Observation.value[x]');
+      expect(valueQuantity.min).toBe(0);
+      expect(observation.elements.length).toBe(originalLength + 1);
+    });
+
+    it('should create a slicing and slice for two-choice elements when finding it by a type-specific path to a child element', () => {
+      const originalLength = observation.elements.length;
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.type = [
+        new ElementDefinitionType('CodeableConcept'),
+        new ElementDefinitionType('Quantity')
+      ];
+      // Since there is more than one choice, it should slice to distinguish between them
+      const valueQuantityUnit = observation.findElementByPath('valueQuantity.unit', fisher);
+      expect(valueX.slicing).toBeDefined();
+      expect(valueX.slicing.discriminator[0]).toEqual({ type: 'type', path: '$this' });
+      expect(valueX.slicing.ordered).toBe(false);
+      expect(valueX.slicing.rules).toBe('open');
+      expect(valueQuantityUnit).toBeDefined();
+      expect(valueQuantityUnit.id).toBe('Observation.value[x]:valueQuantity.unit');
+      const valueQuantity = valueQuantityUnit.parent();
+      expect(valueQuantity.slicing).toBeUndefined();
+      expect(valueQuantity.sliceName).toBe('valueQuantity');
+      expect(valueQuantity.path).toBe('Observation.value[x]');
+      expect(valueQuantity.min).toBe(0);
+      expect(observation.elements.length).toBe(originalLength + 8); // Plus 8 for slice and unfolded Quantity elements
+    });
+
+    it('should create a slice for single-choice elements with other existing slices when finding it by a type-specific path', () => {
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.sliceIt('type', '$this', false, 'open');
+      valueX.addSlice('valueCodeableConcept', new ElementDefinitionType('valueCodeableConcept'));
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      const originalLength = observation.elements.length;
+      // Since there are already other slices, even though this is now a single-choice,
+      // it should create a new slice to be consistent with what's already there.
+      // (This was a subjective decision on our part, not necessarily driven be spec).
+      const valueQuantity = observation.findElementByPath('valueQuantity', fisher);
+      expect(valueQuantity).toBeDefined();
+      expect(valueQuantity.id).toBe('Observation.value[x]:valueQuantity');
+      expect(valueQuantity.slicing).toBeUndefined();
+      expect(valueQuantity.sliceName).toBe('valueQuantity');
+      expect(valueQuantity.path).toBe('Observation.value[x]');
+      expect(valueQuantity.min).toBe(0);
+      expect(observation.elements.length).toBe(originalLength + 1);
+    });
+
+    it('should create a slice for single-choice elements with other existing slices when finding it by a type-specific path to a child element ', () => {
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.sliceIt('type', '$this', false, 'open');
+      valueX.addSlice('valueCodeableConcept', new ElementDefinitionType('valueCodeableConcept'));
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      const originalLength = observation.elements.length;
+      // Since there are already other slices, even though this is now a single-choice,
+      // it should create a new slice to be consistent with what's already there.
+      // (This was a subjective decision on our part, not necessarily driven be spec).
+      const valueQuantityUnit = observation.findElementByPath('valueQuantity.unit', fisher);
+      expect(valueQuantityUnit).toBeDefined();
+      expect(valueQuantityUnit.id).toBe('Observation.value[x]:valueQuantity.unit');
+      const valueQuantity = valueQuantityUnit.parent();
+      expect(valueQuantity.slicing).toBeUndefined();
+      expect(valueQuantity.sliceName).toBe('valueQuantity');
+      expect(valueQuantity.path).toBe('Observation.value[x]');
+      expect(valueQuantity.min).toBe(0);
+      expect(observation.elements.length).toBe(originalLength + 8); // Plus 8 for slice and unfolded Quantity elements
+    });
+
+    it('should return a pre-existing slice for single-choice elements when finding it by a type-specific path', () => {
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.sliceIt('type', '$this', false, 'open');
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      const slice = valueX.addSlice('valueQuantity', new ElementDefinitionType('Quantity'));
+      const originalLength = observation.elements.length;
+      // Since a slice for Quantity already exists, it should return that slice directly
+      const valueQuantity = observation.findElementByPath('valueQuantity', fisher);
+      expect(valueQuantity).toBe(slice);
+      expect(valueQuantity.id).toBe('Observation.value[x]:valueQuantity');
+      expect(observation.elements.length).toBe(originalLength);
+    });
+
+    it('should return a pre-existing slice for single-choice elements when finding it by a type-specific path to a child element', () => {
+      const valueX = observation.findElementByPath('value[x]', fisher);
+      valueX.sliceIt('type', '$this', false, 'open');
+      valueX.type = [new ElementDefinitionType('Quantity')];
+      const slice = valueX.addSlice('valueQuantity', new ElementDefinitionType('Quantity'));
+      const originalLength = observation.elements.length;
+      // Since a slice for Quantity already exists, it should return that slice directly
+      const valueQuantityUnit = observation.findElementByPath('valueQuantity.unit', fisher);
+      expect(valueQuantityUnit).toBeDefined();
+      expect(valueQuantityUnit.parent()).toBe(slice);
+      expect(valueQuantityUnit.id).toBe('Observation.value[x]:valueQuantity.unit');
+      expect(observation.elements.length).toBe(originalLength + 7); // Plus 7 for unfolded Quantity elements
+    });
+
     // Unfolding
     it('should find an element that must be unfolded by path', () => {
       const originalLength = observation.elements.length;
@@ -1614,7 +1750,7 @@ describe('StructureDefinition', () => {
         brackets: ['maiden-name', '0'],
         slices: ['maiden-name']
       });
-      expect(respRate.elements.length).toBe(originalLength + 6);
+      expect(respRate.elements.length).toBe(originalLength + 5);
     });
 
     it('should rename modifierExtensions when they are referred to by name instead of sliceName', () => {
@@ -1636,7 +1772,7 @@ describe('StructureDefinition', () => {
         brackets: ['maiden-name', '0'],
         slices: ['maiden-name']
       });
-      expect(respRate.elements.length).toBe(originalLength + 6);
+      expect(respRate.elements.length).toBe(originalLength + 5);
     });
 
     describe('#Inline Instances', () => {