Repository: FHIR/sushi (ts)

Issue:
Warn on empty string
FHIR indicates that `string` values should have at least one non-whitespace character (see: https://build.fhir.org/datatypes#string).  SUSHI should warn users when they assign an empty string.

If this is complex to do for arbitrary assignments, we should at least do it for the major metadata keywords (`Title:`, `Description:`, etc) -- especially since the VS Code snippets produce empty strings for these by default.

See: https://chat.fhir.org/#narrow/stream/179252-IG-creation/topic/Miss.20leading.20QA.2Ehtml.20results/near/282548095

Pull request:
Warn on empty strings
Fixes #1087 

Adds warnings whenever the user makes their Title or Description an empty string within their Instance, Code System, Structure Definition, or Value Set. 

Testers would most likely want to test situations where Title or Description within these types exist but are empty strings. 

Thanks for reviewing!

Patch:
diff --git a/src/export/CodeSystemExporter.ts b/src/export/CodeSystemExporter.ts
index 0674081..2220afd 100644
--- a/src/export/CodeSystemExporter.ts
+++ b/src/export/CodeSystemExporter.ts
@@ -22,6 +22,14 @@ export class CodeSystemExporter {
   private setMetadata(codeSystem: CodeSystem, fshDefinition: FshCodeSystem): void {
     codeSystem.setName(fshDefinition);
     codeSystem.setId(fshDefinition);
+    if (fshDefinition.title == '') {
+      logger.warn(`Code system ${fshDefinition.name} has a title field that should not be empty.`);
+    }
+    if (fshDefinition.description == '') {
+      logger.warn(
+        `Code system ${fshDefinition.name} has a description field that should not be empty.`
+      );
+    }
     if (fshDefinition.title) codeSystem.title = fshDefinition.title;
     if (fshDefinition.description) codeSystem.description = fshDefinition.description;
     // Version is set to value provided in config, will be overriden if reset by rules
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 98f8225..34994ad 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -476,6 +476,14 @@ export class InstanceExporter implements Fishable {
     const instanceOfStructureDefinition = StructureDefinition.fromJSON(json);
     let instanceDef = new InstanceDefinition();
     instanceDef._instanceMeta.name = fshDefinition.id; // This is name of the instance in the FSH
+    if (fshDefinition.title == '') {
+      logger.warn(`Instance ${fshDefinition.name} has a title field that should not be empty.`);
+    }
+    if (fshDefinition.description == '') {
+      logger.warn(
+        `Instance ${fshDefinition.name} has a description field that should not be empty.`
+      );
+    }
     if (fshDefinition.title) {
       instanceDef._instanceMeta.title = fshDefinition.title;
     }
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index 7f08f48..58e905d 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -299,6 +299,18 @@ export class StructureDefinitionExporter implements Fishable {
     delete structDef.identifier;
     structDef.version = this.tank.config.version; // can be overridden using a rule
     structDef.setName(fshDefinition);
+    if (fshDefinition.title == '') {
+      if (fshDefinition instanceof Profile) {
+      }
+      logger.warn(
+        `${fshDefinition.constructorName} ${fshDefinition.name} has a title field that should not be empty.`
+      );
+    }
+    if (fshDefinition.description == '') {
+      logger.warn(
+        `${fshDefinition.constructorName} ${fshDefinition.name} has a description field that should not be empty.`
+      );
+    }
     if (fshDefinition.title) {
       structDef.title = fshDefinition.title;
       if (
diff --git a/src/export/ValueSetExporter.ts b/src/export/ValueSetExporter.ts
index 0e901b1..8ed5c7a 100644
--- a/src/export/ValueSetExporter.ts
+++ b/src/export/ValueSetExporter.ts
@@ -26,6 +26,14 @@ export class ValueSetExporter {
   private setMetadata(valueSet: ValueSet, fshDefinition: FshValueSet): void {
     valueSet.setName(fshDefinition);
     valueSet.setId(fshDefinition);
+    if (fshDefinition.title == '') {
+      logger.warn(`Value set ${fshDefinition.name} has a title field that should not be empty.`);
+    }
+    if (fshDefinition.description == '') {
+      logger.warn(
+        `Value set ${fshDefinition.name} has a description field that should not be empty.`
+      );
+    }
     if (fshDefinition.title) {
       valueSet.title = fshDefinition.title;
     }


Test patch:
diff --git a/test/export/CodeSystemExporter.test.ts b/test/export/CodeSystemExporter.test.ts
index a3324da..87d32f1 100644
--- a/test/export/CodeSystemExporter.test.ts
+++ b/test/export/CodeSystemExporter.test.ts
@@ -214,6 +214,23 @@ describe('CodeSystemExporter', () => {
     });
   });
 
+  it('should warn when title and/or description is an empty string', () => {
+    const codeSystem = new FshCodeSystem('MyCodeSystem');
+    codeSystem.title = '';
+    codeSystem.description = '';
+    doc.codeSystems.set(codeSystem.name, codeSystem);
+    const exported = exporter.export().codeSystems;
+    expect(exported.length).toBe(1);
+
+    expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+    expect(loggerSpy.getFirstMessage('warn')).toMatch(
+      'Code system MyCodeSystem has a title field that should not be empty.'
+    );
+    expect(loggerSpy.getLastMessage('warn')).toMatch(
+      'Code system MyCodeSystem has a description field that should not be empty.'
+    );
+  });
+
   it('should log a message when the code system has an invalid id', () => {
     const codeSystem = new FshCodeSystem('StrangeSystem')
       .withFile('Strange.fsh')
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index e788ecb..bc9a53d 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -109,6 +109,25 @@ describe('InstanceExporter', () => {
     expect(loggerSpy.getLastMessage('error')).toMatch(/File: Incorrect\.fsh.*Line: 15 - 18\D*/s);
   });
 
+  it('should warn when title and/or description is an empty string', () => {
+    const instance = new Instance('MyInstance');
+    instance.instanceOf = 'Patient';
+    instance.title = '';
+    instance.description = '';
+    doc.instances.set(instance.name, instance);
+    const exported = exporter.export().instances;
+
+    expect(exported.length).toBe(1);
+
+    expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+    expect(loggerSpy.getFirstMessage('warn')).toMatch(
+      'Instance MyInstance has a title field that should not be empty.'
+    );
+    expect(loggerSpy.getLastMessage('warn')).toMatch(
+      'Instance MyInstance has a description field that should not be empty.'
+    );
+  });
+
   it('should export instances with InstanceOf FSHy profile', () => {
     const profileFoo = new Profile('Foo');
     profileFoo.parent = 'Patient';
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index 8140b1d..c378734 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -82,6 +82,80 @@ describe('StructureDefinitionExporter R4', () => {
       expect(pkg.extensions.length).toBe(1);
     });
 
+    it('should warn when the structDef is a profile and title and/or description is an empty string', () => {
+      const profile = new Profile('Foo');
+      profile.parent = 'Patient';
+      profile.title = '';
+      profile.description = '';
+      doc.profiles.set(profile.name, profile);
+      const exported = exporter.export();
+      expect(exported.profiles.length).toBe(1);
+
+      expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+      expect(loggerSpy.getFirstMessage('warn')).toMatch(
+        'Profile Foo has a title field that should not be empty.'
+      );
+      expect(loggerSpy.getLastMessage('warn')).toMatch(
+        'Profile Foo has a description field that should not be empty.'
+      );
+    });
+
+    it('should warn when the structDef is an extension and title and/or description is an empty string', () => {
+      const extension = new Extension('Bar');
+      extension.parent = 'Extension';
+      extension.title = '';
+      extension.description = '';
+      doc.extensions.set(extension.name, extension);
+      const exported = exporter.export();
+
+      expect(exported.extensions.length).toBe(1);
+
+      expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+      expect(loggerSpy.getFirstMessage('warn')).toMatch(
+        'Extension Bar has a title field that should not be empty.'
+      );
+      expect(loggerSpy.getLastMessage('warn')).toMatch(
+        'Extension Bar has a description field that should not be empty.'
+      );
+    });
+
+    it('should warn when the structDef is a logical and title and/or description is an empty string', () => {
+      const logical = new Logical('BackFooTheFuture');
+      logical.parent = 'Element';
+      logical.title = '';
+      logical.description = '';
+      doc.logicals.set(logical.name, logical);
+      const exported = exporter.export();
+
+      expect(exported.logicals.length).toBe(1);
+
+      expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+      expect(loggerSpy.getFirstMessage('warn')).toMatch(
+        'Logical BackFooTheFuture has a title field that should not be empty.'
+      );
+      expect(loggerSpy.getLastMessage('warn')).toMatch(
+        'Logical BackFooTheFuture has a description field that should not be empty.'
+      );
+    });
+
+    it('should warn when the structDef is a resource and title and/or description is an empty string', () => {
+      const resource = new Resource('SpidermanBarFromHome');
+      resource.parent = 'Resource';
+      resource.id = 'PatientResource';
+      resource.title = '';
+      resource.description = '';
+      doc.resources.set(resource.name, resource);
+      exporter.exportStructDef(resource);
+
+      expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+      expect(loggerSpy.getFirstMessage('warn')).toMatch(
+        'Resource SpidermanBarFromHome has a title field that should not be empty.'
+      );
+      expect(loggerSpy.getLastMessage('warn')).toMatch(
+        'Resource SpidermanBarFromHome has a description field that should not be empty.'
+      );
+    });
+
     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';
diff --git a/test/export/ValueSetExporter.test.ts b/test/export/ValueSetExporter.test.ts
index 460940b..2e54922 100644
--- a/test/export/ValueSetExporter.test.ts
+++ b/test/export/ValueSetExporter.test.ts
@@ -91,6 +91,23 @@ describe('ValueSetExporter', () => {
     });
   });
 
+  it('should warn when title and/or description is an empty string', () => {
+    const valueSet = new FshValueSet('BreakfastVS');
+    valueSet.title = '';
+    valueSet.description = '';
+    doc.valueSets.set(valueSet.name, valueSet);
+    const exported = exporter.export().valueSets;
+    expect(exported.length).toBe(1);
+
+    expect(loggerSpy.getAllMessages('warn').length).toBe(2);
+    expect(loggerSpy.getFirstMessage('warn')).toMatch(
+      'Value set BreakfastVS has a title field that should not be empty.'
+    );
+    expect(loggerSpy.getLastMessage('warn')).toMatch(
+      'Value set BreakfastVS has a description field that should not be empty.'
+    );
+  });
+
   it('should log a message when the value set has an invalid id', () => {
     const valueSet = new FshValueSet('BreakfastVS')
       .withFile('Breakfast.fsh')