[8.0 backport] Fix #4599: EnumerableContains.Parse crash on programmatic receivers with custom ToString()

src/LinqTests/Bugs/Bug_4599_enumerable_contains_programmatic_receiver.cs

@@ -0,0 +1,129 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
+using System.Threading.Tasks;
+using Marten;
+using Marten.Testing.Harness;
+using Shouldly;
+
+namespace LinqTests.Bugs;
+
+// Top-level types so the generated mt_doc_* table name stays under the 63-byte
+// PostgreSQL identifier limit (the test class name itself is long).
+//
+// Note: the ToString() override is the trigger. ConstantExpression.ToString() wraps
+// a value in "value(<TypeName>)" only when the value's ToString() is the default
+// Object.ToString(); when the type overrides ToString(), the custom string is used
+// directly. IsCompilableExpression's "value(" string-prefix check then returns false
+// and the receiver is wrongly rejected. HotChocolate's ExpressionParameter<T> has a
+// custom ToString() for diagnostics, which is why their `[UseFiltering]` `in`
+// operator triggers this in practice.
+public sealed class Bug4599ValueHolder<T>
+{
+    public IEnumerable<T> p { get; }
+    public Bug4599ValueHolder(IEnumerable<T> values) => p = values;
+    public override string ToString() => $"Bug4599ValueHolder[{string.Join(',', p)}]";
+}
+
+public sealed class Bug4599HashSetHolder<T>
+{
+    public HashSet<T> p { get; }
+    public Bug4599HashSetHolder(HashSet<T> values) => p = values;
+    public override string ToString() => $"Bug4599HashSetHolder[{string.Join(',', p)}]";
+}
+
+public class Bug4599Doc
+{
+    public Guid Id { get; set; }
+    public string Name { get; set; } = "";
+}
+
+// https://github.com/JasperFx/marten/issues/4599
+//
+// EnumerableContains.Parse crashes with
+//   "variable '<x>' of type '<Document>' referenced from scope '', but it is not defined"
+// when the Contains() receiver is built programmatically as
+// Property(Constant(<wrapper>), "<member>") and the wrapper overrides ToString().
+// HotChocolate's [UseFiltering] `in` operator is one consumer that produces this
+// shape via FilterExpressionBuilder.CreateAndConvertParameter.
+//
+// Each test uses its own DocumentStore with a unique schema so that parallel
+// multi-target test runs (net8.0 / net9.0 / net10.0) don't interfere through a
+// shared mt_doc_bug4599doc table.
+public class Bug_4599_enumerable_contains_programmatic_receiver
+{
+    private static IDocumentStore BuildStore(string schema)
+    {
+        // Include the runtime major version so parallel multi-target test runs
+        // (net8.0 / net9.0 / net10.0) don't collide on the same schema in the
+        // shared marten_testing DB.
+        var suffix = Environment.Version.Major;
+        return DocumentStore.For(opts =>
+        {
+            opts.Connection(ConnectionSource.ConnectionString);
+            opts.DatabaseSchemaName = $"{schema}{suffix}";
+        });
+    }
+
+    private static async Task storeThreeDocs(IDocumentStore store)
+    {
+        await store.Advanced.Clean.CompletelyRemoveAllAsync();
+        await using var session = store.LightweightSession();
+        session.Store(new Bug4599Doc { Id = Guid.NewGuid(), Name = "a" });
+        session.Store(new Bug4599Doc { Id = Guid.NewGuid(), Name = "b" });
+        session.Store(new Bug4599Doc { Id = Guid.NewGuid(), Name = "c" });
+        await session.SaveChangesAsync();
+    }
+
+    [Fact]
+    public async Task enumerable_contains_with_programmatic_receiver_should_query_correctly()
+    {
+        await using var store = BuildStore("b4599e");
+        await storeThreeDocs(store);
+
+        var holder = new Bug4599ValueHolder<string>(new[] { "a", "b" });
+
+        // Build: doc => holder.p.Contains(doc.Name)
+        // -- the receiver is Property(Constant(holder), "p"), NOT a compiler-emitted closure.
+        var doc = Expression.Parameter(typeof(Bug4599Doc), "doc");
+        var nameAccess = Expression.Property(doc, nameof(Bug4599Doc.Name));
+        var receiver = Expression.Property(Expression.Constant(holder), nameof(Bug4599ValueHolder<string>.p));
+        var containsCall = Expression.Call(
+            typeof(Enumerable),
+            nameof(Enumerable.Contains),
+            [typeof(string)],
+            receiver,
+            nameAccess);
+        var predicate = Expression.Lambda<Func<Bug4599Doc, bool>>(containsCall, doc);
+
+        await using var query = store.QuerySession();
+        var results = await query.Query<Bug4599Doc>().Where(predicate).ToListAsync();
+
+        results.Select(x => x.Name).OrderBy(x => x)
+            .ShouldHaveTheSameElementsAs("a", "b");
+    }
+
+    [Fact]
+    public async Task hashset_contains_with_programmatic_receiver_should_query_correctly()
+    {
+        await using var store = BuildStore("b4599h");
+        await storeThreeDocs(store);
+
+        var holder = new Bug4599HashSetHolder<string>(new HashSet<string> { "a", "b" });
+
+        // Build: doc => holder.p.Contains(doc.Name) using HashSet<T>.Contains (instance method)
+        var doc = Expression.Parameter(typeof(Bug4599Doc), "doc");
+        var nameAccess = Expression.Property(doc, nameof(Bug4599Doc.Name));
+        var receiver = Expression.Property(Expression.Constant(holder), nameof(Bug4599HashSetHolder<string>.p));
+        var containsMethod = typeof(HashSet<string>).GetMethod(nameof(HashSet<string>.Contains), [typeof(string)])!;
+        var containsCall = Expression.Call(receiver, containsMethod, nameAccess);
+        var predicate = Expression.Lambda<Func<Bug4599Doc, bool>>(containsCall, doc);
+
+        await using var query = store.QuerySession();
+        var results = await query.Query<Bug4599Doc>().Where(predicate).ToListAsync();
+
+        results.Select(x => x.Name).OrderBy(x => x)
+            .ShouldHaveTheSameElementsAs("a", "b");
+    }
+}

