Fix compiler failing with nested with expression

The previous implementation of `WithParser` used regex, which struggles
with parsing nested structures correctly. This commit improves
`WithParser` to track and parse all nested `with` expressions.

Other improvements:

- Throw meaningful errors when syntax is wrong. Replacing the prior
  behavior of silently ignoring such issues.
- Remove `I` prefix from related interfaces to align with newer code
  conventions.
- Add more unit tests for `with` expression.
- Improve documentation for templating.
- `ExpressionRegexBuilder`:
  - Use words `capture` and `match` correctly.
  - Fix minor issues revealed by new and improved tests:
     - Change regex for matching anything except surrounding
       whitespaces. The new regex ensures that it works even without
       having any preceeding text.
     - Change regex for capturing pipelines. The old regex was only
       matching (non-greedy) first character of the pipeline in tests,
       new regex matches the full pipeline.
- `ExpressionRegexBuilder.spec.ts`:
  - Ensure consistent way to define `describe` and `it` blocks.
  - Replace `expectRegex` tests, regex expectations test internal
    behavior of the class, not the external.
  - Simplified tests by eliminating the need for UUID suffixes/prefixes.

tests/unit/application/Parser/Script/Compiler/Expressions/SyntaxParsers/SyntaxParserTestsRunner.ts

@@ -4,25 +4,26 @@ import { IExpressionParser } from '@/application/Parser/Script/Compiler/Expressi
 import { FunctionCallArgumentCollectionStub } from '@tests/unit/shared/Stubs/FunctionCallArgumentCollectionStub';
 import { ExpressionEvaluationContextStub } from '@tests/unit/shared/Stubs/ExpressionEvaluationContextStub';
 import { PipelineCompilerStub } from '@tests/unit/shared/Stubs/PipelineCompilerStub';
