Repository: FHIR/sushi
Pull request #894 (2021-07-27 20:18:34)

Support sushi-ignoreWarnings.txt file to ignore warnings
This addresses [CIMPL-787](https://standardhealthrecord.atlassian.net/browse/CIMPL-787) by adding support for a `sushi-ignoreWarnings.txt` file which can list warnings to be ignored either by directly listing the warning, or by listing a regex that matches the desired warning.

The file must be named `sushi-ignoreWarnings.txt` and must be in the root directory that SUSHI is working on. Only warnings can be ignored, and each line of the file should either list the full content of a warning to be ignored, for example:
```
Instance I2 is not an instance of a resource, so it should only be used inline on other instances, and it will not be exported to a standalone file. Specify "Usage: #inline" to remove this warning.
```
or list a regex that will be used to match certain messages:
```
/Detected the following non-conformant Resource definitions.*/
```

The only way to ignore a multi-line warning is with a regex.

To test this, I would recommend using a FSH file that will generate some warnings. Here is the sample I was using: https://fshschool.org/FSHOnline/#/share/3flTq4D. In `sushi-beta.2` that should generate 3 warnings, one of which is split over multiple lines. Then test out different ways of ignoring those messages, and ensure it aligns with your expectations.

This will need to be documented on FSHSchool, I'll make a corresponding PR there and link it here when I'm done with it, but first I'm going to let people do an initial review in case anything major changes.

ngfreiter: Thanks for the comments @cmoesel, they make things much cleaner. Should be ready for re-review.
cmoesel: @ngfreiter -- In FHIR's `ignoreWarnings.txt`, you can use `#` to mark comments explaining why you're ignoring something.  Would we want to support something like that?  For example:
```
# ignore everything because I'm lazy
/.*/
```

This technically works already, because we just treat the first line like it is a message to be ignored -- and it just doesn't match any messages.  But it seems like if we do want this as a feature, maybe it would be better to filter out lines starting w/ `#` when we're collecting the ignored warnings.  What do you think?
ngfreiter: > @ngfreiter -- In FHIR's `ignoreWarnings.txt`, you can use `#` to mark comments explaining why you're ignoring something. Would we want to support something like that?

I knew I was forgetting something. Added the comments in https://github.com/FHIR/sushi/pull/894/commits/bd20a93d2333fdbb0a25767e6b0c1d51cfdfed6b.


cmoesel: We could be flexible and allow for it in either place.  If it's in both places, we could emit a warning and then either choose one (root) or concatenate them together so it ignores anything in either file.
jafeltra: > We could be flexible and allow for it in either place. If it's in both places, we could emit a warning and then either choose one (root) or concatenate them together so it ignores anything in either file.

But can you ignore that warning? 😆 

More seriously, I did think about that option, but I do almost feel a little nervous about supporting two ways to do something after the ig-data/folder structure fun. But this is fairly limited and would be helpful probably so I'm not opposed.
ngfreiter: > This functionally looks really good! I just had one question about the location of the file. I think it makes sense to keep in the root folder since that's where the sushi config file is, but I did initially get confused and wanted to put it next to the publisher's ignoreWarnings files inside of `input`. Is there any benefit to putting our file in the same location as the publisher's is expected to be? Semantically, I think that it makes more sense to keep ours where it is, but I wanted to run this same thought process past you.

Updated it so that the file can be placed in both locations in https://github.com/FHIR/sushi/pull/894/commits/d185d0affc1074490ff41e99e007f40b4cc2f989. If it is in only one location, that file is used. If the file is in the `input` folder and the root, only the one in the root is used, and a warning is logged. And yes, you can ignore the warning about `ignoreWarnings` 🙄 .
ngfreiter: If you review this PR, please also give the documentation a look: https://github.com/FSHSchool/site/pull/45

Patch:
diff --git a/src/app.ts b/src/app.ts
index c185618..1c07f90 100644
--- a/src/app.ts
+++ b/src/app.ts
@@ -27,7 +27,8 @@ import {
   writePreprocessedFSH,
   getRawFSHes,
   init,
-  getRandomPun
+  getRandomPun,
+  setIgnoredWarnings
 } from './utils';
 
 const FSH_VERSION = '1.1.0';
@@ -70,6 +71,23 @@ async function app() {
   if (program.debug) logger.level = 'debug';
   input = ensureInputDir(input);
 
+  const rootIgnoreWarningsPath = path.join(input, 'sushi-ignoreWarnings.txt');
+  const nestedIgnoreWarningsPath = path.join(input, 'input', 'sushi-ignoreWarnings.txt');
+  if (fs.existsSync(rootIgnoreWarningsPath)) {
+    setIgnoredWarnings(fs.readFileSync(rootIgnoreWarningsPath, 'utf-8'));
+    if (fs.existsSync(nestedIgnoreWarningsPath)) {
+      logger.warn(
+        'Found sushi-ignoreWarnings.txt files in the following locations:\n\n' +
+          ` - ${rootIgnoreWarningsPath}\n` +
+          ` - ${nestedIgnoreWarningsPath}\n\n` +
+          `Only the file at ${rootIgnoreWarningsPath} will be processed. ` +
+          'Remove one of these files to avoid this warning.'
+      );
+    }
+  } else if (fs.existsSync(nestedIgnoreWarningsPath)) {
+    setIgnoredWarnings(fs.readFileSync(nestedIgnoreWarningsPath, 'utf-8'));
+  }
+
   logger.info(`Running ${getVersion()}`);
 
   logger.info('Arguments:');
diff --git a/src/utils/FSHLogger.ts b/src/utils/FSHLogger.ts
index 2e953a3..3b48d7a 100644
--- a/src/utils/FSHLogger.ts
+++ b/src/utils/FSHLogger.ts
@@ -30,6 +30,17 @@ const withLocation = format(info => {
   return info;
 });
 
+const ignoreWarnings = format(info => {
+  // Only warnings can be ignored
+  if (info.level !== 'warn') {
+    return info;
+  }
+  const shouldIgnore = ignoredWarnings?.some(m => {
+    return typeof m === 'string' ? m === info.message : m.test(info.message);
+  });
+  return shouldIgnore ? false : info;
+});
+
 const incrementCounts = format(info => {
   switch (info.level) {
     case 'info':
@@ -94,10 +105,31 @@ const printer = printf(info => {
 });
 
 export const logger = createLogger({
-  format: combine(incrementCounts(), trackErrorsAndWarnings(), withLocation(), printer),
+  format: combine(
+    ignoreWarnings(),
+    incrementCounts(),
+    trackErrorsAndWarnings(),
+    withLocation(),
+    printer
+  ),
   transports: [new transports.Console()]
 });
 
+let ignoredWarnings: (string | RegExp)[];
+export const setIgnoredWarnings = (messages: string): void => {
+  ignoredWarnings = messages
+    .split(/\r?\n/)
+    .map(m => m.trim())
+    .filter(m => !m.startsWith('#'))
+    .map(m => {
+      if (m.startsWith('/') && m.endsWith('/')) {
+        return new RegExp(m.slice(1, -1));
+      } else {
+        return m;
+      }
+    });
+};
+
 class LoggerStats {
   public numInfo = 0;
   public numWarn = 0;


Test patch:
diff --git a/test/utils/FSHLogger.test.ts b/test/utils/FSHLogger.test.ts
index ab9a3a5..beb8786 100644
--- a/test/utils/FSHLogger.test.ts
+++ b/test/utils/FSHLogger.test.ts
@@ -1,5 +1,5 @@
-import { logger, stats, errorsAndWarnings } from '../../src/utils/FSHLogger';
-
+import { logger, stats, errorsAndWarnings, setIgnoredWarnings } from '../../src/utils/FSHLogger';
+import { leftAlign } from './leftAlign';
 // MUTE_LOGS controls whether or not logs get printed during testing.
 // Usually, we don't want logs actually printed, as they cause clutter.
 const MUTE_LOGS = true;
@@ -115,4 +115,154 @@ describe('FSHLogger', () => {
     expect(errorsAndWarnings.errors).toHaveLength(0);
     expect(errorsAndWarnings.warnings).toHaveLength(0);
   });
+
+  describe('#ignoreWarnings', () => {
+    let messages: string[] = [];
+    let logMock: any;
+    beforeAll(() => {
+      const m = Symbol.for('message');
+      logMock = jest.fn();
+      logMock.mockImplementation((log: any) => {
+        if (log[m]) {
+          messages.push(log[m]);
+        }
+      });
+    });
+
+    afterEach(() => {
+      messages = [];
+    });
+
+    it('should ignore warnings which are listed directly', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        warn1
+        `)
+      );
+      logger.warn('warn1');
+      logger.error('error1');
+      expect(messages).toHaveLength(1);
+      expect(messages[0]).toMatch(/error.*error1/);
+    });
+
+    it('should ignore multiple warnings which are listed directly', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        warn1
+        warn2
+        `)
+      );
+      logger.warn('warn1');
+      logger.warn('warn2');
+      logger.error('error1');
+      expect(messages).toHaveLength(1);
+      expect(messages[0]).toMatch(/error.*error1/);
+    });
+
+    it('should ignore lines beginning with # when listing warnings', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        warn1
+        # warn2
+        `)
+      );
+      logger.warn('warn1');
+      logger.warn('warn2');
+      logger.error('error1');
+      expect(messages).toHaveLength(2);
+      expect(messages[0]).toMatch(/warn.*warn2/);
+      expect(messages[1]).toMatch(/error.*error1/);
+    });
+
+    it('should ignore leading and trailing whitespace for warnings which are listed directly', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(' warn1  ');
+      logger.warn('warn1');
+      logger.error('error1');
+      expect(messages).toHaveLength(1);
+      expect(messages[0]).toMatch(/error.*error1/);
+    });
+
+    it('should not ignore warnings which are unlisted', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        warn1
+        `)
+      );
+      logger.warn('warn1');
+      logger.warn('warn2');
+      logger.error('error1');
+      expect(messages).toHaveLength(2);
+      expect(messages[0]).toMatch(/warn.*warn2/);
+      expect(messages[1]).toMatch(/error.*error1/);
+    });
+
+    it('should not ignore messages which are not warnings, even when listed', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        foo
+        `)
+      );
+      logger.error('foo');
+      logger.info('foo');
+      logger.debug('foo');
+      expect(messages).toHaveLength(3);
+      expect(messages[0]).toMatch(/error.*foo/);
+      expect(messages[1]).toMatch(/info.*foo/);
+      expect(messages[2]).toMatch(/debug.*foo/);
+    });
+
+    it('should ignore warnings which are matched via regex', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        /ignore.*/
+        `)
+      );
+      logger.warn('ignore this message');
+      logger.warn(`ignore this message even though
+      it goes over
+      multiple lines`);
+      expect(messages).toHaveLength(0);
+    });
+
+    it('should ignore warnings which are matched via regex with leading or trailing whitespace', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(' /ignore.*/ ');
+      logger.warn('ignore this message');
+      logger.warn(`ignore this message even though
+      it goes over
+      multiple lines`);
+      expect(messages).toHaveLength(0);
+    });
+
+    it('should ignore warnings which are matched via regex and listed directly', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        /ignore.*/
+        this too
+        `)
+      );
+      logger.warn('ignore this message');
+      logger.warn('this too');
+      expect(messages).toHaveLength(0);
+    });
+
+    it('should not ignore warnings which do not match the regex', () => {
+      logger.transports[0]['write'] = logMock;
+      setIgnoredWarnings(
+        leftAlign(`
+        /ignore.*/
+        `)
+      );
+      logger.warn('please log this');
+      expect(messages).toHaveLength(1);
+    });
+  });
 });