Repository: FHIR/sushi (ts)

Issue:
SUSHI should always put hl7.fhir.uv.tools in scope
According to Grahame Grieve, the IG Tools package is considered a core part of building IGs now and should be loaded automatically (without needing to declare it as a dependency).

So... SUSHI should also automatically load the IG Tools since authors can use extensions defined by it.  It sounds like SUSHI should always load `current` -- but we may want to revisit how we do that to try to reduce unneeded re-downloads (when nothing has changed).  We may also want to be fairly flexible when it fails to download (for whatever reason), as not every IG actually needs it and we don't want it to become a blocker for people who have firewalls or otherwise cannot do dynamic downloads of dependencies.

Pull request:
Automatically load hl7.fhir.uv.tools dependency
Fixes #1166 and completes task [CIMPL-1036](https://standardhealthrecord.atlassian.net/browse/CIMPL-1036).

The current version of this package will always be loaded, even if it is not configured as a dependency. If it is configured as a dependency, use the configured version. When it fails to load, log a warning instead of an error if it was not a configured dependency.

My strategy for implementing this was to consider loading dependencies in two groups: automatic dependencies and configured dependencies. Hopefully, this makes it straightforward to add more automatic dependencies later. There are changes to a lot of the tests for `loadExternalDependencies`, since there's now an additional package that shows up every time. A lot of those tests are really testing `loadConfiguredDependencies`, though, so it may be worthwhile to adjust the tests accordingly. Please let me know if you think that would be useful.

