Repository: FHIR/sushi
Pull request #494 (2020-06-16 13:53:52)

Log an error when constraining non-abstract type to specialization
A type that is not abstract cannot be constrained to any specializations of that type. When the type is not abstract, the type is final, and so it can only be constrained to profiles of that type. The best example is `Quantity`, which is not abstract, and has both specializations, such as `Duration`, and profiles, such as `SimpleQuantity`. So an element of type `Quantity`, say `valueQuantity`, can have the following rule:
```
* valueQuantity only SimpleQuantity
```
but cannot have:
```
* valueQuantity only Duration
```

SUSHI used to allow the second rule, but the IG Publisher does not, so it would have errors. Now the second rule is disallowed, and an error is logged. The justification for this is in https://chat.fhir.org/#narrow/stream/179177-conformance/topic/Narrow.20Quantity.20to.20Duration.20and.20Age. I don't know if there is a location in the spec that really dives into this distinction. The only example I can find of a non-abstract type being specialized is `Quantity`, but the code shouldn't be specific to `Quantity`.

ngfreiter: Fixes #447

Patch:
diff --git a/src/errors/NonAbstractParentOfSpecializationError.ts b/src/errors/NonAbstractParentOfSpecializationError.ts
new file mode 100644
index 0000000..232a886
--- /dev/null
+++ b/src/errors/NonAbstractParentOfSpecializationError.ts
@@ -0,0 +1,7 @@
+export class NonAbstractParentOfSpecializationError extends Error {
+  constructor(public invalidType: string, public nonAbstractParent: string) {
+    super(
+      `The type ${nonAbstractParent} is not abstract, so it cannot be constrained to the specialization ${invalidType}.`
+    );
+  }
+}
diff --git a/src/errors/index.ts b/src/errors/index.ts
index 8e19f46..ea592cd 100644
--- a/src/errors/index.ts
+++ b/src/errors/index.ts
@@ -43,3 +43,4 @@ export * from './ParentDeclaredAsProfileIdError';
 export * from './InvalidResourceTypeError';
 export * from './InvalidExtensionParentError';
 export * from './InvalidTypeAccessError';
+export * from './NonAbstractParentOfSpecializationError';
diff --git a/src/fhirdefs/FHIRDefinitions.ts b/src/fhirdefs/FHIRDefinitions.ts
index 9440193..67e0c79 100644
--- a/src/fhirdefs/FHIRDefinitions.ts
+++ b/src/fhirdefs/FHIRDefinitions.ts
@@ -145,7 +145,8 @@ export class FHIRDefinitions implements Fishable {
         name: result.name as string,
         sdType: result.type as string,
         url: result.url as string,
-        parent: result.baseDefinition as string
+        parent: result.baseDefinition as string,
+        abstract: result.abstract as boolean
       };
     }
   }
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index b5e7886..d6f18cf 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -28,7 +28,8 @@ import {
   MultipleStandardsStatusError,
   InvalidMappingError,
   InvalidFHIRIdError,
-  DuplicateSliceError
+  DuplicateSliceError,
+  NonAbstractParentOfSpecializationError
 } from '../errors';
 import { setPropertyOnDefinitionInstance, splitOnPathPeriods } from './common';
 import { Fishable, Type, Metadata, logger } from '../utils';
