Repository: FHIR/sushi (ts)

Issue:
Instance with unquoted numeric id causes call stack overflow
Assignment rules on Instance `id` should be quoted strings. But, if the value is unquoted and numeric, the initial assignment during export will fail due to being the wrong type. SUSHI sees that the value is numeric, and makes the guess that maybe it's supposed to be assigning an instance identified by that numeric value. It will fish for such an Instance, find the one already being exported. This is because the Instance's `id` accessor method casts the rule's value to a string. This leads to the Instance being exported again, eventually leading to call stack overflow. It would probably make more sense if a non-string value on a rule on `id` did _not_ get returned by the accessor.

Sample FSH:
```
Instance: requireddataforantimicro-response
InstanceOf: Questionnaire
Usage: #example
* id = 3193360
```

In this case, the expected behavior would be an error on the `id` rule indicating that the numeric value is the wrong type.

[Zulip thread](https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/Sushi.3A.20.20Maximum.20call.20stack.20size.20exceeded.20.3F/near/472525892)

Pull request:
Only return FshType id from rule value if the value is a string
**Description:**
When getting the id for a FshType, rules are checked to see if there is one that will set the id and does not refer to an instance. A value that is not a string will not be valid and not be applied. Therefore, only return the value if it is a string.

Rename test file for FshCodeSystem to match name of source file.

**Testing Instructions:**
Confirm that using a non-string value in a rule that sets an entity's `id` does not allow for other rules to fish up that entity using that id. The sample FSH from the issue is an example of this on an Instance:
```
Instance: requireddataforantimicro-response
InstanceOf: Questionnaire
Usage: #example
* id = 3193360
```
In this case, an error will be logged for the `id` rule since it is the wrong type, and there should not be a call stack overflow as described in the linked issue. Behavior should be the same for Profile, Extension, Logical, Resource, CodeSystem, and ValueSet, although these entities will set `id` using a caret rule, not an assignment rule.

**Related Issue:**
Fixes #1519.

