- (Lava) Fixed parsing errors caused by nested Lava comments inside '{% lava %}' tag blocks.

nairdo · nairdo · commit a8c3f8b559e6 · 2025-11-17T17:42:21.000-07:00
diff --git a/Rock.Lava.Fluid/FluidEngine.cs b/Rock.Lava.Fluid/FluidEngine.cs
@@ -606,7 +606,7 @@ private FluidTemplate CreateNewFluidTemplate( string lavaTemplate )
             IFluidTemplate fluidTemplate;
 
             /*
-                10/27/2025 - NA
+                10/27/2025 - N.A.
 
                 Added ConvertToLiquidElsif method while having to remove our custom RegisterLavaElseIfTag() in our
                 custom LavaFluidParser in order to get shortcodes working inside of {% lava %} blocks. It might
@@ -617,7 +617,14 @@ with more effort.
 
                 Reason: Ensures proper parsing of conditional blocks when {% lava %} tags and shortcodes are present.
             */
-            var success = _parser.TryParse( ConvertToLiquidElsif( lavaTemplate ), out fluidTemplate, out error );
+
+            /*
+                11/17/2025 - N.A.
+
+                Changed from calling ConvertToLiquidElsif(...) to use the unused ConvertToLiquid(...) method since
+                it handles both the "elseif" conversion and uses our RemoveLavaComments() method.
+            */
+            var success = _parser.TryParse( ConvertToLiquid( lavaTemplate ), out fluidTemplate, out error );
 
             var fluidTemplateObject = ( FluidTemplate ) fluidTemplate;
 
diff --git a/Rock.Lava/Engine/LavaToLiquidTemplateConverter.cs b/Rock.Lava/Engine/LavaToLiquidTemplateConverter.cs
@@ -44,8 +44,6 @@ public string ConvertToLiquid( string lavaTemplateText )
             string liquidTemplateText;
 
             liquidTemplateText = RemoveLavaComments( lavaTemplateText );
-
-            liquidTemplateText = ReplaceTemplateShortcodes( liquidTemplateText );
             liquidTemplateText = ReplaceElseIfKeyword( liquidTemplateText );
 
             return liquidTemplateText;
diff --git a/Rock.Tests.Performance/Benchmarks/Lava/LavaParsingPerformance.cs b/Rock.Tests.Performance/Benchmarks/Lava/LavaParsingPerformance.cs
@@ -12,7 +12,7 @@ namespace Rock.Tests.Performance.Benchmarks.Security
     /// Performs some basic performance tests on the...
     /// class. TL;DR; It's fast.
     /// </summary>
-    [MemoryDiagnoser( false )]
+    [MemoryDiagnoser( true )]
     [Attributes.OperationsPerSecondColumn]
     [GroupBenchmarksBy( BenchmarkLogicalGroupRule.ByCategory )]
     [CategoriesColumn]
@@ -22,7 +22,7 @@ public class LavaParsingPerformance
 
         #region Test Data
 
-        private readonly string _template = @"
+        private readonly string _templateIfElseIf = @"
 {% liquid
 
     assign NickName = 'Ted'
@@ -43,10 +43,24 @@ public class LavaParsingPerformance
     echo result
 %}
 ";
-        private readonly string _expectedOutput = "male";
+        private readonly string _expectedOutputIfElseIf = "male";
 
-        #endregion
+        private readonly string _templateWithComments = @"
+{% liquid
 
+    //- Comment level one
+    assign isTest = true
+    if isTest
+        
+        //- Comment level two
+        assign isTest = false
+        
+    endif
+    echo isTest
+%}
+";
+        private readonly string _expectedOutputWithComments = "false";
+        #endregion
 
         [GlobalSetup]
         public void Setup()
@@ -59,25 +73,39 @@ public void Setup()
             fluidEngine.Initialize( engineOptions );
             fluidEngine.RegisterFilters( typeof( Rock.Lava.Filters.TemplateFilters ) );
 
