Repository: FHIR/sushi (ts)

Issue:
sushi should check and handle missing array elements
In my haste, I deleted array element 1 and forgot to renumber 

parameter[0].targetProfile = "CEQSubscriptionBundle"
parameter[2].name = #return

This caused sushi to insert a null, resulting in an IG Publisher error 
Not a JSON Object: null (00:13.0946)

Array checking may be worthwhile to ensure that missing elements don't cause IG Publisher crashes.


Pull request:
Logger warning on null values within arrays
Fixes #486 and addresses [CIMPL-416](https://standardhealthrecord.atlassian.net/browse/CIMPL-416), checking and warning when a user has specified a resource with null values within one of its elements. Note that this does not happen on elements with a preceding `_`, as that signifies that an element within an array of primitives has an extension assigned to it. In that case, null values are expected behavior.

[This link](https://fshschool.org/FSHOnline/#/share/3dx0Emf) includes some example FSH that could be used to test these changes. When run on FSHOnline, no warning should be logged to the console. When run against this branch, you should get some warnings letting you know that each of these resources has some null values within their arrays. 

Patch:
diff --git a/src/utils/Processing.ts b/src/utils/Processing.ts
index 7240229..74a891c 100644
--- a/src/utils/Processing.ts
+++ b/src/utils/Processing.ts
@@ -21,6 +21,7 @@ import {
 } from './InstanceDefinitionUtils';
 import { Configuration } from '../fshtypes';
 import { loadConfigurationFromIgResource } from '../import/loadConfigurationFromIgResource';
+import { isPlainObject } from 'lodash';
 
 export function isSupportedFHIRVersion(version: string): boolean {
   // For now, allow current or any 4.x version of FHIR except 4.0.0. This is a quick check; not a guarantee.  If a user passes
@@ -240,6 +241,50 @@ export function fillTank(rawFSHes: RawFSH[], config: Configuration): FSHTank {
   return new FSHTank(docs, config);
 }
 
+export function checkNullValuesOnArray(resource: any, parentName = '', priorPath = ''): void {
+  const resourceName = parentName ? parentName : resource.id ?? resource.name;
+  for (const propertyKey in resource) {
+    const property = resource[propertyKey];
+    const currentPath = !priorPath ? propertyKey : priorPath.concat(`.${propertyKey}`);
+    // If a property's key begins with "_", we'll want to ignore null values on it's top level
+    // but still check any nested objects for null values
+    if (propertyKey.startsWith('_')) {
+      if (isPlainObject(property)) {
+        // If we encounter an object property, we'll want to check its properties as well
+        checkNullValuesOnArray(property, resourceName, currentPath);
+      }
+      if (Array.isArray(property)) {
+        property.forEach((element: any, index: number) => {
+          if (isPlainObject(element)) {
+            // If we encounter an object property, we'll want to check its properties as well
+            checkNullValuesOnArray(element, resourceName, `${currentPath}[${index}]`);
+          }
+        });
+      }
+    } else {
+      if (isPlainObject(property)) {
+        // If we encounter an object property, we'll want to check its properties as well
+        checkNullValuesOnArray(property, resourceName, currentPath);
+      }
+      if (Array.isArray(property)) {
+        const nullIndexes: number[] = [];
+        property.forEach((element: any, index: number) => {
+          if (element === null) nullIndexes.push(index);
+          if (isPlainObject(element)) {
+            // If we encounter an object property, we'll want to check its properties as well
+            checkNullValuesOnArray(element, resourceName, `${currentPath}[${index}]`);
+          }
+        });
+        if (nullIndexes.length > 0) {
+          logger.warn(
+            `The array '${currentPath}' in ${resourceName} is missing values at the following indices: ${nullIndexes}`
+          );
+        }
+      }
+    }
+  }
+}
+
 export function writeFHIRResources(
   outDir: string,
   outPackage: Package,
@@ -272,6 +317,7 @@ export function writeFHIRResources(
             predef.id === resource.id
         )
       ) {
+        checkNullValuesOnArray(resource);
         fs.outputJSONSync(path.join(exportDir, resource.getFileName()), resource.toJSON(snapshot), {
           spaces: 2
         });


Test patch:
diff --git a/test/utils/Processing.test.ts b/test/utils/Processing.test.ts
index c571347..0211391 100644
--- a/test/utils/Processing.test.ts
+++ b/test/utils/Processing.test.ts
@@ -15,7 +15,8 @@ import {
   getRawFSHes,
   hasFshFiles,
   writeFHIRResources,
-  init
+  init,
+  checkNullValuesOnArray
 } from '../../src/utils/Processing';
 import * as loadModule from '../../src/fhirdefs/load';
 import { FHIRDefinitions } from '../../src/fhirdefs';
@@ -463,6 +464,108 @@ describe('Processing', () => {
     });
   });
 
+  describe('#checkNullValuesOnArray', () => {
+    beforeEach(() => {
+      loggerSpy.reset();
+    });
+
+    it('should log a warning when a resource includes null values in a top-level array', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      exInstance.name = [null, 'John', null, 'Doe', null];
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(1);
+      expect(loggerSpy.getLastMessage('warn')).toEqual(
+        "The array 'name' in example-instance is missing values at the following indices: 0,2,4"
+      );
+    });
+
+    it('should log a warning when a resource includes null values in a nested array', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      const nameObj = {
+        testNames: [null, 'John', null, 'Doe', null]
+      };
+      exInstance.name = nameObj;
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(1);
+      expect(loggerSpy.getLastMessage('warn')).toEqual(
+        "The array 'name.testNames' in example-instance is missing values at the following indices: 0,2,4"
+      );
+    });
+
+    it('should log a warning when a resource includes null values in an array, nested within both objects and arrays ', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      const nameObj = {
+        foo: [
+          {
+            coding: [
+              null, // this is bad
+              { system: 'http://foo.org', code: 'bar' }
+            ]
+          }
+        ]
+      };
+      exInstance.name = nameObj;
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(1);
+      expect(loggerSpy.getLastMessage('warn')).toEqual(
+        "The array 'name.foo[0].coding' in example-instance is missing values at the following indices: 0"
+      );
+    });
+
+    it('should ignore null values on primitive arrays, but warn on null values in nested arrays', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      const nameObj = {
+        _foo: [
+          null, // this is ok
+          {
+            extension: [
+              null, // this is NOT ok
+              { url: 'http://foo.org', valueBoolean: true }
+            ]
+          }
+        ]
+      };
+      exInstance.name = nameObj;
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(1);
+      expect(loggerSpy.getLastMessage('warn')).toEqual(
+        "The array 'name._foo[1].extension' in example-instance is missing values at the following indices: 0"
+      );
+    });
+
+    it('should not log a warning when a resource includes null values in an array prefixed with an underscore', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      exInstance._name = [null, 'John', null, 'Doe', null];
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(0);
+    });
+
+    it('should log a warning when a resource includes null values in an array nested within an object prefixed with an underscore', () => {
+      const exInstance = new InstanceDefinition();
+      exInstance.resourceType = 'Profile';
+      exInstance.id = 'example-instance';
+      const nameObj = {
+        testNames: [null, 'John', null, 'Doe', null]
+      };
+      exInstance._name = nameObj;
+      checkNullValuesOnArray(exInstance);
+      expect(loggerSpy.getAllMessages()).toHaveLength(1);
+      expect(loggerSpy.getLastMessage('warn')).toEqual(
+        "The array '_name.testNames' in example-instance is missing values at the following indices: 0,2,4"
+      );
+    });
+  });
+
   describe('#writeFHIRResources()', () => {
     let tempRoot: string;
     let tempIGPubRoot: string;