Repository: FHIR/sushi
Pull request #859 (2021-06-29 01:01:04)

Warn on Custom Resource definition and use
This fixes https://github.com/FHIR/sushi/issues/842 by adding warnings on definition and use of custom `Resources`.

Whenever anyone defines a custom resource using the `Resource` keyword which is not in the `http://hl7.org/fhir/StructureDefinition` domain, they will get a warning telling them a lot of scary things about how doing that is non-conformant to FHIR and a lot of stuff can be expected to break. The warning will only be logged once, and list all defined custom resources.

Whenever anyone uses a custom resource which is not in the `http://hl7.org/fhir/StructureDefinition` domain via a `Reference`, either on a `Profile`, `Extension`, or `Instance`, they will get a warning _if_ their `Profile`, `Extension`, `Instance` is derived from core FHIR. So if you make a `Profile` or your own custom resource, and that resource references another custom resource, then no warning will be logged. For this case, there is one warning for every usage of a resource.

These warnings can be disabled with the `suppressWarnings` configuration property, which takes an array of strings which represent types of warnings that can be suppressed. For example:
```
suppressWarnings:
  - "nonConformantResources"
```
is used to suppress the messages added in this PR.

I'm not really sure about the logging of messages upon use of a custom resource. I guess if we didn't do this, someone could pull a custom resource in as a dependency (probably not via a package but just by pasting the resource into `input`) and then use that resource in their FSH without being warned. But I feel like 99% of the time, the warning upon definition of the custom resource will be sufficient to alert users to the issue. So don't feel bad if in your review you think that should be taken out, because I'm probably on your side.

Here are some cases you will want to verify:
* Warning when defining `Resource` with non-HL7 URL
* No warning when defining `Resource` with `http://hl7.org/fhir/StructureDefinition` URL
* Warning when referencing non-HL7 custom resource on `Profile` derived from FHIR
* No warning when referencing HL7 custom resource on `Profile` derived from FHIR
* No warning when referencing non-HL7 custom resource on `Profile` derived from custom resource
* Warning when referencing non-HL7 custom resource on `Instance` derived from FHIR
* No warning when referencing HL7 custom resource on `Instance` derived from FHIR
* No warning when referencing non-HL7 custom resource on `Instance` derived from custom resource
* All of the above warnings go away if the `suppressWarnings` property described above is added to the config file

cmoesel: @mint-thompson - the ability to suppress the message was put on hold until we can think more about a message suppression mechanism in general.  So it is expected not to work.

