Fix #4599: EnumerableContains.Parse crash on programmatic receivers with custom ToString()#4600
Merged
Merged
Conversation
59c8125 to
a5848d2
Compare
…heck
EnumerableContains.Parse (and the HashSet sibling) crashed for receivers built
as Property(Constant(<wrapper>), "<member>") when the wrapper type overrides
ToString() -- e.g. HotChocolate's ExpressionParameter<T> driving [UseFiltering]
`in`. ConstantExpression.ToString() only wraps in "value(<TypeName>)" when the
value uses the default Object.ToString(); with a custom override, it prints the
custom string directly. The IsCompilableExpression heuristic
(node.Expression.ToString().StartsWith("value(")) then returned false, the
receiver was wrongly rejected by TryToParseConstant, and the parser fell into
ParseWhereForContains -> ReduceToConstant on the *value-side* expression. That
expression still referenced the outer Where lambda parameter, so FEC compiled
a parameter-less lambda with a free parameter inside and crashed with
"variable 'x' of type '<Doc>' referenced from scope '', but it is not defined".
- IsCompilableExpression: walk the member chain to its root and accept if the
root is a ConstantExpression (or null, for static members). Structural; no
string heuristic. Accepts the previously-rejected programmatic-receiver shape.
- ReduceToConstant: add a free-parameter guard (visitor that ignores parameters
bound by lambdas *within* the expression). If a free parameter is found,
throw a clear BadLinqExpressionException naming it rather than letting FEC
surface the confusing scope-binding message. Safety net for any other call
site that might mistakenly feed a parameter-bearing subtree.
Tests cover both Enumerable.Contains and HashSet<T>.Contains receiver shapes
against Postgres; the previously-failing repros now pass. Full
LinqTests Contains/Where/GroupBy/Any/Modulo sweep (437 tests) still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a5848d2 to
b05a792
Compare
This was referenced Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4599.
Problem
EnumerableContains.Parse(and theHashSetEnumerableContainssibling) crashed forContains()receivers built asProperty(Constant(<wrapper>), \"<member>\")when the wrapper type overridesToString()— e.g. HotChocolate'sExpressionParameter<T>driving[UseFiltering]in. The stack:Root cause:
ConstantExpression.ToString()only wraps a value in\"value(<TypeName>)\"when the value uses the defaultObject.ToString(). When the wrapper overridesToString(), the custom string is printed directly, andIsCompilableExpression'snode.Expression.ToString().StartsWith(\"value(\")heuristic returns false. The receiver is then wrongly rejected byTryToParseConstant, andEnumerableContains.Parsefalls intoParseWhereForContains→ReduceToConstant(Arguments.Last())— butArguments.Last()is the value-side expression (doc.Name), which references the outerWherelambda parameter.ReduceToConstantwraps it in a parameter-less lambda and asks FEC to compile, which crashes with the unbound-parameter message.The crash is on the value side; the trigger is the receiver being wrongly rejected.
Fix
IsCompilableExpression: replace theToString().StartsWith(\"value(\")heuristic with a structural check — walk theMemberExpression's.Expressionchain to its root, accept if it bottoms out at aConstantExpression(ornull, for static members). No string heuristic. This correctly accepts programmatic receivers regardless ofToString()behavior.ReduceToConstant: add a free-parameter guard — a smallExpressionVisitorthat finds anyParameterExpressionnot bound by a lambda within the expression. If found, throw a clearBadLinqExpressionExceptionnaming the free parameter instead of letting FEC surface the confusing scope-binding error. Defensive safety net for any future code path that mistakenly feeds a parameter-bearing subtree in.After this change the receiver routes through
EnumerableContains.Parse's constant-receiver branch and the query compiles toWHERE name = ANY(\$1)— same SQL as the manualExpression.Constant(List<T>)workaround.Validation
Bug_4599_enumerable_contains_programmatic_receivercover bothEnumerable.ContainsandHashSet<T>.Containsreceiver shapes against Postgres. Without the fix: both crash with the exact stack above. With the fix: both pass and return correct results.IsCompilableExpression/TryToParseConstantheuristic has 8+ call sites (Where, GroupBy, Any, Modulo, Dictionary, MemoryExtensions, Contains). Ran the full sweep — 437 / 437 LINQ tests pass, no regressions.Backport
I'll open a companion PR cherry-picking this commit to the
8.0branch — the reporter is on 8.37.1 and the bug exists there with the same code.