Repository: FHIR/sushi (ts)

Issue:
SUSHI should not allow abstract resources as profile Parents of InstanceOfs
SUSHI should not allow abstract resources, like `DomainResource` to be the `Parent` of a `Profile` or the `InstanceOf` of an `Instance`. Right now, it allows it, but then the IG Publisher emits an error.

Reported by @markkramerus on Slack.

Pull request description:
Don't create Instances if the nearest specialization ancestor is abstract
Fixes #1218 and completes task [CIMPL-1070](https://standardhealthrecord.atlassian.net/browse/CIMPL-1070).

~When exporting an Instance, check if InstanceOf is abstract. If it is, log an error and do not create the Instance.~

~When exporting a Profile, check if Parent is an abstract resource. If it is, log an error and do not create the Profile. An abstract Profile is allowed as a parent, however.~

Remove unused InvalidProfileParentError.

Small updates to existing tests for InstanceExporter and StructureDefinitionExporter so that they are no longer breaking these new rules.

In the small regression set, one repo showed changes. The [fhir-sdoh-clinicalcare](https://github.com/HL7/fhir-sdoh-clinicalcare) IG contains an [Instance](https://github.com/HL7/fhir-sdoh-clinicalcare/blob/bf4880b/input/fsh/instances/SDOHCC-GroupSDOHPatientsForPayerExample.fsh) of an [abstract Profile](https://github.com/HL7/fhir-sdoh-clinicalcare/blob/bf4880b/input/fsh/profiles/SDOHCCGroup.fsh), which now throws an error.

Update 2023-03-20: Creating a Profile with an abstract parent is allowed. For Instances, look through the definition hierarchy and find the nearest "specialization" ancestor. If that definition is abstract, don't create the Instance.

Patch:
diff --git a/src/errors/AbstractInstanceOfError.ts b/src/errors/AbstractInstanceOfError.ts
new file mode 100644
index 0000000..f7da7ce
--- /dev/null
+++ b/src/errors/AbstractInstanceOfError.ts
@@ -0,0 +1,14 @@
+import { WithSource } from './WithSource';
+import { SourceInfo } from '../fshtypes';
+
+export class AbstractInstanceOfError extends Error implements WithSource {
+  constructor(
+    public instanceName: string,
+    public instanceOf: string,
+    public sourceInfo: SourceInfo
+  ) {
+    super(
+      `The definition for ${instanceName} is an instance of an abstract resource: ${instanceOf}`
+    );
+  }
+}
diff --git a/src/errors/InvalidProfileParentError.ts b/src/errors/InvalidProfileParentError.ts
deleted file mode 100644
index 478e289..0000000
--- a/src/errors/InvalidProfileParentError.ts
+++ /dev/null
@@ -1,10 +0,0 @@
-import { WithSource } from './WithSource';
-import { SourceInfo } from '../fshtypes';
-
-export class InvalidProfileParentError extends Error implements WithSource {
-  constructor(public childName: string, public parentName: string, public sourceInfo: SourceInfo) {
-    super(
-      `Invalid parent ${parentName} specified for profile ${childName}. The parent of a profile must be a resource, a type, or another profile.`
-    );
-  }
-}
diff --git a/src/errors/index.ts b/src/errors/index.ts
index bfdcd64..8afc033 100644
--- a/src/errors/index.ts
+++ b/src/errors/index.ts
@@ -41,7 +41,6 @@ export * from './ParentDeclaredAsIdError';
 export * from './InvalidResourceTypeError';
 export * from './InvalidExtensionParentError';
 export * from './InvalidLogicalParentError';
-export * from './InvalidProfileParentError';
 export * from './InvalidResourceParentError';
 export * from './InvalidTypeAccessError';
 export * from './NonAbstractParentOfSpecializationError';
@@ -53,3 +52,4 @@ export * from './MismatchedBindingTypeError';
 export * from './ValidationError';
 export * from './InvalidElementForSlicingError';
 export * from './ElementAlreadyDefinedError';
+export * from './AbstractInstanceOfError';
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index dbfc937..d203ca5 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -25,6 +25,7 @@ import {
 } from '../fhirtypes/common';
 import { InstanceOfNotDefinedError } from '../errors/InstanceOfNotDefinedError';
 import { InstanceOfLogicalProfileError } from '../errors/InstanceOfLogicalProfileError';
+import { AbstractInstanceOfError } from '../errors/AbstractInstanceOfError';
 import { Package } from '.';
 import { cloneDeep, isEqual, isMatch, merge, padEnd, uniq, upperFirst } from 'lodash';
 import { AssignmentRule } from '../fshtypes/rules';
@@ -601,6 +602,20 @@ export class InstanceExporter implements Fishable {
       }
     }
 