Patch:
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index 2debc22..348ae7f 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -1,4 +1,4 @@
-import { isEmpty } from 'lodash';
+import { isEmpty, padEnd } from 'lodash';
 import {
   ElementDefinition,
   ElementDefinitionBindingStrength,
@@ -45,6 +45,7 @@ import {
 } from '../fhirtypes/common';
 import { Package } from './Package';
 import { isUri } from 'valid-url';
+import chalk from 'chalk';
 
 // Extensions that should not be inherited by derived profiles
 // See: https://jira.hl7.org/browse/FHIR-27535
@@ -71,6 +72,43 @@ export class StructureDefinitionExporter implements Fishable {
     private readonly fisher: MasterFisher
   ) {}
 
+  /**
+   * Checks generated resources for any custom resources which are not in the
+   * http://hl7.org/fhir/StructureDefinition namespace, and are therefore not
+   * conformant.
+   */
+  private warnOnNonConformantResourceDefinitions(): void {
+    const nonConformantResources = this.pkg.resources.filter(
+      r => !r.url.startsWith('http://hl7.org/fhir/StructureDefinition/')
+    );
+    if (nonConformantResources.length) {
+      // logger.warn color
+      const clr = chalk.rgb(179, 98, 0);
+      const maxLength = 61;
+      const nonConformantResourceLogs = nonConformantResources.map(r => {
+        const printableName =
+          r.name.length > maxLength ? r.name.slice(0, maxLength - 3) + '...' : r.name;
+        return clr('│') + padEnd(` - ${printableName}`, 65) + clr('│');
+      });
+
+      // prettier-ignore
+      const message = [
+        clr('\n╭─────────────────────────────────────────────────────────────────') + '' + clr('╮'),
+          clr('│') + ' Detected the following non-conformant Resource definitions:     ' + clr('│'),
+          ...nonConformantResourceLogs,
+          clr('│') + '                                                                 ' + clr('│'),
+          clr('│') + ' Resources not in the "http://hl7.org/fhir/StructureDefinition"  ' + clr('│'),
+          clr('│') + ' namespace are not conformant to the FHIR specification.         ' + clr('│'),
+          clr('│') + ' These resources are incompatible with standard XML/JSON         ' + clr('│'),
+          clr('│') + ' schemas, public test servers, public registries, standard       ' + clr('│'),
+          clr('│') + ' validation, and standard publication tooling. These resources   ' + clr('│'),
+          clr('│') + ' should not be used for inter-organizational data exchange.      ' + clr('│'),
+          clr('╰─────────────────────────────────────────────────────────────────') + '' + clr('╯\n')
+      ];
+      logger.warn(message.join('\n'));
+    }
+  }
+
   /**
    * Processes the fshDefinition's parent, validating it according to its type.
    * Returns the parent's StructureDefinition as the basis for the new StructureDefinition
@@ -867,6 +905,7 @@ export class StructureDefinitionExporter implements Fishable {
         logger.error(e.message, e.sourceInfo || sd.sourceInfo);
       }
     });
+    this.warnOnNonConformantResourceDefinitions();
     if (structureDefinitions.length > 0) {
       logger.info(`Converted ${structureDefinitions.length} FHIR StructureDefinitions.`);
     }


Test patch:
diff --git a/test/export/StructureDefinition.ResourceExporter.test.ts b/test/export/StructureDefinition.ResourceExporter.test.ts
index 8869461..6142d6d 100644
--- a/test/export/StructureDefinition.ResourceExporter.test.ts
+++ b/test/export/StructureDefinition.ResourceExporter.test.ts
@@ -17,6 +17,7 @@ import {
 describe('ResourceExporter', () => {
   let defs: FHIRDefinitions;
   let doc: FSHDocument;
+  let pkg: Package;
   let exporter: StructureDefinitionExporter;
 
   beforeAll(() => {
@@ -32,7 +33,7 @@ describe('ResourceExporter', () => {
     loggerSpy.reset();
     doc = new FSHDocument('fileName');
     const input = new FSHTank([doc], minimalConfig);
-    const pkg = new Package(input.config);
+    pkg = new Package(input.config);
     const fisher = new TestFisher(input, defs, pkg);
     exporter = new StructureDefinitionExporter(input, pkg, fisher);
   });
@@ -279,4 +280,46 @@ describe('ResourceExporter', () => {
     expect(exported.baseDefinition).toBe('http://hl7.org/fhir/StructureDefinition/DomainResource');
     expect(exported.elements).toHaveLength(12); // 9 AlternateIdentification elements + 3 added elements
   });
+
+  it('should not log a warning when exporting a conformant resource', () => {
+    const resource = new Resource('Foo');
+    const caretRule = new CaretValueRule('');
+    caretRule.caretPath = 'url';
+    caretRule.value = 'http://hl7.org/fhir/StructureDefinition/Foo';
+    resource.rules.push(caretRule);
+    doc.resources.set(resource.name, resource);
+    const exported = exporter.export().resources;
+    expect(exported.length).toBe(1);
+    expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+  });
+
+  it('should log a warning when exporting a non-conformant resource', () => {
+    const resource = new Resource('Foo');
+    doc.resources.set(resource.name, resource);
+    const exported = exporter.export().resources;
+    expect(exported.length).toBe(1);
+    expect(loggerSpy.getLastMessage('warn')).toMatch(/non-conformant Resource.*- Foo/s);
+  });
+
+  it('should log a warning when exporting a multiple non-conformant resources', () => {
+    const resource1 = new Resource('Foo');
+    const resource2 = new Resource('Bar');
+    doc.resources.set(resource1.name, resource1);
+    doc.resources.set(resource2.name, resource2);
+    const exported = exporter.export().resources;
+    expect(exported.length).toBe(2);
+    expect(loggerSpy.getLastMessage('warn')).toMatch(/non-conformant Resource.*- Foo.*- Bar/s);
+  });
+
+  it('should log a warning and truncate the name when exporting a non-conformant resource with a long name', () => {
+    const resource = new Resource(
+      'SupercalifragilisticexpialidociousIsSurprisinglyNotEvenLongEnoughOnItsOwn'
+    );
+    doc.resources.set(resource.name, resource);
+    const exported = exporter.export().resources;
+    expect(exported.length).toBe(1);
+    expect(loggerSpy.getLastMessage('warn')).toMatch(
+      /non-conformant Resource.*- SupercalifragilisticexpialidociousIsSurprisinglyNotEvenLon\.\.\./s
+    );
+  });
 });