Repository: FHIR/sushi
Pull request #312 (2020-03-27 17:45:45)

Weaken cardinality checks on slices
This fixes a bug that @cmoesel found on the checking of slice cardinalities. We were being too aggressive in our approach. An individual slice should be allowed to have min < min of sliced element. The min on the sliced element indicates that at least some amount of elements must be present, but it does not indicate that a certain amount of each slice must be present. Therefore we were throwing an error on a valid case. 

I believe it should still be the case that a child element of an individual slice should have min > the cardinalty of that child on the root sliced element. So this PR changes the cardinality check to only throw an error when the cardinality of a child on a slice is < the cardinality of the corresponding child on the root, and not to throw an error when cardinality of a slice is < cardinality of the root sliced element.

There are new tests to test this, but to further test, run SUSHI on master against fsh-mcode, and you will see the extraneous errors that are being generated. Then run SUSHI on this branch and notice that the errors go away. I'd also suggest running SUSHI on https://github.com/FHIR/sushi/commit/e5e4ed54009ab867eb55814d7be48b44801f6ddf, since this was the last commit before this bug was introduced. If you perform a diff on the output of running SUSHI on this branch and https://github.com/FHIR/sushi/commit/e5e4ed54009ab867eb55814d7be48b44801f6ddf, you will notice the output is the same, except for the addition of a path-history parameter, but that is expected.


Patch:
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index d838990..ad03751 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -473,17 +473,23 @@ export class ElementDefinition {
     if (connectedElements.length > 0) {
       // check to see if the card constraint would actually be a problem for the connected element
       // that is to say, if the new card is narrower than the connected card
+      connectedElements
+        .filter(ce => !(ce.path === this.path && ce.id.startsWith(this.id)))
+        // Filter out elements that are directly slices of this, since they may have min < this.min
+        // Children of slices however must have min >= this.min
+        .forEach(ce => {
+          if (ce.min != null && ce.min < min) {
+            throw new NarrowingRootCardinalityError(
+              this.path,
+              ce.id,
+              min,
+              max,
+              ce.min,
+              ce.max ?? '*'
+            );
+          }
+        });
       connectedElements.forEach(ce => {
-        if (ce.min != null && ce.min < min) {
-          throw new NarrowingRootCardinalityError(
-            this.path,
-            ce.id,
-            min,
-            max,
-            ce.min,
-            ce.max ?? '*'
-          );
-        }
         // if the connected element's max is not null and is not *, we can't make the max smaller than its max
         if (ce.max != null && ce.max != '*' && maxInt != null && maxInt < parseInt(ce.max)) {
           throw new NarrowingRootCardinalityError(this.path, ce.id, min, max, ce.min ?? 0, ce.max);


Test patch:
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index 4930e2c..93d2b5c 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -2629,7 +2629,52 @@ describe('StructureDefinitionExporter', () => {
       expect(loggerSpy.getAllLogs('error')).toHaveLength(0);
     });
 
-    it('should not apply a CardRule that would make the cardinality of the child of a slice too wide', () => {
+    it('should apply a CardRule that would make the cardinality of a slice smaller than the root', () => {
+      loggerSpy.reset();
+      // component[Lab] has card 0..1
+      // * component 1..5
+
+      const rootCard = new CardRule('component')
+        .withFile('Narrower.fsh')
+        .withLocation([7, 9, 7, 23]);
+      rootCard.min = 1;
+      rootCard.max = '5';
+
+      observationWithSlice.rules.push(rootCard);
+      doc.profiles.set(observationWithSlice.name, observationWithSlice);
+      exporter.export();
+      const sd = pkg.profiles[0];
+      expect(sd.findElement('Observation.component').max).toBe('5');
+      expect(sd.findElement('Observation.component').min).toBe(1);
+      expect(sd.findElement('Observation.component:Lab').min).toBe(0);
+      expect(sd.findElement('Observation.component:Lab').max).toBe('1');
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+    });
+
+    it('should not apply a CardRule that would make the cardinality of the child of a slice too small', () => {
+      loggerSpy.reset();
+      // * component[Lab].interpretation 0..4
+      // * component.interpretation 1..5 // this rule is invalid!
+      const labCard = new CardRule('component[Lab].interpretation');
+      labCard.max = '4';
+      const rootCard = new CardRule('component.interpretation')
+        .withFile('Narrower.fsh')
+        .withLocation([7, 9, 7, 23]);
+      rootCard.min = 1;
+      rootCard.max = '5';
+
+      observationWithSlice.rules.push(labCard, rootCard);
+      doc.profiles.set(observationWithSlice.name, observationWithSlice);
+      exporter.export();
+      const sd = pkg.profiles[0];
+      expect(sd.findElement('Observation.component.interpretation').max).toBe('*');
+      expect(sd.findElement('Observation.component.interpretation').min).toBe(0);
+      expect(sd.findElement('Observation.component:Lab.interpretation').max).toBe('4');
+      expect(loggerSpy.getLastMessage('error')).toMatch(/cannot be narrowed/s);
+      expect(loggerSpy.getLastMessage('error')).toMatch(/File: Narrower\.fsh.*Line: 7\D*/s);
+    });
+
+    it('should not apply a CardRule that would make the cardinality of the child of a slice too large', () => {
       // * component[Lab].interpretation 0..9
       // * component.interpretation 0..5 // this rule is invalid!
       const labCard = new CardRule('component[Lab].interpretation');