+import { scrambledEqual } from '@/application/Common/Array';
 
 export class SyntaxParserTestsRunner {
   constructor(private readonly sut: IExpressionParser) {
   }
 
-  public expectPosition(...testCases: IExpectPositionTestCase[]) {
+  public expectPosition(...testCases: ExpectPositionTestScenario[]) {
     for (const testCase of testCases) {
       it(testCase.name, () => {
         // act
         const expressions = this.sut.findExpressions(testCase.code);
         // assert
         const actual = expressions.map((e) => e.position);
-        expect(actual).to.deep.equal(testCase.expected);
+        expect(scrambledEqual(actual, testCase.expected));
       });
     }
     return this;
   }
 
-  public expectNoMatch(...testCases: INoMatchTestCase[]) {
+  public expectNoMatch(...testCases: NoMatchTestScenario[]) {
     this.expectPosition(...testCases.map((testCase) => ({
       name: testCase.name,
       code: testCase.code,
@@ -30,7 +31,7 @@ export class SyntaxParserTestsRunner {
     })));
   }
 
-  public expectResults(...testCases: IExpectResultTestCase[]) {
+  public expectResults(...testCases: ExpectResultTestScenario[]) {
     for (const testCase of testCases) {
       it(testCase.name, () => {
         // arrange
@@ -47,7 +48,21 @@ export class SyntaxParserTestsRunner {
     return this;
   }
 
-  public expectPipeHits(data: IExpectPipeHitTestData) {
+  public expectThrows(...testCases: ExpectThrowsTestScenario[]) {
+    for (const testCase of testCases) {
+      it(testCase.name, () => {
+        // arrange
+        const { expectedError } = testCase;
+        // act
+        const act = () => this.sut.findExpressions(testCase.code);
+        // assert
+        expect(act).to.throw(expectedError);
+      });
+    }
+    return this;
+  }
+
+  public expectPipeHits(data: ExpectPipeHitTestScenario) {
     for (const validPipePart of PipeTestCases.ValidValues) {
       this.expectHitPipePart(validPipePart, data);
     }
@@ -56,7 +71,7 @@ export class SyntaxParserTestsRunner {
     }
   }
 
-  private expectHitPipePart(pipeline: string, data: IExpectPipeHitTestData) {
+  private expectHitPipePart(pipeline: string, data: ExpectPipeHitTestScenario) {
     it(`"${pipeline}" hits`, () => {
       // arrange
       const expectedPipePart = pipeline.trim();
@@ -73,14 +88,14 @@ export class SyntaxParserTestsRunner {
       // assert
       expect(expressions).has.lengthOf(1);
       expect(pipelineCompiler.compileHistory).has.lengthOf(1);
-      const actualPipeNames = pipelineCompiler.compileHistory[0].pipeline;
+      const actualPipePart = pipelineCompiler.compileHistory[0].pipeline;
       const actualValue = pipelineCompiler.compileHistory[0].value;
-      expect(actualPipeNames).to.equal(expectedPipePart);
+      expect(actualPipePart).to.equal(expectedPipePart);
       expect(actualValue).to.equal(data.parameterValue);
     });
   }
 
-  private expectMissPipePart(pipeline: string, data: IExpectPipeHitTestData) {
+  private expectMissPipePart(pipeline: string, data: ExpectPipeHitTestScenario) {
     it(`"${pipeline}" misses`, () => {
       // arrange
       const args = new FunctionCallArgumentCollectionStub()
@@ -98,42 +113,51 @@ export class SyntaxParserTestsRunner {
     });
   }
 }
-interface IExpectResultTestCase {
-  name: string;
-  code: string;
-  args: (builder: FunctionCallArgumentCollectionStub) => FunctionCallArgumentCollectionStub;
-  expected: readonly string[];
+
+interface ExpectResultTestScenario {
+  readonly name: string;
+  readonly code: string;
+  readonly args: (
+    builder: FunctionCallArgumentCollectionStub,
+  ) => FunctionCallArgumentCollectionStub;
+  readonly expected: readonly string[];
 }
 
-interface IExpectPositionTestCase {
-  name: string;
-  code: string;
-  expected: readonly ExpressionPosition[];
+interface ExpectThrowsTestScenario {
+  readonly name: string;
+  readonly code: string;
+  readonly expectedError: string;
 }
 
-interface INoMatchTestCase {
-  name: string;
-  code: string;
+interface ExpectPositionTestScenario {
+  readonly name: string;
+  readonly code: string;
+  readonly expected: readonly ExpressionPosition[];
 }
 
-interface IExpectPipeHitTestData {
-  codeBuilder: (pipeline: string) => string;
-  parameterName: string;
-  parameterValue: string;
+interface NoMatchTestScenario {
+  readonly name: string;
+  readonly code: string;
+}
+
+interface ExpectPipeHitTestScenario {
+  readonly codeBuilder: (pipeline: string) => string;
+  readonly parameterName: string;
+  readonly parameterValue: string;
 }
 
 const PipeTestCases = {
   ValidValues: [
     // Single pipe with different whitespace combinations
-    ' | pipe1', ' |pipe1', '|pipe1', ' |pipe1', '   |   pipe1',
+    ' | pipe', ' |pipe', '|pipe', ' |pipe', '   |   pipe',
 
     // Double pipes with different whitespace combinations
-    ' | pipe1 | pipe2', '| pipe1|pipe2', '|pipe1|pipe2', ' |pipe1 |pipe2', '| pipe1 | pipe2| pipe3 |pipe4',
-
-    // Wrong cases, but should match anyway and let pipelineCompiler throw errors
-    '| pip€', '| pip{e} ',
+    ' | pipeFirst | pipeSecond', '| pipeFirst|pipeSecond', '|pipeFirst|pipeSecond', ' |pipeFirst |pipeSecond', '| pipeFirst | pipeSecond| pipeThird |pipeFourth',
   ],
   InvalidValues: [
-    ' pipe1  |pipe2', ' pipe1',
+    ' withoutPipeBefore  |pipe', ' withoutPipeBefore',
+
+    // It's OK to match them (move to valid values if needed) to let compiler throw instead.
+    '| pip€', '| pip{e} ', '| pipeWithNumber55', '| pipe with whitespace',
   ],
 };

tests/unit/application/Parser/Script/Compiler/Expressions/SyntaxParsers/WithParser.spec.ts

@@ -7,15 +7,15 @@ import { SyntaxParserTestsRunner } from './SyntaxParserTestsRunner';
 describe('WithParser', () => {
   const sut = new WithParser();
   const runner = new SyntaxParserTestsRunner(sut);
-  describe('finds as expected', () => {
+  describe('correctly identifies `with` syntax', () => {
     runner.expectPosition(
       {
-        name: 'when no scope is not used',
+        name: 'when no context variable is not used',
         code: 'hello {{ with $parameter }}no usage{{ end }} here',
         expected: [new ExpressionPosition(6, 44)],
       },
       {
-        name: 'when scope is used',
+        name: 'when context variable is used',
         code: 'used here ({{ with $parameter }}value: {{.}}{{ end }})',
         expected: [new ExpressionPosition(11, 53)],
       },
@@ -25,38 +25,70 @@ describe('WithParser', () => {
         expected: [new ExpressionPosition(7, 51), new ExpressionPosition(61, 99)],
       },
       {
-        name: 'tolerate lack of whitespaces',
+        name: 'when nested',
+        code: 'outer: {{ with $outer }}outer value with context variable: {{ . }}, inner: {{ with $inner }}inner value{{ end }}.{{ end }}',
+        expected: [
+          /* outer: */ new ExpressionPosition(7, 122),
+          /* inner: */ new ExpressionPosition(77, 112),
+        ],
+      },
+      {
+        name: 'whitespaces: tolerate lack of whitespaces',
         code: 'no whitespaces {{with $parameter}}value: {{ . }}{{end}}',
         expected: [new ExpressionPosition(15, 55)],
       },
       {
-        name: 'match multiline text',
+        name: 'newlines: match multiline text',
         code: 'non related line\n{{ with $middleLine }}\nline before value\n{{ . }}\nline after value\n{{ end }}\nnon related line',
         expected: [new ExpressionPosition(17, 92)],
       },
+      {
+        name: 'newlines: does not match newlines before',
+        code: '\n{{ with $unimportant }}Text{{ end }}',
+        expected: [new ExpressionPosition(1, 37)],
+      },
+      {
+        name: 'newlines: does not match newlines after',
+        code: '{{ with $unimportant }}Text{{ end }}\n',
+        expected: [new ExpressionPosition(0, 36)],
+      },
+    );
+  });
+  describe('throws with incorrect `with` syntax', () => {
+    runner.expectThrows(
+      {
+        name: 'incorrect `with`: whitespace after dollar sign inside `with` statement',
+        code: '{{with $ parameter}}value: {{ . }}{{ end }}',
+        expectedError: 'Context variable before `with` statement.',
+      },
+      {
+        name: 'incorrect `with`: whitespace before dollar sign inside `with` statement',
+        code: '{{ with$parameter}}value: {{ . }}{{ end }}',
+        expectedError: 'Context variable before `with` statement.',
+      },
+      {
+        name: 'incorrect `with`: missing `with` statement',
+        code: '{{ when $parameter}}value: {{ . }}{{ end }}',
+        expectedError: 'Context variable before `with` statement.',
+      },
+      {
+        name: 'incorrect `end`: missing `end` statement',
+        code: '{{ with $parameter}}value: {{ . }}{{ fin }}',
+        expectedError: 'Missing `end` statement, forgot `{{ end }}?',
+      },
+      {
+        name: 'incorrect `end`: used without `with`',
+        code: 'Value {{ end }}',
+        expectedError: 'Redundant `end` statement, missing `with`?',
+      },
+      {
+        name: 'incorrect "context variable": used without `with`',
+        code: 'Value: {{ . }}',
+        expectedError: 'Context variable before `with` statement.',
+      },
     );
   });
   describe('ignores when syntax is wrong', () => {
-    describe('ignores expression if "with" syntax is wrong', () => {
-      runner.expectNoMatch(
-        {
-          name: 'does not tolerate whitespace after with',
-          code: '{{with $ parameter}}value: {{ . }}{{ end }}',
-        },
-        {
-          name: 'does not tolerate whitespace before dollar',
-          code: '{{ with$parameter}}value: {{ . }}{{ end }}',
-        },
-        {
-          name: 'wrong text at scope end',
-          code: '{{ with$parameter}}value: {{ . }}{{ fin }}',
-        },
-        {
-          name: 'wrong text at expression start',
-          code: '{{ when $parameter}}value: {{ . }}{{ end }}',
-        },
-      );
-    });
     describe('does not render argument if substitution syntax is wrong', () => {
       runner.expectResults(
         {
@@ -83,54 +115,73 @@ describe('WithParser', () => {
       );
     });
   });
-  describe('renders scope conditionally', () => {
-    describe('does not render scope if argument is undefined', () => {
-      runner.expectResults(
-        ...getAbsentStringTestCases().map((testCase) => ({
-          name: `does not render when value is "${testCase.valueName}"`,
-          code: '{{ with $parameter }}dark{{ end }} ',
-          args: (args) => args
-            .withArgument('parameter', testCase.absentValue),
-          expected: [''],
-        })),
-        {
-          name: 'does not render when argument is not provided',
-          code: '{{ with $parameter }}dark{{ end }}',
-          args: (args) => args,
-          expected: [''],
-        },
-      );
+  describe('scope rendering', () => {
+    describe('conditional rendering based on argument value', () => {
+      describe('does not render scope', () => {
+        runner.expectResults(
+          ...getAbsentStringTestCases().map((testCase) => ({
+            name: `does not render when value is "${testCase.valueName}"`,
+            code: '{{ with $parameter }}dark{{ end }} ',
+            args: (args) => args
+              .withArgument('parameter', testCase.absentValue),
+            expected: [''],
+          })),
+          {
+            name: 'does not render when argument is not provided',
+            code: '{{ with $parameter }}dark{{ end }}',
+            args: (args) => args,
+            expected: [''],
+          },
+        );
+      });
+      describe('renders scope', () => {
+        runner.expectResults(
+          ...getAbsentStringTestCases().map((testCase) => ({
+            name: `does not render when value is "${testCase.valueName}"`,
+            code: '{{ with $parameter }}dark{{ end }} ',
+            args: (args) => args
+              .withArgument('parameter', testCase.absentValue),
+            expected: [''],
+          })),
+          {
+            name: 'does not render when argument is not provided',
+            code: '{{ with $parameter }}dark{{ end }}',
+            args: (args) => args,
+            expected: [''],
+          },
+          {
+            name: 'renders scope even if value is not used',
+            code: '{{ with $parameter }}Hello world!{{ end }}',
+            args: (args) => args
+              .withArgument('parameter', 'Hello'),
+            expected: ['Hello world!'],
+          },
+          {
+            name: 'renders value when it has value',
+            code: '{{ with $parameter }}{{ . }} world!{{ end }}',
+            args: (args) => args
+              .withArgument('parameter', 'Hello'),
+            expected: ['Hello world!'],
+          },
+          {
+            name: 'renders value when whitespaces around brackets are missing',
+            code: '{{ with $parameter }}{{.}} world!{{ end }}',
+            args: (args) => args
+              .withArgument('parameter', 'Hello'),
+            expected: ['Hello world!'],
+          },
+          {
+            name: 'renders value multiple times when it\'s used multiple times',
+            code: '{{ with $letterL }}He{{ . }}{{ . }}o wor{{ . }}d!{{ end }}',
+            args: (args) => args
+              .withArgument('letterL', 'l'),
+            expected: ['Hello world!'],
+          },
+        );
+      });
     });
-    describe('render scope when variable has value', () => {
+    describe('whitespace handling inside scope', () => {
       runner.expectResults(
-        {
-          name: 'renders scope even if value is not used',
-          code: '{{ with $parameter }}Hello world!{{ end }}',
-          args: (args) => args
-            .withArgument('parameter', 'Hello'),
-          expected: ['Hello world!'],
-        },
-        {
-          name: 'renders value when it has value',
-          code: '{{ with $parameter }}{{ . }} world!{{ end }}',
-          args: (args) => args
-            .withArgument('parameter', 'Hello'),
-          expected: ['Hello world!'],
-        },
-        {
-          name: 'renders value when whitespaces around brackets are missing',
-          code: '{{ with $parameter }}{{.}} world!{{ end }}',
-          args: (args) => args
-            .withArgument('parameter', 'Hello'),
-          expected: ['Hello world!'],
-        },
-        {
-          name: 'renders value multiple times when it\'s used multiple times',
-          code: '{{ with $letterL }}He{{ . }}{{ . }}o wor{{ . }}d!{{ end }}',
-          args: (args) => args
-            .withArgument('letterL', 'l'),
-          expected: ['Hello world!'],
-        },
         {
           name: 'renders value in multi-lined text',
           code: '{{ with $middleLine }}line before value\n{{ . }}\nline after value{{ end }}',
@@ -145,42 +196,71 @@ describe('WithParser', () => {
             .withArgument('middleLine', 'value line'),
           expected: ['line before value\nvalue line\nline after value'],
         },
+        {
+          name: 'does not render trailing whitespace after value',
+          code: '{{ with $parameter }}{{ . }}! {{ end }}',
+          args: (args) => args
+            .withArgument('parameter', 'Hello world'),
+          expected: ['Hello world!'],
+        },
+        {
+          name: 'does not render trailing newline after value',
+          code: '{{ with $parameter }}{{ . }}!\r\n{{ end }}',
+          args: (args) => args
+            .withArgument('parameter', 'Hello world'),
+          expected: ['Hello world!'],
+        },
+        {
+          name: 'does not render leading newline before value',
+          code: '{{ with $parameter }}\r\n{{ . }}!{{ end }}',
+          args: (args) => args
+            .withArgument('parameter', 'Hello world'),
+          expected: ['Hello world!'],
+        },
+        {
+          name: 'does not render leading whitespaces before value',
+          code: '{{ with $parameter }}  {{ . }}!{{ end }}',
+          args: (args) => args
+            .withArgument('parameter', 'Hello world'),
+          expected: ['Hello world!'],
+        },
+        {
+          name: 'does not render leading newline and whitespaces before value',
+          code: '{{ with $parameter }}\r\n  {{ . }}!{{ end }}',
+          args: (args) => args
+            .withArgument('parameter', 'Hello world'),
+          expected: ['Hello world!'],
+        },
+      );
+    });
+    describe('nested with statements', () => {
+      runner.expectResults(
+        {
+          name: 'renders nested with statements correctly',
+          code: '{{ with $outer }}Outer: {{ with $inner }}Inner: {{ . }}{{ end }}, Outer again: {{ . }}{{ end }}',
+          args: (args) => args
+            .withArgument('outer', 'OuterValue')
+            .withArgument('inner', 'InnerValue'),
+          expected: [
+            'Inner: InnerValue',
+            'Outer: {{ with $inner }}Inner: {{ . }}{{ end }}, Outer again: OuterValue',
+          ],
+        },
+        {
+          name: 'renders nested with statements with context variables',
+          code: '{{ with $outer }}{{ with $inner }}{{ . }}{{ . }}{{ end }}{{ . }}{{ end }}',
+          args: (args) => args
+            .withArgument('outer', 'O')
+            .withArgument('inner', 'I'),
+          expected: [
+            'II',
+            '{{ with $inner }}{{ . }}{{ . }}{{ end }}O',
+          ],
+        },
       );
     });
   });
-  describe('ignores trailing and leading whitespaces and newlines inside scope', () => {
-    runner.expectResults(
-      {
-        name: 'does not render trailing whitespace after value',
-        code: '{{ with $parameter }}{{ . }}! {{ end }}',
-        args: (args) => args
-          .withArgument('parameter', 'Hello world'),
-        expected: ['Hello world!'],
-      },
-      {
-        name: 'does not render trailing newline after value',
-        code: '{{ with $parameter }}{{ . }}!\r\n{{ end }}',
-        args: (args) => args
-          .withArgument('parameter', 'Hello world'),
-        expected: ['Hello world!'],
-      },
-      {
-        name: 'does not render leading newline before value',
-        code: '{{ with $parameter }}\r\n{{ . }}!{{ end }}',
-        args: (args) => args
-          .withArgument('parameter', 'Hello world'),
-        expected: ['Hello world!'],
-      },
-      {
-        name: 'does not render leading whitespace before value',
-        code: '{{ with $parameter }}  {{ . }}!{{ end }}',
-        args: (args) => args
-          .withArgument('parameter', 'Hello world'),
-        expected: ['Hello world!'],
-      },
-    );
-  });
-  describe('compiles pipes in scope as expected', () => {
+  describe('pipe behavior', () => {
     runner.expectPipeHits({
       codeBuilder: (pipeline) => `{{ with $argument }} {{ .${pipeline}}} {{ end }}`,
       parameterName: 'argument',