Patch:
diff --git a/src/fshtypes/FshCodeSystem.ts b/src/fshtypes/FshCodeSystem.ts
index f6a6e52..690f8ed 100644
--- a/src/fshtypes/FshCodeSystem.ts
+++ b/src/fshtypes/FshCodeSystem.ts
@@ -25,8 +25,8 @@ export class FshCodeSystem extends FshEntity {
 
   get id() {
     const assignedId = getNonInstanceValueFromRules(this, 'id', '', 'id');
-    if (assignedId) {
-      return assignedId.toString();
+    if (typeof assignedId === 'string') {
+      return assignedId;
     }
     return this._id;
   }
diff --git a/src/fshtypes/FshStructure.ts b/src/fshtypes/FshStructure.ts
index 65f2dde..d785af0 100644
--- a/src/fshtypes/FshStructure.ts
+++ b/src/fshtypes/FshStructure.ts
@@ -22,8 +22,8 @@ export abstract class FshStructure extends FshEntity {
 
   get id() {
     const assignedId = getNonInstanceValueFromRules(this, 'id', '', 'id');
-    if (assignedId) {
-      return assignedId.toString();
+    if (typeof assignedId === 'string') {
+      return assignedId;
     }
     return this._id;
   }
diff --git a/src/fshtypes/FshValueSet.ts b/src/fshtypes/FshValueSet.ts
index 80cf7e2..7a534b6 100644
--- a/src/fshtypes/FshValueSet.ts
+++ b/src/fshtypes/FshValueSet.ts
@@ -25,8 +25,8 @@ export class FshValueSet extends FshEntity {
 
   get id() {
     const assignedId = getNonInstanceValueFromRules(this, 'id', '', 'id');
-    if (assignedId) {
-      return assignedId.toString();
+    if (typeof assignedId === 'string') {
+      return assignedId;
     }
     return this._id;
   }
diff --git a/src/fshtypes/Instance.ts b/src/fshtypes/Instance.ts
index a9d5aaa..4d14645 100644
--- a/src/fshtypes/Instance.ts
+++ b/src/fshtypes/Instance.ts
@@ -25,8 +25,8 @@ export class Instance extends FshEntity {
 
   get id() {
     const assignedId = getNonInstanceValueFromRules(this, 'id', '', 'id');
-    if (assignedId) {
-      return assignedId.toString();
+    if (typeof assignedId === 'string') {
+      return assignedId;
     }
     return this._id;
   }


Test patch:
diff --git a/test/fshtypes/CodeSystem.test.ts b/test/fshtypes/FshCodeSystem.test.ts
similarity index 72%
rename from test/fshtypes/CodeSystem.test.ts
rename to test/fshtypes/FshCodeSystem.test.ts
index 66822ec..1d009a3 100644
--- a/test/fshtypes/CodeSystem.test.ts
+++ b/test/fshtypes/FshCodeSystem.test.ts
@@ -1,9 +1,9 @@
 import 'jest-extended';
 import { FshCodeSystem } from '../../src/fshtypes/FshCodeSystem';
-import { ConceptRule } from '../../src/fshtypes/rules';
+import { CaretValueRule, ConceptRule } from '../../src/fshtypes/rules';
 import { EOL } from 'os';
 
-describe('CodeSystem', () => {
+describe('FshCodeSystem', () => {
   describe('#constructor', () => {
     it('should set initial properties correctly', () => {
       const cs = new FshCodeSystem('MyCodeSystem');
@@ -15,6 +15,37 @@ describe('CodeSystem', () => {
     });
   });
 
+  describe('#id', () => {
+    it('should return an id set by a string caret rule', () => {
+      const p = new FshCodeSystem('MyCodeSystem');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = 'different-id';
+      p.rules.push(idRule);
+      expect(p.id).toBe('different-id');
+    });
+
+    it('should not return an id set by a non-string caret rule', () => {
+      const p = new FshCodeSystem('MyCodeSystem');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = BigInt(23456);
+      idRule.rawValue = '23456';
+      p.rules.push(idRule);
+      expect(p.id).toBe('MyCodeSystem');
+    });
+
+    it('should not return an id set by an instance caret rule', () => {
+      const p = new FshCodeSystem('MyCodeSystem');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = 'different-id';
+      idRule.isInstance = true;
+      p.rules.push(idRule);
+      expect(p.id).toBe('MyCodeSystem');
+    });
+  });
+
   describe('#toFSH', () => {
     it('should produce FSH for the simplest CodeSystem', () => {
       const input = new FshCodeSystem('SimpleCodeSystem');
diff --git a/test/fshtypes/FshValueSet.test.ts b/test/fshtypes/FshValueSet.test.ts
index 3e84d93..4c42fc1 100644
--- a/test/fshtypes/FshValueSet.test.ts
+++ b/test/fshtypes/FshValueSet.test.ts
@@ -2,6 +2,7 @@ import 'jest-extended';
 import { FshValueSet } from '../../src/fshtypes/FshValueSet';
 import { EOL } from 'os';
 import {
+  CaretValueRule,
   ValueSetConceptComponentRule,
   ValueSetFilterComponentRule
 } from '../../src/fshtypes/rules';
@@ -19,6 +20,37 @@ describe('ValueSet', () => {
     });
   });
 
+  describe('#id', () => {
+    it('should return an id set by a string caret rule', () => {
+      const p = new FshValueSet('MyValueSet');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = 'different-id';
+      p.rules.push(idRule);
+      expect(p.id).toBe('different-id');
+    });
+
+    it('should not return an id set by a non-string caret rule', () => {
+      const p = new FshValueSet('MyValueSet');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = BigInt(23456);
+      idRule.rawValue = '23456';
+      p.rules.push(idRule);
+      expect(p.id).toBe('MyValueSet');
+    });
+
+    it('should not return an id set by an instance caret rule', () => {
+      const p = new FshValueSet('MyValueSet');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = 'different-id';
+      idRule.isInstance = true;
+      p.rules.push(idRule);
+      expect(p.id).toBe('MyValueSet');
+    });
+  });
+
   describe('#toFSH', () => {
     it('should produce FSH for the simplest ValueSet', () => {
       const vs = new FshValueSet('MyValueSet');
diff --git a/test/fshtypes/Instance.test.ts b/test/fshtypes/Instance.test.ts
index d256c2d..f5352c2 100644
--- a/test/fshtypes/Instance.test.ts
+++ b/test/fshtypes/Instance.test.ts
@@ -24,6 +24,16 @@ describe('Instance', () => {
       expect(p.id).toBe('different-id');
     });
 
+    it('should not return an id set by a non-string assignment rule', () => {
+      const p = new Instance('MyInstance');
+      const idRule = new AssignmentRule('id');
+      idRule.value = BigInt(12345);
+      idRule.rawValue = '12345';
+      p.rules.push(idRule);
+      // the id is still the default
+      expect(p.id).toBe('MyInstance');
+    });
+
     it('should not return an id set by an instance assignment rule', () => {
       const p = new Instance('MyInstance');
       const idRule = new AssignmentRule('id');
diff --git a/test/fshtypes/Profile.test.ts b/test/fshtypes/Profile.test.ts
index 86473a7..d844b30 100644
--- a/test/fshtypes/Profile.test.ts
+++ b/test/fshtypes/Profile.test.ts
@@ -29,6 +29,16 @@ describe('Profile', () => {
       expect(p.id).toBe('different-id');
     });
 
+    it('should not return an id set by a non-string caret rule', () => {
+      const p = new Profile('MyObservation');
+      const idRule = new CaretValueRule('');
+      idRule.caretPath = 'id';
+      idRule.value = BigInt(23456);
+      idRule.rawValue = '23456';
+      p.rules.push(idRule);
+      expect(p.id).toBe('MyObservation');
+    });
+
     it('should not return an id set by an instance caret rule', () => {
       const p = new Profile('MyObservation');
       const idRule = new CaretValueRule('');