Fix #6: Add eager/non-throwing validation for expression syntax errors

src/Serilog.Expressions/Expressions/Compilation/ExpressionValidationException.cs

@@ -0,0 +1,26 @@
+// Copyright © Serilog Contributors
+//
+// Licensed under the Apache License, Version 2.0 (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.apache.org/licenses/LICENSE-2.0
+//
+// 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.
+
+namespace Serilog.Expressions.Compilation;
+
+class ExpressionValidationException : ArgumentException
+{
+    public ExpressionValidationException(string message) : base(message)
+    {
+    }
+
+    public ExpressionValidationException(string message, Exception innerException) : base(message, innerException)
+    {
+    }
+}

src/Serilog.Expressions/Expressions/Compilation/Linq/LinqExpressionCompiler.cs

@@ -16,6 +16,7 @@
 using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Text;
+using Serilog.Debugging;
 using Serilog.Events;
 using Serilog.Expressions.Ast;
 using Serilog.Expressions.Compilation.Transformations;
@@ -99,12 +100,24 @@ ExpressionBody Splice(Expression<Evaluatable> lambda)
     protected override ExpressionBody Transform(CallExpression call)
     {
         if (!_nameResolver.TryResolveFunctionName(call.OperatorName, out var m))
-            throw new ArgumentException($"The function name `{call.OperatorName}` was not recognized.");
+            throw new ExpressionValidationException($"The function name `{call.OperatorName}` was not recognized.");
 
         var methodParameters = m.GetParameters()
             .Select(info => (pi: info, optional: info.GetCustomAttribute<OptionalAttribute>() != null))
             .ToList();
 
+        // Log warning for CI modifier usage on functions that don't support it
+        // Note: We log a warning rather than throwing to maintain backward compatibility
+        // Previously, invalid CI usage was silently ignored
+        if (call.IgnoreCase)
+        {
+            var supportsStringComparison = methodParameters.Any(p => p.pi.ParameterType == typeof(StringComparison));
+            if (!supportsStringComparison)
+            {
+                SelfLog.WriteLine($"The function `{call.OperatorName}` does not support case-insensitive operation; the 'ci' modifier will be ignored.");
+            }
+        }
+
         var allowedParameters = methodParameters.Where(info => info.pi.ParameterType == typeof(LogEventPropertyValue)).ToList();
         var requiredParameterCount = allowedParameters.Count(info => !info.optional);
 

src/Serilog.Expressions/Expressions/Compilation/Text/TextMatchingTransformer.cs

@@ -51,14 +51,21 @@ Expression TryCompileIndexOfMatch(bool ignoreCase, Expression corpus, Expression
     {
         if (regex is ConstantExpression { Constant: ScalarValue { Value: string s } })
         {
-            var opts = RegexOptions.Compiled | RegexOptions.ExplicitCapture;
-            if (ignoreCase)
-                opts |= RegexOptions.IgnoreCase;
-            var compiled = new Regex(s, opts, TimeSpan.FromMilliseconds(100));
-            return new IndexOfMatchExpression(Transform(corpus), compiled);
+            try
+            {
+                var opts = RegexOptions.Compiled | RegexOptions.ExplicitCapture;
+                if (ignoreCase)
+                    opts |= RegexOptions.IgnoreCase;
+                var compiled = new Regex(s, opts, TimeSpan.FromMilliseconds(100));
+                return new IndexOfMatchExpression(Transform(corpus), compiled);
+            }
+            catch (ArgumentException ex)
+            {
+                throw new ExpressionValidationException($"Invalid regular expression in IndexOfMatch: {ex.Message}", ex);
+            }
         }
 
-        SelfLog.WriteLine($"Serilog.Expressions: `IndexOfMatch()` requires a constant string regular expression argument; found ${regex}.");
+        SelfLog.WriteLine($"Serilog.Expressions: `IndexOfMatch()` requires a constant string regular expression argument; found {regex}.");
         return new CallExpression(false, Operators.OpUndefined);
     }
 }

src/Serilog.Expressions/Expressions/SerilogExpression.cs

@@ -53,8 +53,8 @@ public static CompiledExpression Compile(string expression,
     /// <param name="result">A function that evaluates the expression in the context of a log event.</param>
     /// <param name="error">The reported error, if compilation was unsuccessful.</param>
     /// <returns>True if the function could be created; otherwise, false.</returns>
-    /// <remarks>Regular expression syntax errors currently generate exceptions instead of producing friendly
-    /// errors.</remarks>
+    /// <remarks>Validation errors including invalid regular expressions and unknown function names are returned
+    /// as friendly error messages. Invalid case-insensitive modifiers are ignored with warnings.</remarks>
     public static bool TryCompile(
         string expression,
         [MaybeNullWhen(false)] out CompiledExpression result,
@@ -75,8 +75,8 @@ public static bool TryCompile(
     /// <param name="result">A function that evaluates the expression in the context of a log event.</param>
     /// <param name="error">The reported error, if compilation was unsuccessful.</param>
     /// <returns>True if the function could be created; otherwise, false.</returns>
-    /// <remarks>Regular expression syntax errors currently generate exceptions instead of producing friendly
-    /// errors.</remarks>
+    /// <remarks>Validation errors including invalid regular expressions and unknown function names are returned
+    /// as friendly error messages. Invalid case-insensitive modifiers are ignored with warnings.</remarks>
     public static bool TryCompile(string expression,
         IFormatProvider? formatProvider,
         NameResolver nameResolver,
@@ -101,10 +101,20 @@ static bool TryCompileImpl(string expression,
             return false;
         }
 
-        var evaluate = ExpressionCompiler.Compile(root, formatProvider, DefaultFunctionNameResolver.Build(nameResolver));
-        result = evt => evaluate(new(evt));
-        error = null;
-        return true;
+        try
+        {
+            var evaluate = ExpressionCompiler.Compile(root, formatProvider, DefaultFunctionNameResolver.Build(nameResolver));
+            result = evt => evaluate(new(evt));
+            error = null;
+            return true;
+        }
+        catch (ExpressionValidationException ex)
+        {
+            // Catch validation errors from compilation
+            result = null;
+            error = ex.Message;
+            return false;
+        }
     }
 
     /// <summary>

test/Serilog.Expressions.Tests/ExpressionValidationTests.cs

@@ -0,0 +1,184 @@
+using Xunit;
+
+namespace Serilog.Expressions.Tests;
+
+public class ExpressionValidationTests
+{
+    [Theory]
+    [InlineData("IsMatch(Name, '[invalid')", "Invalid regular expression")]
+    [InlineData("IndexOfMatch(Text, '(?<')", "Invalid regular expression")]
+    [InlineData("IsMatch(Name, '(?P<name>)')", "Invalid regular expression")]
+    [InlineData("IsMatch(Name, '(unclosed')", "Invalid regular expression")]
+    [InlineData("IndexOfMatch(Text, '*invalid')", "Invalid regular expression")]
+    public void InvalidRegularExpressionsAreReportedGracefully(string expression, string expectedErrorFragment)
+    {
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+        Assert.False(result);
+        Assert.Contains(expectedErrorFragment, error);
+        Assert.Null(compiled);
+    }
+
+    [Theory]
+    [InlineData("UnknownFunction()", "The function name `UnknownFunction` was not recognized.")]
+    [InlineData("foo(1, 2, 3)", "The function name `foo` was not recognized.")]
+    [InlineData("MyCustomFunc(Name)", "The function name `MyCustomFunc` was not recognized.")]
+    [InlineData("notAFunction()", "The function name `notAFunction` was not recognized.")]
+    public void UnknownFunctionNamesAreReportedGracefully(string expression, string expectedError)
+    {
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+        Assert.False(result);
+        Assert.Equal(expectedError, error);
+        Assert.Null(compiled);
+    }
+
+    [Theory]
+    [InlineData("Length(Name) ci")]
+    [InlineData("Round(Value, 2) ci")]
+    [InlineData("Now() ci")]
+    [InlineData("TypeOf(Value) ci")]
+    [InlineData("IsDefined(Prop) ci")]
+    public void InvalidCiModifierUsageCompilesWithWarning(string expression)
+    {
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+        Assert.True(result, $"Failed to compile: {error}");
+        Assert.NotNull(compiled);
+        Assert.Null(error);
+    }
+
+    [Theory]
+    [InlineData("Contains(Name, 'test') ci")]
+    [InlineData("StartsWith(Path, '/api') ci")]
+    [InlineData("EndsWith(File, '.txt') ci")]
+    [InlineData("IsMatch(Email, '@example') ci")]
+    [InlineData("IndexOfMatch(Text, 'pattern') ci")]
+    [InlineData("IndexOf(Name, 'x') ci")]
+    [InlineData("Name = 'test' ci")]
+    [InlineData("Name <> 'test' ci")]
+    [InlineData("Name like '%test%' ci")]
+    public void ValidCiModifierUsageCompiles(string expression)
+    {
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+        Assert.True(result, $"Failed to compile: {error}");
+        Assert.NotNull(compiled);
+        Assert.Null(error);
+    }
+
+    [Fact]
+    public void FirstErrorIsReportedInComplexExpressions()
+    {
+        var expression = "UnknownFunc() and Length(Value) > 5";
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+
+        Assert.False(result);
+        Assert.Null(compiled);
+
+        // Should report the first error encountered
+        Assert.Contains("UnknownFunc", error);
+        Assert.NotNull(error);
+    }
+
+    [Fact]
+    public void ValidExpressionsStillCompileWithoutErrors()
+    {
+        var validExpressions = new[]
+        {
+            "IsMatch(Name, '^[A-Z]')",
+            "IndexOfMatch(Text, '\\d+')",
+            "Contains(Name, 'test') ci",
+            "Length(Items) > 5",
+            "Round(Value, 2)",
+            "TypeOf(Data) = 'String'",
+            "Name like '%test%'",
+            "StartsWith(Path, '/') and EndsWith(Path, '.json')"
+        };
+
+        foreach (var expr in validExpressions)
+        {
+            var result = SerilogExpression.TryCompile(expr, out var compiled, out var error);
+            Assert.True(result, $"Failed to compile: {expr}. Error: {error}");
+            Assert.NotNull(compiled);
+            Assert.Null(error);
+        }
+    }
+
+    [Fact]
+    public void CompileMethodStillThrowsForInvalidExpressions()
+    {
+        // Ensure Compile() method (not TryCompile) maintains throwing behavior for invalid expressions
+        Assert.Throws<ArgumentException>(() => 
+            SerilogExpression.Compile("UnknownFunction()"));
+
+        Assert.Throws<ArgumentException>(() => 
+            SerilogExpression.Compile("IsMatch(Name, '[invalid')"));
+
+        // CI modifier on non-supporting functions compiles with warning
+        var compiledWithCi = SerilogExpression.Compile("Length(Name) ci");
+        Assert.NotNull(compiledWithCi);
+
+        Assert.Throws<ArgumentException>(() => 
+            SerilogExpression.Compile("IndexOfMatch(Text, '(?<')"));
+    }
+
+    [Theory]
+    [InlineData("IsMatch(Name, Name)")] // Non-constant pattern
+    [InlineData("IndexOfMatch(Text, Value)")] // Non-constant pattern
+    public void NonConstantRegexPatternsHandledGracefully(string expression)
+    {
+        // These should compile but may log warnings (not errors)
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+
+        // These compile successfully but return undefined at runtime
+        Assert.True(result);
+        Assert.NotNull(compiled);
+        Assert.Null(error);
+    }
+
+    [Fact]
+    public void RegexTimeoutIsRespected()
+    {
+        // This regex should compile fine - timeout only matters at runtime
+        var expression = @"IsMatch(Text, '(a+)+b')";
+
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+
+        Assert.True(result);
+        Assert.NotNull(compiled);
+        Assert.Null(error);
+    }
+
+    [Fact]
+    public void ComplexExpressionsReportFirstError()
+    {
+        var expression = "UnknownFunc1() or Length(Value) > 5";
+        var result = SerilogExpression.TryCompile(expression, out var compiled, out var error);
+
+        Assert.False(result);
+        Assert.Null(compiled);
+        Assert.NotNull(error);
+
+        // Should report the first error encountered during compilation
+        Assert.Contains("UnknownFunc1", error);
+    }
+
+    [Fact]
+    public void BackwardCompatibilityPreservedForInvalidCiUsage()
+    {
+        // These previously compiled (CI was silently ignored)
+        // They should still compile with the new changes
+        var expressions = new[]
+        {
+            "undefined() ci",
+            "null = undefined() ci",
+            "Length(Name) ci",
+            "Round(Value, 2) ci"
+        };
+
+        foreach (var expr in expressions)
+        {
+            var result = SerilogExpression.TryCompile(expr, out var compiled, out var error);
+            Assert.True(result, $"Breaking change detected: {expr} no longer compiles. Error: {error}");
+            Assert.NotNull(compiled);
+            Assert.Null(error);
+        }
+    }
+}