-            // Verify that it's working as expected.
-            var output = fluidEngine.ParseTemplate( _template );
+            // Verify both templates are working as expected.
+            var output = fluidEngine.ParseTemplate( _templateIfElseIf );
             if ( output.HasErrors != false )
             {
                 throw new System.Exception( "Lava engine setup failed: the template has errors." );
             }
 
             var renderResult = fluidEngine.RenderTemplate( output.Template, new LavaRenderParameters() );
-            if ( output == null || renderResult.Text.Trim() != _expectedOutput )
+            if ( output == null || renderResult.Text.Trim() != _expectedOutputIfElseIf )
             {
-                throw new System.Exception( "Lava engine setup failed: unexpected output." );
+                throw new System.Exception( $"Lava engine setup failed: unexpected _expectedOutputIfElseIf output (was {renderResult.Text.Trim()})" );
+            }
+
+            output = fluidEngine.ParseTemplate( _templateWithComments );
+            renderResult = fluidEngine.RenderTemplate( output.Template, new LavaRenderParameters() );
+            if ( output == null || renderResult.Text.Trim() != _expectedOutputWithComments )
+            {
+                throw new System.Exception( $"Lava engine setup failed: unexpected _expectedOutputWithComments output (was {renderResult.Text.Trim()})." );
             }
         }
 
         [Benchmark]
-        [BenchmarkCategory( "Lava" )]
+        [BenchmarkCategory( "Lava ElseIf" )]
         public LavaParseResult ParseLavaIfElseIfTemplate()
         {
-            return fluidEngine.ParseTemplate( _template );
+            return fluidEngine.ParseTemplate( _templateIfElseIf );
+        }
+
+        [Benchmark]
+        [BenchmarkCategory( "Lava Comments" )]
+        public LavaParseResult ParseLavaTemplateWithComments()
+        {
+            return fluidEngine.ParseTemplate( _templateWithComments );
         }
 
     }
diff --git a/Rock.Tests/Lava/LiquidKeywordTests.cs b/Rock.Tests/Lava/LiquidKeywordTests.cs
@@ -604,13 +604,79 @@ echo result
 ";
             var expectedOutput = @"male";
 
-            TestHelper.AssertTemplateOutput( expectedOutput, template, mergeValues );
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, mergeValues, ignoreWhitespace: true );
+            } );
+
         }
 
