Repository: FHIR/sushi (ts)

Issue:
SUSHI emits spurious warnings on use of soft indexing
SUSHI 2.2.2 introduced a bug that causes SUSHI to log warnings on many valid uses of soft indexing.  For example, the following causes a number of warnings:

```
Instance: MyPatient
InstanceOf: Patient
* name[+].use = #usual
* name[=].family = "Smith"
* name[=].given = "John"
* name[+].use = #old
* name[=].family = "Schnidt"
* name[=].given[+] = "John"
* name[=].given[+] = "Jacob"
* name[=].given[+] = "Jingle"
* name[=].given[+] = "Heimer"
* name[=].period.end = "1960"
* name[+].use = #official
* name[=].family = "Smith"
* name[=].given[+] = "John"
* name[=].given[+] = "Jacob"
* name[=].given[+] = "Jingleheimer"
* name[=].period.start = "1960-01-01"
* name[+].use = #nickname
* name[=].family = "Smith"
* name[=].given = "Jack"
```

Since the above is completely valid, there should be no warnings.

See: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/SUSHI.202.2E2.2E2/near/264514577

Pull request:
Fix Spurious Soft Indexing Warnings
Fixes #978 

SUSHI 2.2.2 introduced a bug that caused warnings to be produced for completely valid uses of soft indexing.  See #978 for an example.

To test, run the example code in #978 using SUSHI 2.2.2 (or master).  Then run the same code using this PR.

_NOTE: I have done a full regression and confirmed this does not affect the actual output of any IGs._