+    // an instance can't be created if the specialization it is created from is abstract.
+    // see also the FHIR documentation for StructureDefinition.abstract
+    let ancestor = json;
+    while (ancestor != null && ancestor.derivation !== 'specialization') {
+      ancestor = this.fisher.fishForFHIR(ancestor.baseDefinition);
+    }
+    if (ancestor?.abstract === true) {
+      throw new AbstractInstanceOfError(
+        fshDefinition.name,
+        ancestor.name,
+        fshDefinition.sourceInfo
+      );
+    }
+
     const instanceOfStructureDefinition = StructureDefinition.fromJSON(json);
     let instanceDef = new InstanceDefinition();
     instanceDef._instanceMeta.name = fshDefinition.name; // This is name of the instance in the FSH


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index 234d79b..a6fc467 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -110,6 +110,43 @@ describe('InstanceExporter', () => {
     expect(loggerSpy.getLastMessage('error')).toMatch(/File: Incorrect\.fsh.*Line: 15 - 18\D*/s);
   });
 
+  it('should log a message with source information when the instanceOf is an abstract specialization', () => {
+    const instance = new Instance('MyAbstractInstance')
+      .withFile('Abstract.fsh')
+      .withLocation([16, 1, 20, 27]);
+    instance.instanceOf = 'DomainResource';
+    doc.instances.set(instance.name, instance);
+    const exported = exporter.export().instances;
+    expect(exported.length).toBe(0);
+    expect(loggerSpy.getLastMessage('error')).toMatch(/File: Abstract\.fsh.*Line: 16 - 20\D*/s);
+    expect(loggerSpy.getLastMessage('error')).toMatch(
+      /The definition for MyAbstractInstance is an instance of an abstract resource: DomainResource/s
+    );
+  });
+
+  it('should log a message with source information when the instanceOf is a profile whose nearest specialization is abstract', () => {
+    const firstProfile = new Profile('ProfileOfSomethingAbstract');
+    firstProfile.parent = 'DomainResource';
+    doc.profiles.set(firstProfile.name, firstProfile);
+    const secondProfile = new Profile('SecondaryProfile');
+    secondProfile.parent = 'ProfileOfSomethingAbstract';
+    doc.profiles.set(secondProfile.name, secondProfile);
+    sdExporter.export();
+
+    const instance = new Instance('MyAbstractInstance')
+      .withFile('Abstract.fsh')
+      .withLocation([17, 1, 23, 26]);
+    instance.instanceOf = 'SecondaryProfile';
+    doc.instances.set(instance.name, instance);
+
+    const exported = exporter.export().instances;
+    expect(exported.length).toBe(0);
+    expect(loggerSpy.getLastMessage('error')).toMatch(/File: Abstract\.fsh.*Line: 17 - 23\D*/s);
+    expect(loggerSpy.getLastMessage('error')).toMatch(
+      /The definition for MyAbstractInstance is an instance of an abstract resource: DomainResource/s
+    );
+  });
+
   it('should warn when title and/or description is an empty string', () => {
     const instance = new Instance('MyInstance');
     instance.instanceOf = 'Patient';
@@ -8260,7 +8297,7 @@ describe('InstanceExporter', () => {
 
     beforeEach(() => {
       instance = new Instance('Foo');
-      instance.instanceOf = 'Resource';
+      instance.instanceOf = 'Organization';
       doc.instances.set(instance.name, instance);
 
       patientInstance = new Instance('TestPatient');
diff --git a/test/export/StructureDefinition.ProfileExporter.test.ts b/test/export/StructureDefinition.ProfileExporter.test.ts
index 75bd544..075fd91 100644
--- a/test/export/StructureDefinition.ProfileExporter.test.ts
+++ b/test/export/StructureDefinition.ProfileExporter.test.ts
@@ -159,6 +159,26 @@ describe('ProfileExporter', () => {
     expect(exported[2].baseDefinition === exported[1].url);
   });
 
+  it('should export a profile with an abstract profile parent', () => {
+    const profileFoo = new Profile('Foo');
+    profileFoo.parent = 'Basic';
+    const abstractRule = new CaretValueRule('');
+    abstractRule.caretPath = 'abstract';
+    abstractRule.value = true;
+    profileFoo.rules.push(abstractRule);
+    const profileBar = new Profile('Bar');
+    profileBar.parent = 'Foo';
+    doc.profiles.set(profileFoo.name, profileFoo);
+    doc.profiles.set(profileBar.name, profileBar);
+    const exported = exporter.export().profiles;
+    expect(exported.length).toBe(2);
+    expect(exported[0].name).toBe('Foo');
+    expect(exported[0].abstract).toBeTrue();
+    expect(exported[1].name).toBe('Bar');
+    expect(exported[1].abstract).toBeFalse();
+    expect(loggerSpy.getAllMessages('error').length).toBe(0);
+  });
+
   it('should export a profile with a logical parent', () => {
     const profile = new Profile('Foo');
     profile.parent = 'ELTSSServiceModel';
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index a01c31f..1424a7d 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -160,7 +160,7 @@ describe('StructureDefinitionExporter R4', () => {
     it('should log a message when the structure definition has an invalid id', () => {
       const profile = new Profile('Wrong').withFile('Wrong.fsh').withLocation([1, 8, 4, 18]);
       profile.id = 'will?not?work';
-      profile.parent = 'DomainResource';
+      profile.parent = 'Observation';
       doc.profiles.set(profile.name, profile);
       exporter.exportStructDef(profile);
       const exported = pkg.profiles[0];
@@ -172,7 +172,7 @@ describe('StructureDefinitionExporter R4', () => {
     it('should not log a message when the structure definition overrides an invalid id with a Caret Rule', () => {
       const profile = new Profile('Wrong').withFile('Wrong.fsh').withLocation([1, 8, 4, 18]);
       profile.id = 'will?not?work';
-      profile.parent = 'DomainResource';
+      profile.parent = 'Patient';
       const idRule = new CaretValueRule('');
       idRule.caretPath = 'id';
       idRule.value = 'will-work';
@@ -187,7 +187,7 @@ describe('StructureDefinitionExporter R4', () => {
     it('should log a message when the structure definition overrides an invalid id with an invalid Caret Rule', () => {
       const profile = new Profile('Wrong').withFile('Wrong.fsh').withLocation([1, 8, 4, 18]);
       profile.id = 'will?not?work';
-      profile.parent = 'DomainResource';
+      profile.parent = 'Patient';
       const idRule = new CaretValueRule('').withFile('Wrong.fsh').withLocation([3, 8, 3, 18]);
       idRule.caretPath = 'id';
       idRule.value = 'Still Wont Work!';
@@ -203,7 +203,7 @@ describe('StructureDefinitionExporter R4', () => {
     it('should log a message when the structure definition overrides an valid id with an invalid Caret Rule', () => {
       const profile = new Profile('Wrong').withFile('Wrong.fsh').withLocation([1, 8, 4, 18]);
       profile.id = 'valid-id';
-      profile.parent = 'DomainResource';
+      profile.parent = 'Observation';
       const idRule = new CaretValueRule('').withFile('Wrong.fsh').withLocation([3, 8, 3, 18]);
       idRule.caretPath = 'id';
       idRule.value = 'This Is Not Right!';
@@ -218,7 +218,7 @@ describe('StructureDefinitionExporter R4', () => {
 
     it('should log a message when the structure definition has an invalid name', () => {
       const profile = new Profile('Not-good').withFile('Wrong.fsh').withLocation([2, 8, 5, 18]);
-      profile.parent = 'DomainResource';
+      profile.parent = 'Observation';
       doc.profiles.set(profile.name, profile);
       exporter.exportStructDef(profile);
       const exported = pkg.profiles[0];
@@ -231,7 +231,7 @@ describe('StructureDefinitionExporter R4', () => {
 
     it('should not log a message when the structure definition overrides an invalid name with a Caret Rule', () => {
       const profile = new Profile('Not-good').withFile('Wrong.fsh').withLocation([2, 8, 5, 18]);
-      profile.parent = 'DomainResource';
+      profile.parent = 'Observation';
       const nameRule = new CaretValueRule('');
       nameRule.caretPath = 'name';
       nameRule.value = 'NotGood';
@@ -245,7 +245,7 @@ describe('StructureDefinitionExporter R4', () => {
 
     it('should log a message when the structure definition overrides an invalid name with an invalid Caret Rule', () => {
       const profile = new Profile('Not-good').withFile('Wrong.fsh').withLocation([2, 8, 5, 18]);
-      profile.parent = 'DomainResource';
+      profile.parent = 'Observation';
       const nameRule = new CaretValueRule('').withFile('Wrong.fsh').withLocation([3, 8, 3, 18]);
       nameRule.caretPath = 'name';
       nameRule.value = 'Not-good';
@@ -262,7 +262,7 @@ describe('StructureDefinitionExporter R4', () => {
 
     it('should log a message when the structure definition overrides a valid name with an invalid Caret Rule', () => {
       const profile = new Profile('NotGood').withFile('Wrong.fsh').withLocation([2, 8, 5, 18]);
-      profile.parent = 'DomainResource';
+      profile.parent = 'Patient';
       const nameRule = new CaretValueRule('').withFile('Wrong.fsh').withLocation([3, 8, 3, 18]);
       nameRule.caretPath = 'name';
       nameRule.value = 'Not-good';
@@ -279,7 +279,7 @@ describe('StructureDefinitionExporter R4', () => {
 
     it('should sanitize the id and log a message when a valid name is used to make an invalid id', () => {
       const profile = new Profile('Not_good_id').withFile('Wrong.fsh').withLocation([2, 8, 5, 18]);
-      profile.parent = 'DomainResource';
+      profile.parent = 'Patient';
       doc.profiles.set(profile.name, profile);
       exporter.exportStructDef(profile);
       const exported = pkg.profiles[0];