+        /// <summary>
+        /// Verify that Rock's custom "//-" comments works inside an IF tag that is inside a {% lava %} {% liquid %} tag.
+        /// </summary>
+        [TestMethod]
+        public void LavaTag_WithInnerIfTagAndLineComments_IsProcessedCorrectly()
+        {
+            var template = @"
+{% liquid
+
+    //- Comment level one
+    assign isTest = true
+    if isTest
+        
+        //- Comment level two
+        assign isTest = false
+        
+    endif
+    echo isTest
+
+%}
+";
+            var expectedOutput = @"false";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
+        }
+
+
+        /// <summary>
+        /// Verify that Rock's custom "/- -/" block comments works inside an IF tag that is inside a {% lava %} {% liquid %} tag.
+        /// </summary>
+        [TestMethod]
+        public void LavaTag_WithCommentesNestedInIfTag_IsProcessedCorrectly()
+        {
+            var template = @"
+{% lava
+    //- Comment level one
+    assign isTest = true
+    if isTest
+        
+        /-
+        assign isTest = false
+        -/
+
+    endif
+    echo isTest
+%}
+";
+            var expectedOutput = @"true";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
+
+        }
+
+        /// <summary>
+        /// Verify that Rock's custom "/- -/" block comments works inside an IF tag that is inside a {% lava %} {% liquid %} tag.
+        /// </summary>
         [TestMethod]
         public void LavaTag_WithInnerIfTagAndBlockComment_SpanningMultipleLines_IsProcessedCorrectly()
         {
-            var input = @"
+            var template = @"
 {% liquid
     assign isTest = true
     /- This is a block comment...
@@ -623,10 +689,94 @@ ... spanning multiple lines. -/
     echo isTest
 %}
 ";
-
             var expectedOutput = @"false";
 
-            TestHelper.AssertTemplateOutput( expectedOutput, input );
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
+        }
+
+        /// <summary>
+        /// Verify that Rock's custom "//-" block comments works inside a FOR tag that is inside a {% lava %} {% liquid %} tag.
+        /// </summary>
+        [TestMethod]
+        public void LavaTag_WithSingleLineCommentInsideForTag_IsProcessedCorrectly()
+        {
+            var template = @"
+{% lava
+    //- Comment level one
+    assign test = -1
+    assign loopCount = 5
+    for i in (0..loopCount)
+        
+        //- Comment level two
+        assign test = i
+        
+    endfor
+    echo test
+%}
+";
+            var expectedOutput = @"5";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
+        }
+
+        /// <summary>
+        /// Verify that Rock's custom "//-" block comments works inside a FOR tag that is inside a {% lava %} {% liquid %} tag.
+        /// </summary>
+        [TestMethod]
+        public void LavaTag_WithCommentBlockInsideForTag_IsProcessedCorrectly()
+        {
+            var template = @"
+{% lava
+    //- Comment level one
+    assign test = -1
+    assign loopCount = 5
+    for i in (0..loopCount)
+        
+        /-
+        assign test = i
+        -/
+        
+    endfor
+    echo test
+%}
+";
+            var expectedOutput = @"-1";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
+        }
+
+        /// <summary>
+        /// Verify that Liquid's "#" comments works inside a {% lava %} {% liquid %} tag.
+        /// NOTE: This is not supported in Rock (or Fluid as far as I can tell).
+        /// </summary>
+        [TestMethod]
+        [Ignore] // Ignored because this is not currently supported in Fluid.
+        public void LavaTag_WithSingleLineHashComment_IsProcessedCorrectly()
+        {
+            var template = @"
+{% liquid
+    
+    # This is a comment
+    assign test = 5
+        
+    echo test
+%}
+";
+            var expectedOutput = @"5";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, template, ignoreWhitespace: true );
+            } );
         }
         #endregion
 
diff --git a/Rock.Tests/Lava/Tags/LavaShortcodeTagTests.cs b/Rock.Tests/Lava/Tags/LavaShortcodeTagTests.cs
@@ -0,0 +1,61 @@
+﻿// <copyright>
+// Copyright by the Spark Development Network
+//
+// Licensed under the Rock Community License (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.rockrms.com/license
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+//
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Rock.Lava;
+using Rock.Tests.Shared;
+
+namespace Rock.Tests.Lava.Tags
+{
+    /// <summary>
+    /// Verifies Lava shortcode behavior.
+    /// </summary>
+    [TestClass]
+    [TestCategory( TestFeatures.Lava )]
+    public class LavaShortcodeTagTests : LavaUnitTestBase
+    {
+        /// <summary>
+        /// A dynamic shortcode should produce the expected output.
+        /// </summary>
+        [TestMethod]
+        public void CustomDynamicLavaShortCode_ProducesExpectedOutput()
+        {
+            var shortcodeTemplate = @"
+{% assign x = 42 %}
+The answer is {{ x }}.
+";
+            // Create a new test shortcode with the "execute" command permission.
+            var shortcodeDefinition = new DynamicShortcodeDefinition();
+
+            shortcodeDefinition.ElementType = LavaShortcodeTypeSpecifier.Inline;
+            shortcodeDefinition.TemplateMarkup = shortcodeTemplate;
+            shortcodeDefinition.Name = "shortcode_execute";
+
+            var input = @"
+{[ shortcode_execute ]}
+";
+            var expectedOutput = "The answer is 42.";
+
+            TestHelper.ExecuteForActiveEngines( ( engine ) =>
+            {
+                engine.RegisterShortcode( shortcodeDefinition.Name, ( shortcodeName ) => { return shortcodeDefinition; } );
+
+                TestHelper.AssertTemplateOutput( engine, expectedOutput, input, ignoreWhitespace: true );
+            } );
+        }
+    }
+}