src/Marten/Linq/Parsing/LinqInternalExtensions.cs

@@ -205,11 +205,24 @@ public static ISqlFragment ReduceToValue(this IQueryableMemberCollection collect
 
     public static bool IsCompilableExpression(this MemberExpression node)
     {
-        // Field of the containing code
+        // A MemberExpression is reducible to a constant at parse time iff its inner
+        // subtree contains no free ParameterExpression -- which correctly covers both
+        // compiler-emitted closure captures (Property(Constant(<displayClass>), "p"))
+        // *and* shapes whose inner is a MethodCall over closure-captured values
+        // (e.g. Member(Call(ElementAt, [Member(Constant(<displayClass>), "list"),
+        // Constant(2)]), "Date")).
+        //
+        // The previous heuristic gated on `node.Expression.ToString().StartsWith("value(")`,
+        // which incidentally covered MethodCall shapes whose printed form started with
+        // the closure access, *but* missed programmatic receivers whose wrapper type
+        // overrides ToString() (the #4599 trigger). The "no free parameter" structural
+        // check is the precise predicate the heuristic was approximating and handles
+        // both cases correctly.
         if (node.Expression == null) return true;
 
-        return (node.Expression is ConstantExpression || node.Expression != null) &&
-               node.Expression.ToString().StartsWith("value(");
+        var finder = new FreeParameterFinder();
+        finder.Visit(node.Expression);
+        return finder.Found == null;
     }
 
     public static bool TryToParseConstant(this Expression? expression, [NotNullWhen(true)]out ConstantExpression? constant)
@@ -337,6 +350,22 @@ public static ConstantExpression ReduceToConstant(this Expression expression)
             return constantExpression;
         }
 
+        // Guard: if the expression references a free ParameterExpression (one not
+        // bound by a lambda *inside* the expression), wrapping it in our parameter-less
+        // Lambda<Func<object>> below produces a body with unbound parameters, and
+        // FastExpressionCompiler crashes with the confusing
+        //   "variable 'x' of type 'Doc' referenced from scope '', but it is not defined"
+        // message. That symptom (see #4599) is almost always a Linq-parser bug at the
+        // call site; surface a clear error instead of leaking the FEC message.
+        var freeParameterFinder = new FreeParameterFinder();
+        freeParameterFinder.Visit(expression);
+        if (freeParameterFinder.Found != null)
+        {
+            var p = freeParameterFinder.Found;
+            throw new BadLinqExpressionException(
+                $"Marten cannot reduce the expression '{expression}' to a constant because it references the free parameter '{p.Name}' of type '{p.Type.Name}'. This usually indicates a Linq parser issue at the call site.");
+        }
+
         var lambdaWithoutParameters = Expression.Lambda<Func<object>>(Expression.Convert(expression, typeof(object)));
         var compiledLambda = FastExpressionCompiler.ExpressionCompiler.CompileFast(lambdaWithoutParameters);
 
@@ -352,4 +381,35 @@ public static ConstantExpression ReduceToConstant(this Expression expression)
                 "Error while trying to find a value for the Linq expression " + expression, e);
         }
     }
+
+    /// <summary>
+    /// Walks an expression looking for the first <see cref="ParameterExpression"/>
+    /// that is *not* bound by a lambda inside the expression itself. Used by
+    /// <see cref="ReduceToConstant"/> as a safety net against feeding parameter-bearing
+    /// subtrees into the parameter-less lambda + FEC compile path (see #4599).
+    /// </summary>
+    private sealed class FreeParameterFinder: ExpressionVisitor
+    {
+        private readonly HashSet<ParameterExpression> _bound = new();
+
+        public ParameterExpression? Found { get; private set; }
+
+        protected override Expression VisitLambda<T>(Expression<T> node)
+        {
+            foreach (var p in node.Parameters) _bound.Add(p);
+            var result = base.VisitLambda(node);
+            foreach (var p in node.Parameters) _bound.Remove(p);
+            return result;
+        }
+
+        protected override Expression VisitParameter(ParameterExpression node)
+        {
+            if (Found == null && !_bound.Contains(node))
+            {
+                Found = node;
+            }
+
+            return node;
+        }
+    }
 }