Patch:
diff --git a/src/utils/Processing.ts b/src/utils/Processing.ts
index 738d543..d44b082 100644
--- a/src/utils/Processing.ts
+++ b/src/utils/Processing.ts
@@ -18,6 +18,7 @@ import {
 import { Package } from '../export';
 import { Configuration } from '../fshtypes';
 import { axiosGet } from './axiosUtils';
+import { ImplementationGuideDependsOn } from '../fhirtypes';
 
 const EXT_PKG_TO_FHIR_PKG_MAP: { [key: string]: string } = {
   'hl7.fhir.extensions.r2': 'hl7.fhir.r2.core#1.0.2',
@@ -26,6 +27,20 @@ const EXT_PKG_TO_FHIR_PKG_MAP: { [key: string]: string } = {
   'hl7.fhir.extensions.r5': 'hl7.fhir.r5.core#current'
 };
 
+const CERTIFICATE_MESSAGE =
+  '\n\nSometimes this error occurs in corporate or educational environments that use proxies and/or SSL ' +
+  'inspection.\nTroubleshooting tips:\n' +
+  '  1. If a non-proxied network is available, consider connecting to that network instead.\n' +
+  '  2. Set NODE_EXTRA_CA_CERTS as described at https://bit.ly/3ghJqJZ (RECOMMENDED).\n' +
+  '  3. Disable certificate validation as described at https://bit.ly/3syjzm7 (NOT RECOMMENDED).\n';
+
+export const AUTOMATIC_DEPENDENCIES: ImplementationGuideDependsOn[] = [
+  {
+    packageId: 'hl7.fhir.uv.tools',
+    version: 'current'
+  }
+];
+
 export function isSupportedFHIRVersion(version: string): boolean {
   // For now, allow current or any 4.x/5.x version of FHIR except 4.0.0. This is a quick check; not a guarantee.  If a user passes
   // in an invalid version that passes this test (e.g., 4.99.0), it is still expected to fail when we load dependencies.
@@ -189,13 +204,47 @@ export async function loadExternalDependencies(
     );
   }
   dependencies.push({ packageId: fhirPackageId, version: fhirVersion });
+  const automaticPromises = loadAutomaticDependencies(dependencies, defs);
+  const configuredPromises = loadConfiguredDependencies(
+    dependencies,
+    fhirVersion,
+    config.filePath,
+    defs
+  );
+
+  return Promise.all(configuredPromises.concat(automaticPromises)).then(() => {});
+}
+
+export function loadAutomaticDependencies(
+  configuredDependencies: ImplementationGuideDependsOn[],
+  defs: FHIRDefinitions
+): Promise<void | FHIRDefinitions>[] {
+  return AUTOMATIC_DEPENDENCIES.map(dep => {
+    if (configuredDependencies.some(cd => cd.packageId === dep.packageId)) {
+      return Promise.resolve();
+    } else {
+      return loadDependency(dep.packageId, dep.version, defs).catch(e => {
+        let message = `Failed to load automatically-provided ${dep.packageId}#${dep.version}: ${e.message}`;
+        if (/certificate/.test(e.message)) {
+          message += CERTIFICATE_MESSAGE;
+        }
+        logger.warn(message);
+      });
+    }
+  });
+}
 
-  // Load dependencies
-  const promises = dependencies.map(dep => {
+function loadConfiguredDependencies(
+  dependencies: ImplementationGuideDependsOn[],
+  fhirVersion: string,
+  configPath: string,
+  defs: FHIRDefinitions
+): Promise<void | FHIRDefinitions>[] {
+  return dependencies.map(dep => {
     if (dep.version == null) {
       logger.error(
         `Failed to load ${dep.packageId}: No version specified. To specify the version in your ` +
-          `${path.basename(config.filePath)}, either use the simple dependency format:\n\n` +
+          `${path.basename(configPath)}, either use the simple dependency format:\n\n` +
           'dependencies:\n' +
           `  ${dep.packageId}: current\n\n` +
           'or use the detailed dependency format to specify other properties as well:\n\n' +
@@ -224,19 +273,12 @@ export async function loadExternalDependencies(
       return loadDependency(dep.packageId, dep.version, defs).catch(e => {
         let message = `Failed to load ${dep.packageId}#${dep.version}: ${e.message}`;
         if (/certificate/.test(e.message)) {
-          message +=
-            '\n\nSometimes this error occurs in corporate or educational environments that use proxies and/or SSL ' +
-            'inspection.\nTroubleshooting tips:\n' +
-            '  1. If a non-proxied network is available, consider connecting to that network instead.\n' +
-            '  2. Set NODE_EXTRA_CA_CERTS as described at https://bit.ly/3ghJqJZ (RECOMMENDED).\n' +
-            '  3. Disable certificate validation as described at https://bit.ly/3syjzm7 (NOT RECOMMENDED).\n';
+          message += CERTIFICATE_MESSAGE;
         }
         logger.error(message);
       });
     }
   });
-
-  return Promise.all(promises).then(() => {});
 }
 
 export function getRawFSHes(input: string): RawFSH[] {


Test patch:
diff --git a/test/testhelpers/asserts.ts b/test/testhelpers/asserts.ts
index 68fd1f6..7b0301d 100644
--- a/test/testhelpers/asserts.ts
+++ b/test/testhelpers/asserts.ts
@@ -22,6 +22,7 @@ import {
 } from '../../src/fshtypes/rules';
 import { FshCode, ValueSetFilter } from '../../src/fshtypes';
 import { splitOnPathPeriods } from '../../src/fhirtypes/common';
+import { AUTOMATIC_DEPENDENCIES } from '../../src/utils';
 
 export function assertCardRule(rule: Rule, path: string, min: number, max: number | string): void {
   expect(rule).toBeInstanceOf(CardRule);
@@ -288,3 +289,9 @@ export function assertConceptRule(
     expect(conceptRule.hierarchy).toEqual(hierarchy);
   }
 }
+
+export function assertAutomaticDependencies(packages: string[]) {
+  AUTOMATIC_DEPENDENCIES.forEach(dep => {
+    expect(packages).toContain(`${dep.packageId}#${dep.version}`);
+  });
+}
diff --git a/test/utils/Processing.test.ts b/test/utils/Processing.test.ts
index 2584b50..6df02c4 100644
--- a/test/utils/Processing.test.ts
+++ b/test/utils/Processing.test.ts
@@ -5,14 +5,17 @@ import path from 'path';
 import temp from 'temp';
 import { minimalConfig } from './minimalConfig';
 import { loggerSpy } from '../testhelpers/loggerSpy';
+import { assertAutomaticDependencies } from '../testhelpers/asserts';
 import readlineSync from 'readline-sync';
 import {
+  AUTOMATIC_DEPENDENCIES,
   isSupportedFHIRVersion,
   ensureInputDir,
   findInputDir,
   ensureOutputDir,
   readConfig,
   loadExternalDependencies,
+  loadAutomaticDependencies,
   getRawFSHes,
   hasFshFiles,
   writeFHIRResources,
@@ -328,7 +331,11 @@ describe('Processing', () => {
         .mockImplementation(
           async (packageName: string, version: string, FHIRDefs: FHIRDefinitions) => {
             // the mock loader can find hl7.fhir.(r2|r3|r4|r5|us).core
-            if (/^hl7.fhir.(r2|r3|r4|r4b|r5|us).core$/.test(packageName)) {
+            // it can also find automatic dependencies
+            if (
+              /^hl7.fhir.(r2|r3|r4|r4b|r5|us).core$/.test(packageName) ||
+              AUTOMATIC_DEPENDENCIES.some(dep => dep.packageId === packageName)
+            ) {
               FHIRDefs.packages.push(`${packageName}#${version}`);
               return Promise.resolve(FHIRDefs);
             } else if (/^self-signed.package$/.test(packageName)) {
@@ -348,9 +355,10 @@ describe('Processing', () => {
       usCoreDependencyConfig.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, usCoreDependencyConfig).then(() => {
-        expect(defs.packages.length).toBe(2);
+        expect(defs.packages.length).toBe(2 + AUTOMATIC_DEPENDENCIES.length);
         expect(defs.packages).toContain('hl7.fhir.r4.core#4.0.1');
         expect(defs.packages).toContain('hl7.fhir.us.core#3.1.0');
+        assertAutomaticDependencies(defs.packages);
         expect(loggerSpy.getAllLogs('warn')).toHaveLength(0);
       });
     });
@@ -360,7 +368,8 @@ describe('Processing', () => {
       config.fhirVersion = ['4.1.0'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r4b.core#4.1.0']);
+        expect(defs.packages).toContain('hl7.fhir.r4b.core#4.1.0');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /support for pre-release versions of FHIR is experimental/s
         );
@@ -372,7 +381,8 @@ describe('Processing', () => {
       config.fhirVersion = ['4.3.0-snapshot1'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r4b.core#4.3.0-snapshot1']);
+        expect(defs.packages).toContain('hl7.fhir.r4b.core#4.3.0-snapshot1');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /support for pre-release versions of FHIR is experimental/s
         );
@@ -384,7 +394,8 @@ describe('Processing', () => {
       config.fhirVersion = ['4.3.0'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r4b.core#4.3.0']);
+        expect(defs.packages).toContain('hl7.fhir.r4b.core#4.3.0');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getAllLogs('warn')).toHaveLength(0);
       });
     });
@@ -394,7 +405,8 @@ describe('Processing', () => {
       config.fhirVersion = ['4.5.0'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r5.core#4.5.0']);
+        expect(defs.packages).toContain('hl7.fhir.r5.core#4.5.0');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /support for pre-release versions of FHIR is experimental/s
         );
@@ -406,7 +418,8 @@ describe('Processing', () => {
       config.fhirVersion = ['5.0.0-snapshot1'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r5.core#5.0.0-snapshot1']);
+        expect(defs.packages).toContain('hl7.fhir.r5.core#5.0.0-snapshot1');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /support for pre-release versions of FHIR is experimental/s
         );
@@ -418,7 +431,8 @@ describe('Processing', () => {
       config.fhirVersion = ['5.0.0'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r5.core#5.0.0']);
+        expect(defs.packages).toContain('hl7.fhir.r5.core#5.0.0');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getAllLogs('warn')).toHaveLength(0);
       });
     });
@@ -428,7 +442,8 @@ describe('Processing', () => {
       config.fhirVersion = ['current'];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, config).then(() => {
-        expect(defs.packages).toEqual(['hl7.fhir.r5.core#current']);
+        expect(defs.packages).toContain('hl7.fhir.r5.core#current');
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /support for pre-release versions of FHIR is experimental/s
         );
@@ -449,8 +464,9 @@ describe('Processing', () => {
         virtualExtensionsConfig.dependencies = [{ packageId: extId, version: fhirVersion }];
         const defs = new FHIRDefinitions();
         return loadExternalDependencies(defs, virtualExtensionsConfig).then(() => {
-          expect(defs.packages.length).toBe(1);
+          expect(defs.packages.length).toBe(1 + AUTOMATIC_DEPENDENCIES.length);
           expect(defs.packages).toContain(`${fhirId}#${fhirVersion}`);
+          assertAutomaticDependencies(defs.packages);
           expect(defs.supplementalFHIRPackages).toEqual([`${suppFhirId}#${suppFhirVersion}`]);
           expect(loggerSpy.getAllLogs('error')).toHaveLength(0);
         });
@@ -495,8 +511,9 @@ describe('Processing', () => {
       ];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, virtualExtensionsConfig).then(() => {
-        expect(defs.packages.length).toBe(1);
+        expect(defs.packages.length).toBe(1 + AUTOMATIC_DEPENDENCIES.length);
         expect(defs.packages).toContain('hl7.fhir.r4.core#4.0.1');
+        assertAutomaticDependencies(defs.packages);
         expect(defs.supplementalFHIRPackages).toEqual(['hl7.fhir.r2.core#1.0.2']);
         expect(loggerSpy.getLastMessage('warn')).toMatch(
           /Incorrect package version: hl7\.fhir\.extensions\.r2#1\.0\.2\./
@@ -510,8 +527,9 @@ describe('Processing', () => {
       badDependencyConfig.dependencies = [{ packageId: 'hl7.does.not.exist', version: 'current' }];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, badDependencyConfig).then(() => {
-        expect(defs.packages.length).toBe(1);
+        expect(defs.packages.length).toBe(1 + AUTOMATIC_DEPENDENCIES.length);
         expect(defs.packages).toContain('hl7.fhir.r4.core#4.0.1');
+        assertAutomaticDependencies(defs.packages);
         expect(loggerSpy.getLastMessage('error')).toMatch(
           /Failed to load hl7\.does\.not\.exist#current/s
         );
@@ -527,8 +545,9 @@ describe('Processing', () => {
       ];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, selfSignedDependencyConfig).then(() => {
-        expect(defs.packages.length).toBe(1);
+        expect(defs.packages.length).toBe(1 + AUTOMATIC_DEPENDENCIES.length);
         expect(defs.packages).toContain('hl7.fhir.r4.core#4.0.1');
+        assertAutomaticDependencies(defs.packages);
         expect(loggerSpy.getLastMessage('error')).toMatch(
           /Failed to load self-signed\.package#1\.0\.0/s
         );
@@ -544,8 +563,9 @@ describe('Processing', () => {
       badDependencyConfig.dependencies = [{ packageId: 'hl7.fhir.r4.core' }];
       const defs = new FHIRDefinitions();
       return loadExternalDependencies(defs, badDependencyConfig).then(() => {
-        expect(defs.packages.length).toBe(1);
+        expect(defs.packages.length).toBe(1 + AUTOMATIC_DEPENDENCIES.length);
         expect(defs.packages).toContain('hl7.fhir.r4.core#4.0.1');
+        assertAutomaticDependencies(defs.packages);
         expect(loggerSpy.getLastMessage('error')).toMatch(
           /Failed to load hl7\.fhir\.r4\.core: No version specified\./s
         );
@@ -555,6 +575,98 @@ describe('Processing', () => {
     });
   });
 
+  describe('#loadAutomaticDependencies', () => {
+    let loadDependencySpy: jest.SpyInstance;
+
+    beforeAll(() => {
+      loadDependencySpy = jest.spyOn(loadModule, 'loadDependency');
+    });
+
+    beforeEach(() => {
+      loggerSpy.reset();
+      loadDependencySpy.mockReset();
+    });
+
+    afterAll(() => {
+      loadDependencySpy.mockRestore();
+    });
+
+    it('should load each automatic dependency', () => {
+      loadDependencySpy.mockImplementation(
+        async (packageName: string, version: string, FHIRDefs: FHIRDefinitions) => {
+          FHIRDefs.packages.push(`${packageName}#${version}`);
+          return Promise.resolve(FHIRDefs);
+        }
+      );
+      const config = cloneDeep(minimalConfig);
+      config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
+      const defs = new FHIRDefinitions();
+      return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
+        expect(defs.packages).toHaveLength(1);
+        expect(defs.packages).toContain('hl7.fhir.uv.tools#current');
+        expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+        expect(loadDependencySpy).toHaveBeenCalledTimes(1);
+      });
+    });
+
+    it('should not load dependencies that are present in the config', () => {
+      loadDependencySpy.mockImplementation(
+        async (packageName: string, version: string, FHIRDefs: FHIRDefinitions) => {
+          FHIRDefs.packages.push(`${packageName}#${version}`);
+          return Promise.resolve(FHIRDefs);
+        }
+      );
+      const config = cloneDeep(minimalConfig);
+      config.dependencies = [{ packageId: 'hl7.fhir.uv.tools', version: '2.2.0' }];
+      const defs = new FHIRDefinitions();
+      return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
+        expect(defs.packages).toHaveLength(0);
+        expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+        expect(loadDependencySpy).toHaveBeenCalledTimes(0);
+      });
+    });
+
+    it('should log a warning when it fails to load an automatic dependency', () => {
+      loadDependencySpy.mockImplementation(async (packageName: string, version: string) => {
+        throw new PackageLoadError(`${packageName}#${version}`);
+      });
+      const config = cloneDeep(minimalConfig);
+      config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
+      const defs = new FHIRDefinitions();
+      return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
+        expect(defs.packages).toHaveLength(0);
+        expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
+        expect(loadDependencySpy).toHaveBeenCalledTimes(1);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /Failed to load automatically-provided hl7\.fhir\.uv\.tools#current/s
+        );
+        // But don't log the warning w/ details about proxies
+        expect(loggerSpy.getLastMessage('warn')).not.toMatch(/SSL/s);
+      });
+    });
+
+    it('should log a more detailed warning when it fails to load an automatic dependency due to certificate issue', () => {
+      loadDependencySpy.mockImplementation(async () => {
+        throw new Error('self signed certificate in certificate chain');
+      });
+      const config = cloneDeep(minimalConfig);
+      config.dependencies = [{ packageId: 'hl7.fhir.us.core', version: '3.1.0' }];
+      const defs = new FHIRDefinitions();
+      return Promise.all(loadAutomaticDependencies(config.dependencies, defs)).then(() => {
+        expect(defs.packages).toHaveLength(0);
+        expect(loggerSpy.getAllMessages('warn')).toHaveLength(1);
+        expect(loadDependencySpy).toHaveBeenCalledTimes(1);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /Failed to load automatically-provided hl7\.fhir\.uv\.tools#current/s
+        );
+        // AND it should log the detailed message about SSL
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /Sometimes this error occurs in corporate or educational environments that use proxies and\/or SSL inspection/s
+        );
+      });
+    });
+  });
+
   describe('#getRawFSHes()', () => {
     it('should return a RawFSH for each file in the input directory that ends with .fsh', () => {
       const input = path.join(__dirname, 'fixtures', 'fsh-files');