From bc4124655a1d0181c76f9afa7703dad617367690 Mon Sep 17 00:00:00 2001
From: Chris Moesel <cmoesel@mitre.org>
Date: Thu, 19 Dec 2024 18:02:56 -0500
Subject: [PATCH] Log invalid path syntax due to unmatched brackets

---
 src/utils/PathUtils.ts       | 20 +++++++++++++++++---
 test/utils/PathUtils.test.ts | 29 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/utils/PathUtils.ts b/src/utils/PathUtils.ts
index b21b8b49b..033cab37d 100644
--- a/src/utils/PathUtils.ts
+++ b/src/utils/PathUtils.ts
@@ -2,6 +2,7 @@ import { flatten } from 'lodash';
 import { InstanceDefinition, PathPart } from '../fhirtypes';
 import { splitOnPathPeriods } from '../fhirtypes/common';
 import { CaretValueRule, Rule } from '../fshtypes/rules';
+import { SourceInfo } from '../fshtypes/FshEntity';
 import { logger } from './FSHLogger';
 
 /**
@@ -9,7 +10,7 @@ import { logger } from './FSHLogger';
  * @param {string} fshPath - A syntactically valid path in FSH
  * @returns {PathPart[]} an array of PathParts that is easier to work with
  */
-export function parseFSHPath(fshPath: string): PathPart[] {
+export function parseFSHPath(fshPath: string, sourceInfo?: SourceInfo): PathPart[] {
   const pathParts: PathPart[] = [];
   const seenSlices: string[] = [];
   const indexRegex = /^[0-9]+$/;
@@ -30,6 +31,19 @@ export function parseFSHPath(fshPath: string): PathPart[] {
       if (seenSlices.length > 0) {
         parsedPart.slices = [...seenSlices];
       }
+      const parsedPartLength =
+        parsedPart.base.length +
+        parsedPart.brackets
+          .map(b => b.length + 2)
+          .reduce((total: number, current: number) => total + current, 0);
+      if (pathPart.length !== parsedPartLength) {
+        const message = `Error processing path due to unmatched brackets: ${fshPath}. `;
+        if (sourceInfo) {
+          logger.error(message, sourceInfo);
+        } else {
+          logger.error(message);
+        }
+      }
     }
     pathParts.push(parsedPart);
   }
@@ -196,11 +210,11 @@ export function resolveSoftIndexing(rules: Array<Rule | CaretValueRule>, strict
   // Parsing and separating rules by base name and bracket indexes
   const parsedRules = rules.map(rule => {
     const parsedPath: { path: PathPart[]; caretPath?: PathPart[] } = {
-      path: parseFSHPath(rule.path)
+      path: parseFSHPath(rule.path, rule.sourceInfo)
     };
     // If we have a CaretValueRule, we'll need a second round of parsing for the caret path
     if (rule instanceof CaretValueRule) {
-      parsedPath.caretPath = parseFSHPath(rule.caretPath);
+      parsedPath.caretPath = parseFSHPath(rule.caretPath, rule.sourceInfo);
     }
     return parsedPath;
   });
diff --git a/test/utils/PathUtils.test.ts b/test/utils/PathUtils.test.ts
index 7dbc14a85..0a73c9c5c 100644
--- a/test/utils/PathUtils.test.ts
+++ b/test/utils/PathUtils.test.ts
@@ -319,6 +319,7 @@ describe('PathUtils', () => {
       const testPath = 'item1.item2.item3';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(3);
       expect(pathParts[0]).toEqual({ base: 'item1' });
       expect(pathParts[1]).toEqual({ base: 'item2' });
@@ -329,6 +330,7 @@ describe('PathUtils', () => {
       const testPath = 'item1[0].item2[0].item3[0]';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(3);
       expect(pathParts[0]).toEqual({ base: 'item1', brackets: ['0'] });
       expect(pathParts[1]).toEqual({ base: 'item2', brackets: ['0'] });
@@ -339,6 +341,7 @@ describe('PathUtils', () => {
       const testPath = 'item1[10].item2[11].item3[12]';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(3);
       expect(pathParts[0]).toEqual({ base: 'item1', brackets: ['10'] });
       expect(pathParts[1]).toEqual({ base: 'item2', brackets: ['11'] });
@@ -349,6 +352,7 @@ describe('PathUtils', () => {
       const testPath = 'item1[10][Slice1].item2[11][Slice2].item3[12][Slice3]';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(3);
       expect(pathParts[0]).toEqual({
         base: 'item1',
@@ -371,6 +375,7 @@ describe('PathUtils', () => {
       const testPath = 'item1[10][foo[x]].item2[11][bar[x]][baz[x]].value[x]';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(3);
       expect(pathParts[0]).toEqual({
         base: 'item1',
@@ -391,6 +396,7 @@ describe('PathUtils', () => {
       const testPath = 'value[x][foo]';
       const pathParts = parseFSHPath(testPath);
 
+      expect(loggerSpy.getAllLogs('error')).toBeEmpty();
       expect(pathParts).toHaveLength(1);
       expect(pathParts[0]).toEqual({
         base: 'value[x]',
@@ -398,6 +404,29 @@ describe('PathUtils', () => {
         slices: ['foo']
       });
     });
+
+    it('should properly detect syntax errors in path elements (drawn from real world example)', () => {
+      const testPath = 'item1[+}.value[x]';
+      const sourceInfo = {
+        file: 'testfile.fsh',
+        location: { startLine: 5, startColumn: 0, endLine: 5, endColumn: 19 }
+      };
+      const pathParts = parseFSHPath(testPath, sourceInfo);
+
+      expect(loggerSpy.getLastMessage('error')).toMatch(
+        'Error processing path due to unmatched brackets: item1[+}.value[x]'
+      );
+      expect(loggerSpy.getLastMessage('error')).toMatch(/File: testfile\.fsh.*Line: 5\D*/s);
+
+      expect(pathParts).toHaveLength(2);
+      expect(pathParts[0]).toEqual({
+        base: 'item1',
+        brackets: []
+      });
+      expect(pathParts[1]).toEqual({
+        base: 'value[x]'
+      });
+    });
   });
 
   describe('#collectValuesAtElementIdOrPath', () => {