entities with same name (#1528)

* adding conditional for checking same name with entities, start to tests * added earlier conditional, tests, modified tests affected * edit to conditional, tests from feedback * fix to tests, prettier * format, cleanup * removing first approach within exporters * adding conditional to fshtank, unit tests * reverting first approach unit test * reverting changes to elementdefinition.constraintype.test.ts * convert error to warning, log singular warning with duplicate entity names * adding space between duplicate names in warning message --------- Co-authored-by: Chris Moesel <[email protected]>
FHIR · Dec 30, 2024 · dc4b717 · dc4b717
1 parent 2bfe6ad
commit dc4b717
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 0 deletions.
diff --git a/src/app.ts b/src/app.ts
@@ -269,6 +269,7 @@ async function runBuild(input: string, program: OptionValues, helpText: string)
     config = readConfig(originalInput);
     updateConfig(config, program);
     tank = fillTank(rawFSH, config);
+    tank.checkDuplicateNameEntities();
   } catch (e) {
     // If no errors have been logged yet, log this exception so the user knows why we're exiting
     if (stats.numError === 0) {

diff --git a/src/import/FSHTank.ts b/src/import/FSHTank.ts
@@ -25,6 +25,7 @@ import {
 } from '../fhirtypes/common';
 import flatMap from 'lodash/flatMap';
 import { getNonInstanceValueFromRules } from '../fshtypes/common';
+import { logger } from '../utils/FSHLogger';
 
 export class FSHTank implements Fishable {
   constructor(
@@ -139,6 +140,50 @@ export class FSHTank implements Fishable {
     return undefined;
   }
 
+  checkDuplicateNameEntities(): undefined {
+    const allEntities = [
+      ...this.getAllStructureDefinitions(),
+      ...this.getAllInstances(),
+      ...this.getAllMappings(),
+      ...this.getAllInvariants(),
+      ...this.getAllValueSets(),
+      ...this.getAllCodeSystems(),
+      ...this.getAllRuleSets(),
+      ...this.getAllExtensions()
+    ];
+
+    const duplicateEntities = new Set();
+    allEntities.forEach(entity => {
+      if (
+        this.docs.some(
+          doc =>
+            (doc.profiles.has(entity.name) && doc.profiles.get(entity.name) != entity) ||
+            (doc.extensions.has(entity.name) && doc.extensions.get(entity.name) != entity) ||
+            (doc.logicals.has(entity.name) && doc.logicals.get(entity.name) != entity) ||
+            (doc.resources.has(entity.name) && doc.resources.get(entity.name) != entity) ||
+            (doc.instances.has(entity.name) && doc.instances.get(entity.name) != entity) ||
+            (doc.mappings.has(entity.name) && doc.mappings.get(entity.name) != entity) ||
+            (doc.invariants.has(entity.name) && doc.invariants.get(entity.name) != entity) ||
+            (doc.valueSets.has(entity.name) && doc.valueSets.get(entity.name) != entity) ||
+            (doc.codeSystems.has(entity.name) && doc.codeSystems.get(entity.name) != entity) ||
+            (doc.ruleSets.has(entity.name) && doc.ruleSets.get(entity.name) != entity)
+        )
+      ) {
+        duplicateEntities.add(entity.name);
+      }
+    });
+
+    if (duplicateEntities.size > 0) {
+      logger.warn(
+        'Detected FSH entity definitions with duplicate names. While FSH allows for duplicate ' +
+          'names across entity types, they can lead to ambiguous results when referring to these ' +
+          'entities by name elsewhere (e.g., in references). Consider using unique names in FSH ' +
+          'declarations and assigning duplicated names using caret assignment rules instead. ' +
+          `Detected duplicate names: ${Array.from(duplicateEntities).join(', ')}.`
+      );
+    }
+  }
+
   fish(
     item: string,
     ...types: Type[]

diff --git a/src/run/FshToFhir.ts b/src/run/FshToFhir.ts
@@ -73,6 +73,7 @@ export async function fshToFhir(
     rawFSHes.push(new RawFSH(input));
   }
   const tank = fillTank(rawFSHes, config);
+  tank.checkDuplicateNameEntities();
 
   // process FSH text into FHIR
   const outPackage = exportFHIR(tank, defs);

diff --git a/test/run/FshToFhir.test.ts b/test/run/FshToFhir.test.ts
@@ -170,6 +170,79 @@ describe('#FshToFhir', () => {
       expect(results.fhir[1].snapshot).toBeUndefined();
     });
 
+    it('should throw error when converting FSH into FHIR with several inputs of different entity types with duplicate names', async () => {
+      const results = await fshToFhir([
+        leftAlign(`
+      Profile: MyPatient1
+      Parent: Patient
+      * name MS
+       `),
+        leftAlign(`
+      Instance: MyPatient1
+      InstanceOf: MyPatient1
+       `),
+        leftAlign(`
+      Profile: MyPatient2
+      Parent: Patient
+      * name MS
+        `),
+        leftAlign(`
+      Instance: MyPatient2
+      InstanceOf: MyPatient2
+       `),
+        leftAlign(`
+      Profile: MyPatient3
+      Parent: Patient
+      * name MS
+        `),
+        leftAlign(`
+      Instance: MyPatient4
+      InstanceOf: MyPatient3
+        `),
+        leftAlign(`
+      Instance: MyPatient5
+      InstanceOf: MyPatient3
+     `)
+      ]);
+      expect(results.errors).toHaveLength(0);
+      expect(results.warnings).toHaveLength(1);
+      expect(results.fhir).toHaveLength(7);
+      expect(results.fhir[0].id).toBe('MyPatient1');
+      expect(results.fhir[1].id).toBe('MyPatient2');
+      expect(results.fhir[2].id).toBe('MyPatient3');
+      expect(results.fhir[3].id).toBe('MyPatient1');
+      expect(results.fhir[4].id).toBe('MyPatient2');
+      expect(results.fhir[5].id).toBe('MyPatient4');
+      expect(results.fhir[6].id).toBe('MyPatient5');
+
+      expect(results.warnings[0].message).toMatch(
+        'Detected FSH entity definitions with duplicate names. While FSH allows for duplicate ' +
+          'names across entity types, they can lead to ambiguous results when referring to these ' +
+          'entities by name elsewhere (e.g., in references). Consider using unique names in FSH ' +
+          'declarations and assigning duplicated names using caret assignment rules instead. ' +
+          'Detected duplicate names: MyPatient1, MyPatient2.'
+      );
+    });
+
+    it('should not throw error when converting FSH into FHIR with several inputs of different entity types with different names', async () => {
+      const results = await fshToFhir([
+        leftAlign(`
+      Profile: MyPatient1
+      Parent: Patient
+      * name MS
+       `),
+        leftAlign(`
+      Instance: MyPatient2
+      InstanceOf: MyPatient1
+       `)
+      ]);
+      expect(results.errors).toHaveLength(0);
+      expect(results.warnings).toHaveLength(0);
+      expect(results.fhir).toHaveLength(2);
+      expect(results.fhir[0].id).toBe('MyPatient1');
+      expect(results.fhir[1].id).toBe('MyPatient2');
+    });
+
     it('should trace errors back to the originating input when multiple inputs are given', async () => {
       const results = await fshToFhir([
         leftAlign(`