Patch:
diff --git a/src/import/FSHImporter.ts b/src/import/FSHImporter.ts
index 65ff421..f9a2580 100644
--- a/src/import/FSHImporter.ts
+++ b/src/import/FSHImporter.ts
@@ -2059,74 +2059,77 @@ export class FSHImporter extends FSHVisitor {
     parentCtx: ParserRuleContext,
     isPathRule: boolean
   ): string[] {
-    const location = this.extractStartStop(parentCtx);
-    const currentIndent = location.startColumn - DEFAULT_START_COLUMN;
-    const contextIndex = currentIndent / INDENT_WIDTH;
+    try {
+      const location = this.extractStartStop(parentCtx);
+      const currentIndent = location.startColumn - DEFAULT_START_COLUMN;
+      const contextIndex = currentIndent / INDENT_WIDTH;
 
-    if (!this.isValidContext(location, currentIndent, this.pathContext)) {
-      return path;
-    }
+      if (!this.isValidContext(location, currentIndent, this.pathContext)) {
+        return path;
+      }
 
-    // If the element is not indented, just reset the context
-    if (contextIndex === 0) {
-      // If the last context still has [+], that means the path was never used
-      // and the [+] will be discarded without incrementing
-      if (
-        this.pathContext.length > 0 &&
-        this.pathContext[this.pathContext.length - 1].some(p => /\[\+\]/.test(p))
-      ) {
-        logger.warn(
-          'The previous line(s) use path rules to establish a context using soft indexing ' +
-            '(e.g., [+]), but that context is reset by the following rule before it is ever ' +
-            'used. As a result, the previous path context will be ignored and its ' +
-            'soft-indices will not be incremented',
+      // If the element is not indented, just reset the context
+      if (contextIndex === 0) {
+        // If the last context still has [+], that means the path was never used
+        // and the [+] will be discarded without incrementing
+        if (
+          this.pathContext.length > 0 &&
+          this.pathContext[this.pathContext.length - 1].some(p => /\[\+\]/.test(p))
+        ) {
+          logger.warn(
+            'The previous line(s) use path rules to establish a context using soft indexing ' +
+              '(e.g., [+]), but that context is reset by the following rule before it is ever ' +
+              'used. As a result, the previous path context will be ignored and its ' +
+              'soft-indices will not be incremented',
+            {
+              location,
+              file: this.currentFile
+            }
+          );
+        }
+        this.pathContext = [path];
+        return path;
+      }
+
+      if (path.length === 1 && path[0] === '.') {
+        logger.error(
+          "The special '.' path is only allowed in top-level rules. The rule will be processed as if it is not indented.",
           {
             location,
             file: this.currentFile
           }
         );
+        return path;
       }
-      this.pathContext = [path];
-      return path;
-    }
 
-    if (path.length === 1 && path[0] === '.') {
-      logger.error(
-        "The special '.' path is only allowed in top-level rules. The rule will be processed as if it is not indented.",
-        {
-          location,
-          file: this.currentFile
-        }
-      );
-      return path;
-    }
-
-    // Otherwise, get the context based on the indent level.
-    const currentContext = this.pathContext[contextIndex - 1];
-    if (currentContext.length === 0) {
-      logger.error(
-        'Rule cannot be indented below rule which has no path. The rule will be processed as if it is not indented.',
-        { location, file: this.currentFile }
-      );
-      return path;
-    }
+      // Otherwise, get the context based on the indent level.
+      const currentContext = this.pathContext[contextIndex - 1];
+      if (currentContext.length === 0) {
+        logger.error(
+          'Rule cannot be indented below rule which has no path. The rule will be processed as if it is not indented.',
+          { location, file: this.currentFile }
+        );
+        return path;
+      }
 
-    // Trim out-of-scope contexts
-    this.pathContext.splice(contextIndex);
+      // Trim out-of-scope contexts
+      this.pathContext.splice(contextIndex);
 
-    // Get the new path and add as the last context
-    const fullPath = currentContext.concat(path);
-    this.pathContext.push(fullPath);
+      // Get the new path and add as the last context
+      const fullPath = currentContext.concat(path);
+      this.pathContext.push(fullPath);
 
-    // Once we have used the existing context in a non-path-only rule, replace [+] with [=] in all
-    // existing contexts so that the [+] isn't re-applied in further contexts
-    if (!isPathRule) {
-      this.pathContext.forEach((path, i) => {
-        this.pathContext[i] = path.map(c => c.replace(/\[\+\]/g, '[=]'));
-      });
+      return fullPath;
+    } finally {
+      // NOTE: This block is in finally so it is always executed, no matter where/when we exit the function
+      // Once we have used the existing context in a non-path-only rule, replace [+] with [=] in all
+      // existing contexts so that the [+] isn't re-applied in further contexts
+      if (!isPathRule) {
+        this.pathContext.forEach((path, i) => {
+          this.pathContext[i] = path.map(c => c.replace(/\[\+\]/g, '[=]'));
+        });
+      }
     }
-
-    return fullPath;
   }
 
   private isValidContext(


Test patch:
diff --git a/test/import/FSHImporter.context.test.ts b/test/import/FSHImporter.context.test.ts
index e5c8254..20cf02e 100644
--- a/test/import/FSHImporter.context.test.ts
+++ b/test/import/FSHImporter.context.test.ts
@@ -233,6 +233,26 @@ describe('FSHImporter', () => {
       assertAssignmentRule(instance.rules[0], 'item[+].linkId', 'bar');
     });
 
+    it('should not log a warning for non-path rules that increment the index', () => {
+      const input = leftAlign(`
+      Instance: Foo
+      InstanceOf: Questionnaire
+      * item[+].linkId = "foo"
+      * item[+].linkId = "bar"
+    `);
+
+      const result = importSingleText(input, 'Context.fsh');
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+      expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+      expect(result.instances.size).toBe(1);
+      const instance = result.instances.get('Foo');
+      expect(instance.name).toBe('Foo');
+      expect(instance.instanceOf).toBe('Questionnaire');
+      expect(instance.rules.length).toBe(2);
+      assertAssignmentRule(instance.rules[0], 'item[+].linkId', 'foo');
+      assertAssignmentRule(instance.rules[1], 'item[+].linkId', 'bar');
+    });
+
     it('should change + to = when setting context on children of soft indexed rules which are not path rules', () => {
       const input = leftAlign(`
       Instance: Foo