Repository: FHIR/sushi (ts)

Issue:
`Canonical()` behavior when a CodeSystem and a ValueSet have exact same Ids
If we define a CodeSystem and a ValueSet with the exact Ids:

```
CodeSystem: FooBar
Id:         foo-bar
Title:     "Foo Bar CodeSystem"

* #v1
    "value1"
* #v2
    "value2"
```

```
ValueSet: FooBar
Id: foo-bar
Title: "Foo Bar ValueSet"

* include codes from system FooBar
```

`Canonical(FooBar)` always return the canonical url of the ValueSet. 

* Is there a way we could choose which resource's url we'd like to receive?
* If not, it would be great if SUSHI could through an error to highlight there is some ambiguity here. 

Pull request:
entities with same name
**Description:** This PR adds conditionals to check for entities that exist with the same name, and logs an error when entities are defined with a duplicate name.

**Testing Instructions:** jest unit tests

**Related Issue:** Fixes #1469


Patch:
diff --git a/src/app.ts b/src/app.ts
index b7c4a12..dc709aa 100644
--- 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
index 8f9b1ff..bb380d9 100644
--- 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
index 73e6c13..fd3ca4e 100644
--- 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);


Test patch:
diff --git a/test/run/FshToFhir.test.ts b/test/run/FshToFhir.test.ts
index 5e17247..fd9d3f3 100644
--- 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(`