[8.0 backport] Fix #4599: EnumerableContains.Parse crash on programmatic receivers with custom ToString()#4601
Merged
Conversation
…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>
jeremydmiller
force-pushed
the
backport/4599-enumerablecontains-receiver-8.0
branch
from
2181cc0 to
1cc2cf6
Compare
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
1 participant
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.
Backport of #4600 to the `8.0` branch. The reporter is on 8.37.1 and the same bug exists there with the same code.
Cherry-pick of commit `a5848d272` (the master fix) — applied cleanly with the auto-merge.
Summary
`EnumerableContains.Parse` and `HashSetEnumerableContains.Parse` crashed for `Contains()` receivers built as `Property(Constant(), "")` when the wrapper type overrides `ToString()`. Triggered by HotChocolate's `[UseFiltering]` `in` operator, which builds receivers that way via `FilterExpressionBuilder.CreateAndConvertParameter`. The crash: `FastExpressionCompiler` throwing `variable 'x' of type '' referenced from scope '', but it is not defined`.
Fix:
See #4600 for the full discussion.
Validation on the `8.0` branch
🤖 Generated with Claude Code