Repository: FHIR/sushi
Pull request #125 (2020-01-22 20:38:25)

Report duplicate aliases w/ different values
If a FSHTank contains duplicate aliases with different values, only keep the first one and report the others as errors.  Addresses #124.


Patch:
diff --git a/src/import/FSHImporter.ts b/src/import/FSHImporter.ts
index d2532b8..5358341 100644
--- a/src/import/FSHImporter.ts
+++ b/src/import/FSHImporter.ts
@@ -45,7 +45,6 @@ import {
   ValueSetFilterValueTypeError,
   ValueSetFilterMissingValueError
 } from '../errors';
-import flatMap from 'lodash/flatMap';
 import isEqual from 'lodash/isEqual';
 import sortBy from 'lodash/sortBy';
 
@@ -101,28 +100,44 @@ export class FSHImporter extends FSHVisitor {
   }
 
   import(rawFSHes: RawFSH[]): FSHDocument[] {
+    this.allAliases = new Map();
     const docs: FSHDocument[] = [];
     const contexts: pc.DocContext[] = [];
+
+    // Preprocess the FSH files
     rawFSHes.forEach(rawFSH => {
-      docs.push(new FSHDocument(rawFSH.path));
-      contexts.push(this.parseDoc(rawFSH.content, rawFSH.path));
+      // Create and store doc for main import process
+      const doc = new FSHDocument(rawFSH.path);
+      docs.push(doc);
+
+      // Create and store context for main import process
+      const ctx = this.parseDoc(rawFSH.content, rawFSH.path);
+      contexts.push(ctx);
+
+      // Collect the aliases and store in global map
+      ctx.entity().forEach(e => {
+        if (e.alias()) {
+          const [name, value] = e
+            .alias()
+            .SEQUENCE()
+            .map(s => s.getText());
+          if (this.allAliases.has(name) && this.allAliases.get(name) !== value) {
+            logger.error(
+              `Alias ${name} cannot be redefined to ${value}; it is already defined as ${this.allAliases.get(
+                name
+              )}.`,
+              { file: doc.file ?? '', location: this.extractStartStop(e.alias()) }
+            );
+            // don't set it -- just keep the original definition
+          } else {
+            this.allAliases.set(name, value);
+            doc.aliases.set(name, value);
+          }
+        }
+      });
     });
 
-    // Import all aliases first
-    this.allAliases = new Map(
-      flatMap(
-        contexts.map((context, index) => {
-          this.currentDoc = docs[index];
-          this.currentFile = this.currentDoc.file ?? '';
-          const currentAliases = this.getAliases(context);
-          this.currentDoc = null;
-          this.currentFile = null;
-          return currentAliases;
-        }),
-        aliases => Array.from(aliases)
-      )
-    );
-
+    // Now do the main import
     contexts.forEach((context, index) => {
       this.currentDoc = docs[index];
       this.currentFile = this.currentDoc.file ?? '';
@@ -134,16 +149,6 @@ export class FSHImporter extends FSHVisitor {
     return docs;
   }
 
-  getAliases(ctx: pc.DocContext): Map<string, string> {
-    ctx.entity().forEach(e => {
-      if (e.alias()) {
-        this.visitAlias(e.alias());
-      }
-    });
-
-    return this.currentDoc.aliases;
-  }
-
   visitDoc(ctx: pc.DocContext): void {
     ctx.entity().forEach(e => {
       this.visitEntity(e);
@@ -172,10 +177,6 @@ export class FSHImporter extends FSHVisitor {
     }
   }
 
-  visitAlias(ctx: pc.AliasContext): void {
-    this.currentDoc.aliases.set(ctx.SEQUENCE()[0].getText(), ctx.SEQUENCE()[1].getText());
-  }
-
   visitProfile(ctx: pc.ProfileContext) {
     const profile = new Profile(ctx.SEQUENCE().getText())
       .withLocation(this.extractStartStop(ctx))


Test patch:
diff --git a/test/import/FSHImporter.Alias.test.ts b/test/import/FSHImporter.Alias.test.ts
index a357ffb..1957776 100644
--- a/test/import/FSHImporter.Alias.test.ts
+++ b/test/import/FSHImporter.Alias.test.ts
@@ -1,11 +1,14 @@
 import { importText, RawFSH } from '../../src/import';
 import { ValueSetRule } from '../../src/fshtypes/rules';
 import { importSingleText } from '../testhelpers/importSingleText';
+import { loggerSpy } from '../testhelpers';
 
 // Aliases are tested as part of the other entity tests where aliases are allowed
 // but these tests ensure that aliases work generally and can be in any order
 
 describe('FSHImporter', () => {
+  beforeEach(() => loggerSpy.reset());
+
   describe('Alias', () => {
     it('should collect and return aliases in result', () => {
       const input = `
@@ -31,6 +34,70 @@ describe('FSHImporter', () => {
       expect(result.aliases.get('UCUM')).toBe('http://unitsofmeasure.org');
     });
 
+    it('should report when the same alias is defined twice w/ different values in the same file', () => {
+      const input = `
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/ValueSet/omb-race-category
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/StructureDefinition/us-core-race
+      `;
+
+      const result = importSingleText(input);
+      expect(result.aliases.size).toBe(1);
+      expect(result.aliases.get('USCoreRace')).toBe(
+        'http://hl7.org/fhir/us/core/ValueSet/omb-race-category'
+      );
+      expect(loggerSpy.getLastMessage()).toMatch(
+        /Alias USCoreRace cannot be redefined to http:\/\/hl7.org\/fhir\/us\/core\/StructureDefinition\/us-core-race; it is already defined as http:\/\/hl7.org\/fhir\/us\/core\/ValueSet\/omb-race-category\..*Line: 3\D/s
+      );
+    });
+
+    it('should report when the same alias is defined twice w/ different values in different files', () => {
+      const input = `
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/ValueSet/omb-race-category
+      `;
+      const input2 = `
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/StructureDefinition/us-core-race
+      `;
+
+      const results = importText([
+        new RawFSH(input, 'Alias1.fsh'),
+        new RawFSH(input2, 'Alias2.fsh')
+      ]);
+      expect(results).toHaveLength(2);
+      expect(results[0].aliases.size).toBe(1);
+      expect(results[0].aliases.get('USCoreRace')).toBe(
+        'http://hl7.org/fhir/us/core/ValueSet/omb-race-category'
+      );
+      expect(results[1].aliases.size).toBe(0);
+      expect(loggerSpy.getLastMessage()).toMatch(
+        /Alias USCoreRace cannot be redefined to http:\/\/hl7.org\/fhir\/us\/core\/StructureDefinition\/us-core-race; it is already defined as http:\/\/hl7.org\/fhir\/us\/core\/ValueSet\/omb-race-category\..*File: Alias2\.fsh.*Line: 2\D/s
+      );
+    });
+
+    it('should not report an error when the same alias is defined multiple times w/ the same values', () => {
+      const input = `
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/StructureDefinition/us-core-race
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/StructureDefinition/us-core-race
+      `;
+      const input2 = `
+      Alias: USCoreRace = http://hl7.org/fhir/us/core/StructureDefinition/us-core-race
+      `;
+
+      const results = importText([
+        new RawFSH(input, 'Alias1.fsh'),
+        new RawFSH(input2, 'Alias2.fsh')
+      ]);
+      expect(results).toHaveLength(2);
+      expect(results[0].aliases.size).toBe(1);
+      expect(results[0].aliases.get('USCoreRace')).toBe(
+        'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
+      );
+      expect(results[1].aliases.size).toBe(1);
+      expect(results[1].aliases.get('USCoreRace')).toBe(
+        'http://hl7.org/fhir/us/core/StructureDefinition/us-core-race'
+      );
+      expect(loggerSpy.getLastLog()).toBeUndefined();
+    });
+
     it('should translate an alias when the alias is defined before its use', () => {
       const input = `
       Alias: LOINC = http://loinc.org
diff --git a/test/testhelpers/loggerSpy.ts b/test/testhelpers/loggerSpy.ts
index 49995de..30fc23a 100644
--- a/test/testhelpers/loggerSpy.ts
+++ b/test/testhelpers/loggerSpy.ts
@@ -26,7 +26,7 @@ class LoggerSpy {
   }
 
   getMessageAtIndex(index: number): string {
-    return this.getLogAtIndex(index).message;
+    return this.getLogAtIndex(index)?.message;
   }
 
   getFirstMessage(): string {
@@ -36,6 +36,10 @@ class LoggerSpy {
   getLastMessage(): string {
     return this.getMessageAtIndex(-1);
   }
+
+  reset(): void {
+    this.mockWriter.mockReset();
+  }
 }
 
 export const loggerSpy = new LoggerSpy();