Repository: FHIR/sushi
Pull request #1381 (2023-11-07 18:45:26)

Value Set concept components have one code per line
Completes task [CIMPL-1201](https://standardhealthrecord.atlassian.net/browse/CIMPL-1201).

A ValueSetConceptComponentRule may have more than one code. When producing FSH for the rule, each code needs to be written as a separate line of FSH. This is true regardless of whether there are any ValueSets associated with the rule.

mint-thompson: @cmoesel I completely agree that it is weird. This is a case where FSH allows it because FHIR allows it, and SUSHI allows it because FSH allows it. SUSHI requires a system when listing specific concepts, which is aligned with ValueSet invariant `vsd-2`, and at that point, the concept is uniquely identified. So, leaving this kind of thing out of examples in the documentation seems completely reasonable to me: this doesn't feel like it is ever useful or necessary in order to build a perfectly good ValueSet.
jafeltra: This PR was confusing to me when I first looked at it, and maybe this is why. I guess one question I have is, shouldn't GoFSH, and therefore the toFSH, produce FSH that we encourage people to use? But are we possibly losing information if we leave out the "from valueset" parts?
mint-thompson: We definitely want to preserve information. But, leaving out the value sets from the include/exclude concept rule doesn't really cause any loss of information, by my understanding of ValueSet definitions. It would mean that the round trip comparison will show a difference here, though. I'll take some more time to think about this.
cmoesel: Since the overall result is defined as the intersection between the concepts and the value sets, there is a difference _if_ the value set does not have the concept. Again, it would be silly if anyone did that -- but removing the value set is not necessarily harmless.

Patch:
diff --git a/src/fshtypes/rules/ValueSetComponentRule.ts b/src/fshtypes/rules/ValueSetComponentRule.ts
index 6821b2d..247d594 100644
--- a/src/fshtypes/rules/ValueSetComponentRule.ts
+++ b/src/fshtypes/rules/ValueSetComponentRule.ts
@@ -26,18 +26,22 @@ export class ValueSetConceptComponentRule extends ValueSetComponentRule {
   }
 
   toFSH() {
-    // if this rule has valueSets in its "from" definition, write using the typical syntax
-    // otherwise, write each code on its own line, and omit "include" and "from system"
+    // each code needs to be its own line of FSH
     if (this.from.valueSets?.length > 0) {
-      const inclusionPart = `* ${this.inclusion ? 'include' : 'exclude'} `;
-      let conceptPart = this.concepts.map(concept => concept.toString()).join(' and ');
-      let fromPart = fromString(this.from);
-      // if the result is more than 100 characters long, build it again, but with linebreaks
-      if (inclusionPart.length + conceptPart.length + fromPart.length > 100) {
-        conceptPart = this.concepts.map(concept => concept.toString()).join(` and${EOL}    `);
-        fromPart = `${EOL}   ` + fromString(this.from, ` and${EOL}    `);
-      }
-      return `${inclusionPart}${conceptPart}${fromPart}`;
+      // no need for the include keyword when we're just including one code at a time
+      const inclusionPart = `* ${this.inclusion ? '' : 'exclude '}`;
+      const originalFromPart = fromValueSetsString(this.from);
+      return this.concepts
+        .map(concept => {
+          const conceptPart = concept.toString();
+          let fromPart = originalFromPart;
+          // if the result is more than 100 characters long, build it again, but with linebreaks
+          if (inclusionPart.length + conceptPart.length + fromPart.length > 100) {
+            fromPart = `${EOL}   ` + fromValueSetsString(this.from, ` and${EOL}    `);
+          }
+          return `${inclusionPart}${conceptPart}${fromPart}`;
+        })
+        .join(EOL);
     } else {
       const inclusionPart = `* ${this.inclusion ? '' : 'exclude '}`;
       return this.concepts
@@ -104,3 +108,8 @@ function fromString(from: ValueSetComponentFrom, separator = ' and ') {
   }
   return fromString;
 }
+
+function fromValueSetsString(from: ValueSetComponentFrom, separator = ' and ') {
+  if (from.valueSets == null) return '';
+  return ` from valueset ${from.valueSets.join(separator)}`;
+}


Test patch:
diff --git a/test/fshtypes/rules/ValueSetComponentRule.test.ts b/test/fshtypes/rules/ValueSetComponentRule.test.ts
index 1e118f6..cf2d546 100644
--- a/test/fshtypes/rules/ValueSetComponentRule.test.ts
+++ b/test/fshtypes/rules/ValueSetComponentRule.test.ts
@@ -60,7 +60,7 @@ describe('ValueSetConceptComponentRule', () => {
 
     it('should produce FSH for a ValueSetConceptComponentRule with a concept from a system', () => {
       const rule = new ValueSetConceptComponentRule(true);
-      rule.concepts.push(new FshCode('foo'));
+      rule.concepts.push(new FshCode('foo', 'someSystem'));
       rule.from.system = 'someSystem';
 
       expect(rule.toFSH()).toBe('* someSystem#foo');
@@ -68,8 +68,8 @@ describe('ValueSetConceptComponentRule', () => {
 
     it('should produce FSH for a ValueSetConceptComponentRule with several concepts included from a system', () => {
       const rule = new ValueSetConceptComponentRule(true);
-      rule.concepts.push(new FshCode('foo'));
-      rule.concepts.push(new FshCode('bar'));
+      rule.concepts.push(new FshCode('foo', 'someSystem'));
+      rule.concepts.push(new FshCode('bar', 'someSystem'));
       rule.from.system = 'someSystem';
 
       expect(rule.toFSH()).toBe(`* someSystem#foo${EOL}* someSystem#bar`);
@@ -77,8 +77,8 @@ describe('ValueSetConceptComponentRule', () => {
 
     it('should produce FSH for a ValueSetConceptComponentRule with several concepts excluded from a system', () => {
       const rule = new ValueSetConceptComponentRule(false);
-      rule.concepts.push(new FshCode('foo'));
-      rule.concepts.push(new FshCode('bar'));
+      rule.concepts.push(new FshCode('foo', 'someSystem'));
+      rule.concepts.push(new FshCode('bar', 'someSystem'));
       rule.from.system = 'someSystem';
 
       expect(rule.toFSH()).toBe(`* exclude someSystem#foo${EOL}* exclude someSystem#bar`);
@@ -87,14 +87,16 @@ describe('ValueSetConceptComponentRule', () => {
     it('should produce FSH for a ValueSetConceptComponentRule with a concept included from a valueset', () => {
       const rule = new ValueSetConceptComponentRule(true);
       rule.concepts.push(new FshCode('foo', 'bar'));
+      rule.from.system = 'bar';
       rule.from.valueSets = ['someValueSet'];
 
-      expect(rule.toFSH()).toBe('* include bar#foo from valueset someValueSet');
+      expect(rule.toFSH()).toBe('* bar#foo from valueset someValueSet');
     });
 
     it('should produce FSH for a ValueSetConceptComponentRule with a concept excluded from a valueset', () => {
       const rule = new ValueSetConceptComponentRule(false);
       rule.concepts.push(new FshCode('foo', 'bar'));
+      rule.from.system = 'bar';
       rule.from.valueSets = ['someValueSet'];
 
       expect(rule.toFSH()).toBe('* exclude bar#foo from valueset someValueSet');
@@ -103,14 +105,16 @@ describe('ValueSetConceptComponentRule', () => {
     it('should produce FSH for a ValueSetConceptComponentRule with a concept included from several valuesets', () => {
       const rule = new ValueSetConceptComponentRule(true);
       rule.concepts.push(new FshCode('foo', 'bar'));
+      rule.from.system = 'bar';
       rule.from.valueSets = ['someValueSet', 'otherValueSet'];
 
-      expect(rule.toFSH()).toBe('* include bar#foo from valueset someValueSet and otherValueSet');
+      expect(rule.toFSH()).toBe('* bar#foo from valueset someValueSet and otherValueSet');
     });
 
     it('should produce FSH for a ValueSetConceptComponentRule with a concept excluded from several valuesets', () => {
       const rule = new ValueSetConceptComponentRule(false);
       rule.concepts.push(new FshCode('foo', 'bar'));
+      rule.from.system = 'bar';
       rule.from.valueSets = ['someValueSet', 'otherValueSet'];
 
       expect(rule.toFSH()).toBe('* exclude bar#foo from valueset someValueSet and otherValueSet');
@@ -118,47 +122,80 @@ describe('ValueSetConceptComponentRule', () => {
 
     it('should produce FSH for a ValueSetConceptComponentRule with a concept included from a system and several valuesets', () => {
       const rule = new ValueSetConceptComponentRule(true);
-      rule.concepts.push(new FshCode('foo'));
+      rule.concepts.push(new FshCode('foo', 'someSystem'));
       rule.from.system = 'someSystem';
       rule.from.valueSets = ['someValueSet', 'otherValueSet'];
 
-      expect(rule.toFSH()).toBe(
-        '* include #foo from system someSystem and valueset someValueSet and otherValueSet'
-      );
+      expect(rule.toFSH()).toBe('* someSystem#foo from valueset someValueSet and otherValueSet');
     });
 
     it('should produce FSH for a ValueSetConceptComponentRule with a concept excluded from a system and several valuesets', () => {
       const rule = new ValueSetConceptComponentRule(false);
-      rule.concepts.push(new FshCode('foo'));
+      rule.concepts.push(new FshCode('foo', 'someSystem'));
       rule.from.system = 'someSystem';
       rule.from.valueSets = ['someValueSet', 'otherValueSet'];
 
       expect(rule.toFSH()).toBe(
-        '* exclude #foo from system someSystem and valueset someValueSet and otherValueSet'
+        '* exclude someSystem#foo from valueset someValueSet and otherValueSet'
+      );
+    });
+
+    it('should produce FSH for a ValueSetConceptComponentRule with several concepts included from a valueset', () => {
+      const rule = new ValueSetConceptComponentRule(true);
+      rule.concepts.push(new FshCode('bear', 'zoo'));
+      rule.concepts.push(new FshCode('pelican', 'zoo'));
+      rule.from.system = 'zoo';
+      rule.from.valueSets = ['someValueSet'];
+
+      expect(rule.toFSH()).toBe(
+        `* zoo#bear from valueset someValueSet${EOL}* zoo#pelican from valueset someValueSet`
       );
     });
 
     it('should format a long ValueSetConceptComponentRule to take up multiple lines', () => {
       const rule = new ValueSetConceptComponentRule(true);
       rule.concepts = [
-        new FshCode('cookies', undefined, 'Cookies'),
-        new FshCode('candy', undefined, 'Candy'),
-        new FshCode('chips', undefined, 'Chips'),
-        new FshCode('cakes', undefined, 'Cakes'),
-        new FshCode('verylargecakes', undefined, 'Very Large Cakes')
+        new FshCode(
+          'cookies',
+          'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS',
+          'Cookies'
+        ),
+        new FshCode(
+          'candy',
+          'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS',
+          'Candy'
+        ),
+        new FshCode(
+          'chips',
+          'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS',
+          'Chips'
+        ),
+        new FshCode(
+          'cakes',
+          'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS',
+          'Cakes'
+        ),
+        new FshCode(
+          'verylargecakes',
+          'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS',
+          'Very Large Cakes'
+        )
       ];
       rule.from.system = 'http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS';
       rule.from.valueSets = ['http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS'];
 
       const result = rule.toFSH();
       const expectedResult = [
-        '* include #cookies "Cookies" and',
-        '    #candy "Candy" and',
-        '    #chips "Chips" and',
-        '    #cakes "Cakes" and',
-        '    #verylargecakes "Very Large Cakes"',
-        '    from system http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS and',
-        '    valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS'
+        '* http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS#cookies "Cookies"',
+        '    from valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS',
+        '* http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS#candy "Candy"',
+        '    from valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS',
+        '* http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS#chips "Chips"',
+        '    from valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS',
+        '* http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS#cakes "Cakes"',
+        '    from valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS',
+        '* http://fhir.food-pyramid.org/FoodPyramidGuide/CodeSystems/FoodGroupsCS#verylargecakes "Very Large Cakes"',
+        '    from valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS'
       ].join(EOL);
       expect(result).toEqual(expectedResult);
     });