Repository: FHIR/sushi (ts)

Issue:
Filter operator "in" can have a code value
As described here:
https://www.hl7.org/fhir/snomedct.html#4.3.1.0.8.2
the filter operator can take a concept as a value, not just a string.

https://github.com/FHIR/sushi/blob/d785865cf9d84e89316b0abec9ff2c9671fd5871/src/import/FSHImporter.ts#L1853



Pull request description:
Support code values in =, in, and not-in filters
This PR fixes #812 by adding support for code values in the `=`, `in`, and `not-in` filter operators. Based on the [documentation](http://hl7.org/fhir/R4/valueset-definitions.html#ValueSet.compose.include.filter.value) and discussion, we believe these filter operators should _only_ accept code values. We will continue support string values for backwards compatibility, but we will officially deprecate support for strings and log a warning. There are multiple projects, including mCODE, that use string values for these filters which would break if we removed string values immediately. The `is-a`, `is-not-a`, `descendent-of` and `generalizes` remain supporting only code values since we believe that is the correct interpretation of the documentation.

Patch:
diff --git a/src/import/FSHImporter.ts b/src/import/FSHImporter.ts
index e53d69e..dfe3bcd 100644
--- a/src/import/FSHImporter.ts
+++ b/src/import/FSHImporter.ts
@@ -1933,8 +1933,19 @@ export class FSHImporter extends FSHVisitor {
       case VsOperator.EQUALS:
       case VsOperator.IN:
       case VsOperator.NOT_IN:
-        if (typeof value !== 'string') {
-          throw new ValueSetFilterValueTypeError(operator, 'string');
+        // NOTE: We believe that =, in, and not-in operators should ONLY support code values
+        // based on the filter value documentation:
+        // http://hl7.org/fhir/R4/valueset-definitions.html#ValueSet.compose.include.filter.value
+        // Both string and code are supported for now in order to maintain backwards compatibility.
+        if (typeof value === 'string') {
+          logger.warn(
+            `The match value of filter operator "${operator}" must be a code. ` +
+              'For string valued properties, use the regex filter operator with a regular expression value. ' +
+              'Support for using strings as filter values has been deprecated and will be removed in a future release.'
+          );
+        }
+        if (typeof value !== 'string' && !(value instanceof FshCode)) {
+          throw new ValueSetFilterValueTypeError(operator, 'code');
         }
         break;
       case VsOperator.IS_A:


Test patch:
diff --git a/test/import/FSHImporter.ValueSet.test.ts b/test/import/FSHImporter.ValueSet.test.ts
index 3463ef2..ed08261 100644
--- a/test/import/FSHImporter.ValueSet.test.ts
+++ b/test/import/FSHImporter.ValueSet.test.ts
@@ -516,7 +516,7 @@ describe('FSHImporter', () => {
         expect(loggerSpy.getLastMessage('error')).toMatch(/File: Deprecated\.fsh.*Line: 3\D*/s);
       });
 
-      it('should parse a value set that uses filter operator =', () => {
+      it('should parse a value set that uses filter operator = with string value', () => {
         const input = `
         ValueSet: ZooVS
         * codes from system ZOO where version = "2.0"
@@ -532,6 +532,27 @@ describe('FSHImporter', () => {
             value: '2.0'
           }
         ]);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /match value of filter operator "=" must be a code/
+        );
+      });
+
+      it('should parse a value set that uses filter operator = with code value', () => {
+        const input = `
+        ValueSet: ZooVS
+        * codes from system ZOO where version = #two
+        `;
+        const result = importSingleText(input, 'Zoo.fsh');
+        expect(result.valueSets.size).toBe(1);
+        const valueSet = result.valueSets.get('ZooVS');
+        expect(valueSet.rules.length).toBe(1);
+        assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, [
+          {
+            property: 'version',
+            operator: VsOperator.EQUALS,
+            value: new FshCode('two').withLocation([3, 49, 3, 52]).withFile('Zoo.fsh')
+          }
+        ]);
       });
 
       it('should parse a value set that uses filter operator with numeric property name', () => {
@@ -552,7 +573,7 @@ describe('FSHImporter', () => {
         ]);
       });
 
-      it('should log an error when the = filter has a non-string value', () => {
+      it('should log an error when the = filter has a non-string and non-code value', () => {
         const input = `
         ValueSet: ZooVS
         * codes from system ZOO where version = /[1-9].*/
@@ -562,7 +583,7 @@ describe('FSHImporter', () => {
         const valueSet = result.valueSets.get('ZooVS');
         expect(valueSet.rules.length).toBe(1);
         assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, []);
-        expect(loggerSpy.getLastMessage('error')).toMatch(/"=".*string/);
+        expect(loggerSpy.getLastMessage('error')).toMatch(/"=".*code/);
         expect(loggerSpy.getLastMessage('error')).toMatch(/File: Zoo\.fsh.*Line: 3\D*/s);
       });
 
@@ -720,7 +741,7 @@ describe('FSHImporter', () => {
         expect(loggerSpy.getLastMessage('error')).toMatch(/File: MostlyDogs\.fsh.*Line: 3\D*/s);
       });
 
-      it('should parse a value set that uses filter operator in', () => {
+      it('should parse a value set that uses filter operator in with a string value', () => {
         const input = `
         ValueSet: CatAndDogVS
         * codes from system ZOO where concept in "#cat, #dog"
@@ -736,9 +757,12 @@ describe('FSHImporter', () => {
             value: '#cat, #dog'
           }
         ]);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /match value of filter operator "in" must be a code/
+        );
       });
 
-      it('should log an error when the in filter has a non-string value', () => {
+      it('should parse a value set that use filter operator in with a code value', () => {
         const input = `
         ValueSet: CatAndDogVS
         * codes from system ZOO where concept in ZOO#cat
@@ -747,12 +771,30 @@ describe('FSHImporter', () => {
         expect(result.valueSets.size).toBe(1);
         const valueSet = result.valueSets.get('CatAndDogVS');
         expect(valueSet.rules.length).toBe(1);
+        assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, [
+          {
+            property: 'concept',
+            operator: VsOperator.IN,
+            value: new FshCode('cat', 'ZOO').withLocation([3, 50, 3, 56]).withFile('CatDog.fsh')
+          }
+        ]);
+      });
+
+      it('should log an error when the in filter has a non-string or non-code value', () => {
+        const input = `
+        ValueSet: CatAndDogVS
+        * codes from system ZOO where concept in true
+        `;
+        const result = importSingleText(input, 'CatDog.fsh');
+        expect(result.valueSets.size).toBe(1);
+        const valueSet = result.valueSets.get('CatAndDogVS');
+        expect(valueSet.rules.length).toBe(1);
         assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, []);
-        expect(loggerSpy.getLastMessage('error')).toMatch(/"in".*string/);
+        expect(loggerSpy.getLastMessage('error')).toMatch(/"in".*code/);
         expect(loggerSpy.getLastMessage('error')).toMatch(/File: CatDog\.fsh.*Line: 3\D*/s);
       });
 
-      it('should parse a value set that uses filter operator not-in', () => {
+      it('should parse a value set that uses filter operator not-in with a string value', () => {
         const input = `
         ValueSet: NoGooseVS
         * codes from system ZOO where concept not-in "#goose"
@@ -768,9 +810,30 @@ describe('FSHImporter', () => {
             value: '#goose'
           }
         ]);
+        expect(loggerSpy.getLastMessage('warn')).toMatch(
+          /match value of filter operator "not-in" must be a code/
+        );
+      });
+
+      it('should parse a value set that uses filter operator not-in with a code value', () => {
+        const input = `
+        ValueSet: NoGooseVS
+        * codes from system ZOO where concept not-in #goose
+        `;
+        const result = importSingleText(input, 'NoGoose.fsh');
+        expect(result.valueSets.size).toBe(1);
+        const valueSet = result.valueSets.get('NoGooseVS');
+        expect(valueSet.rules.length).toBe(1);
+        assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, [
+          {
+            property: 'concept',
+            operator: VsOperator.NOT_IN,
+            value: new FshCode('goose').withLocation([3, 54, 3, 59]).withFile('NoGoose.fsh')
+          }
+        ]);
       });
 
-      it('should log an error when the not-in filter has a non-string value', () => {
+      it('should log an error when the not-in filter has a non-string and non-code value', () => {
         const input = `
         ValueSet: NoGooseVS
         * codes from system ZOO where concept not-in /duck|goose/
@@ -780,7 +843,7 @@ describe('FSHImporter', () => {
         const valueSet = result.valueSets.get('NoGooseVS');
         expect(valueSet.rules.length).toBe(1);
         assertValueSetFilterComponent(valueSet.rules[0], 'ZOO', undefined, []);
-        expect(loggerSpy.getLastMessage('error')).toMatch(/"not-in".*string/);
+        expect(loggerSpy.getLastMessage('error')).toMatch(/"not-in".*code/);
         expect(loggerSpy.getLastMessage('error')).toMatch(/File: NoGoose\.fsh.*Line: 3\D*/s);
       });