@@ -771,6 +772,7 @@ export class ElementDefinition {
     // type itself or any of its parents.  For example, a BloodPressure profile could match on
     // an Observation already having a BP profile, an Observation type w/ no profiles, a
     // DomainResource type w/ no profiles, or a Resource type w/ no profiles.
+    let specializationOfNonAbstractType = false;
     for (const md of lineage) {
       if (type.isReference) {
         // References always have a code 'Reference' w/ the referenced type's defining URL set as
@@ -787,6 +789,13 @@ export class ElementDefinition {
         matchedType = targetTypes.find(t2 => {
           const matchesUnprofiledResource = t2.code === md.id && isEmpty(t2.profile);
           const matchesProfile = t2.code === md.sdType && t2.profile?.includes(md.url);
+          // True if we match an unprofiled type that is not abstract, is a parent, and that we are
+          // specializing (the type does not match the sdType of the type to match)
+          specializationOfNonAbstractType =
+            matchesUnprofiledResource &&
+            !md.abstract &&
+            md.id !== lineage[0].id &&
+            md.id !== lineage[0].sdType;
           return matchesUnprofiledResource || matchesProfile;
         });
       }
@@ -801,6 +810,8 @@ export class ElementDefinition {
         type.isReference ? `Reference(${type.type})` : type.type,
         targetTypes
       );
+    } else if (specializationOfNonAbstractType) {
+      throw new NonAbstractParentOfSpecializationError(type.type, matchedType.code);
     }
 
     return {
diff --git a/src/utils/Fishable.ts b/src/utils/Fishable.ts
index a143a30..5973bfa 100644
--- a/src/utils/Fishable.ts
+++ b/src/utils/Fishable.ts
@@ -17,6 +17,7 @@ export interface Metadata {
   sdType?: string;
   url?: string;
   parent?: string;
+  abstract?: boolean;
 }
 
 export interface Fishable {


Test patch:
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index addff8c..ed65bac 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -1597,20 +1597,19 @@ describe('StructureDefinitionExporter', () => {
     // * value[x] only Duration or Age
     // * value[x].comparator = #>=
 
-    const profile = new Profile('QuantifiedObservation');
-    profile.parent = 'Observation';
+    const extension = new Extension('QuantifiedExtension');
 
     const onlyRule = new OnlyRule('value[x]');
     onlyRule.types = [{ type: 'Duration' }, { type: 'Age' }];
-    profile.rules.push(onlyRule);
+    extension.rules.push(onlyRule);
 
     const fixedValueRule = new FixedValueRule('value[x].comparator');
     fixedValueRule.fixedValue = new FshCode('>=');
-    profile.rules.push(fixedValueRule);
+    extension.rules.push(fixedValueRule);
 
-    exporter.exportStructDef(profile);
-    const sd = pkg.profiles[0];
-    const fixedElement = sd.findElement('Observation.value[x].comparator');
+    exporter.exportStructDef(extension);
+    const sd = pkg.extensions[0];
+    const fixedElement = sd.findElement('Extension.value[x].comparator');
     expect(fixedElement.patternCode).toBe('>=');
   });
 
diff --git a/test/fhirdefs/FHIRDefinitions.test.ts b/test/fhirdefs/FHIRDefinitions.test.ts
index 9589fc4..83aec43 100644
--- a/test/fhirdefs/FHIRDefinitions.test.ts
+++ b/test/fhirdefs/FHIRDefinitions.test.ts
@@ -289,6 +289,7 @@ describe('FHIRDefinitions', () => {
     it('should find base FHIR resources', () => {
       const conditionByID = defs.fishForMetadata('Condition', Type.Resource);
       expect(conditionByID).toEqual({
+        abstract: false,
         id: 'Condition',
         name: 'Condition',
         sdType: 'Condition',
@@ -303,6 +304,7 @@ describe('FHIRDefinitions', () => {
     it('should find base FHIR primitive types', () => {
       const booleanByID = defs.fishForMetadata('boolean', Type.Type);
       expect(booleanByID).toEqual({
+        abstract: false,
         id: 'boolean',
         name: 'boolean',
         sdType: 'boolean',
@@ -317,6 +319,7 @@ describe('FHIRDefinitions', () => {
     it('should find base FHIR complex types', () => {
       const addressByID = defs.fishForMetadata('Address', Type.Type);
       expect(addressByID).toEqual({
+        abstract: false,
         id: 'Address',
         name: 'Address',
         sdType: 'Address',
@@ -331,6 +334,7 @@ describe('FHIRDefinitions', () => {
     it('should find base FHIR profiles', () => {
       const vitalSignsByID = defs.fishForMetadata('vitalsigns', Type.Profile);
       expect(vitalSignsByID).toEqual({
+        abstract: false,
         id: 'vitalsigns',
         name: 'observation-vitalsigns',
         sdType: 'Observation',
@@ -349,6 +353,7 @@ describe('FHIRDefinitions', () => {
         Type.Extension
       );
       expect(maidenNameExtensionByID).toEqual({
+        abstract: false,
         id: 'patient-mothersMaidenName',
         name: 'mothersMaidenName',
         sdType: 'Extension',
@@ -511,6 +516,7 @@ describe('FHIRDefinitions', () => {
     it('should globally find any definition', () => {
       const conditionByID = defs.fishForMetadata('Condition');
       expect(conditionByID).toEqual({
+        abstract: false,
         id: 'Condition',
         name: 'Condition',
         sdType: 'Condition',
@@ -523,6 +529,7 @@ describe('FHIRDefinitions', () => {
 
       const booleanByID = defs.fishForMetadata('boolean');
       expect(booleanByID).toEqual({
+        abstract: false,
         id: 'boolean',
         name: 'boolean',
         sdType: 'boolean',
@@ -535,6 +542,7 @@ describe('FHIRDefinitions', () => {
 
       const addressByID = defs.fishForMetadata('Address');
       expect(addressByID).toEqual({
+        abstract: false,
         id: 'Address',
         name: 'Address',
         sdType: 'Address',
@@ -547,6 +555,7 @@ describe('FHIRDefinitions', () => {
 
       const vitalSignsProfileByID = defs.fishForMetadata('vitalsigns');
       expect(vitalSignsProfileByID).toEqual({
+        abstract: false,
         id: 'vitalsigns',
         name: 'observation-vitalsigns',
         sdType: 'Observation',
@@ -560,6 +569,7 @@ describe('FHIRDefinitions', () => {
 
       const maidenNameExtensionByID = defs.fishForMetadata('patient-mothersMaidenName');
       expect(maidenNameExtensionByID).toEqual({
+        abstract: false,
         id: 'patient-mothersMaidenName',
         name: 'mothersMaidenName',
         sdType: 'Extension',
diff --git a/test/fhirtypes/ElementDefinition.constrainType.test.ts b/test/fhirtypes/ElementDefinition.constrainType.test.ts
index 8756292..0c792bb 100644
--- a/test/fhirtypes/ElementDefinition.constrainType.test.ts
+++ b/test/fhirtypes/ElementDefinition.constrainType.test.ts
@@ -522,5 +522,18 @@ describe('ElementDefinition', () => {
       }).toThrow(/No definition for the type "VitalBillboards" could be found./);
       expect(clone).toEqual(hasMember);
     });
+
+    it('should throw NonAbstractParentError when constraining a non-abstract parent to a specialization of it', () => {
+      const valueX = observation.elements.find(e => e.id === 'Observation.value[x]');
+      const clone = cloneDeep(valueX);
+      expect(() => {
+        const valueConstraint = new OnlyRule('value[x]');
+        valueConstraint.types = [{ type: 'Duration' }];
+        clone.constrainType(valueConstraint, fisher);
+      }).toThrow(
+        /The type Quantity is not abstract, so it cannot be constrained to the specialization Duration/
+      );
+      expect(clone).toEqual(valueX);
+    });
   });
 });
diff --git a/test/utils/MasterFisher.test.ts b/test/utils/MasterFisher.test.ts
index da851e6..aa9c669 100644
--- a/test/utils/MasterFisher.test.ts
+++ b/test/utils/MasterFisher.test.ts
@@ -129,6 +129,7 @@ describe('MasterFisher', () => {
 
     const resultMD = fisher.fishForMetadata('Patient');
     expect(resultMD).toEqual({
+      abstract: false,
       id: 'Patient',
       name: 'Patient',
       sdType: 'Patient',