From eb7b22d44ae2da899df4d4df2d1d69afb6e0994b Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 04:27:15 +0200 Subject: [PATCH 01/13] Test what queries hand to a LINQ provider Every query test so far runs over AsQueryable() and compares rows, so a tree that only LINQ to Objects can run passes. These scenarios also check the tree: the query must stay on its provider, and must not contain what a C# expression lambda never does - a block, a parameter nothing binds, a call into FSharp.Core, a join nested in a collection selector, a member read from a tuple built without naming its members. A scenario names what is wrong with its tree today. The name has to be removed by the change that makes it untrue, because a scenario fails when its tree is cleaner than it says. Co-Authored-By: Claude Fable 5.1 --- .../FSharp.Core.UnitTests.fsproj | 9 + .../QueryKeyScenarios.fs | 170 +++++++++ .../QueryMethodSyntaxScenarios.fs | 116 ++++++ .../QueryOperatorScenarios.fs | 197 ++++++++++ .../QueryProjectionScenarios.fs | 130 +++++++ .../QueryProviderShape.fs | 355 ++++++++++++++++++ .../Microsoft.FSharp.Linq/QueryScenario.fs | 189 ++++++++++ .../QueryScenarioDomain.fs | 109 ++++++ .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 31 ++ .../QueryStructureScenarios.fs | 268 +++++++++++++ 10 files changed, 1574 insertions(+) create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj index 494509d0906..3a20b60f7e4 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj +++ b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj @@ -92,6 +92,15 @@ + + + + + + + + + diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs new file mode 100644 index 00000000000..03011a4d99f --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs @@ -0,0 +1,170 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryKeyScenarios + +open System.Linq +open System.Linq.Expressions +open Microsoft.FSharp.Linq.RuntimeHelpers + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private customers (data: Data) = Seq.ofList data.Customers + +/// Orders paired with the lines that share both the order number and, as quantity, the customer number. +let private matchingLines (data: Data) = + seq { + for o in data.Orders do + for l in data.Lines do + if l.OrderId = o.Id && l.Quantity = o.CustomerId then + yield o.Id, l.Id + } + +let private matchingLinesOrNone (data: Data) = + seq { + for o in data.Orders do + match matchingLines data |> Seq.filter (fun (order, _) -> order = o.Id) |> List.ofSeq with + | [] -> yield o.Id, -1 + | matches -> yield! matches + } + +let private joins () = + [ Scenario.rows + "join on a scalar" + (fun db -> query { for c in db.Customers do join o in db.Orders on (c.Id = o.CustomerId); select (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do for o in c.Orders -> c.Name, o.Id }) + + Scenario.rows + "join on a tuple" + (fun db -> + query { for o in db.Orders do join l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)); select (o.Id, l.Id) }) + matchingLines + + Scenario.rows + "join on a struct tuple" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on (struct (o.Id, o.CustomerId) = struct (l.OrderId, l.Quantity)) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on a record, fields in declaration order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({ OrderNumber = o.Id; Units = o.CustomerId } = { OrderNumber = l.OrderId; Units = l.Quantity }) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on a record, fields out of declaration order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({ Units = o.CustomerId; OrderNumber = o.Id } = { Units = l.Quantity; OrderNumber = l.OrderId }) + select (o.Id, l.Id) + }) + matchingLines + |> Scenario.violating block + + Scenario.rows + "join on an anonymous record, fields in alphabetical order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({| Order = o.Id; Units = o.CustomerId |} = {| Order = l.OrderId; Units = l.Quantity |}) + select (o.Id, l.Id) + }) + matchingLines + + Scenario.rows + "join on an anonymous record, fields out of alphabetical order" + (fun db -> + query { + for o in db.Orders do + join l in db.Lines on ({| Units = o.CustomerId; Order = o.Id |} = {| Units = l.Quantity; Order = l.OrderId |}) + select (o.Id, l.Id) + }) + matchingLines + |> Scenario.violating block + + Scenario.rows + "groupJoin on a tuple" + (fun db -> + query { + for o in db.Orders do + groupJoin l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)) into lines + select (o.Id, lines.Count()) + }) + (fun data -> seq { for o in data.Orders -> o.Id, matchingLines data |> Seq.filter (fun (order, _) -> order = o.Id) |> Seq.length }) + + Scenario.rows + "leftOuterJoin on a tuple" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on ((o.Id, o.CustomerId) = (l.OrderId, l.Quantity)) into lines + for l in lines do + select (o.Id, (if obj.ReferenceEquals(l, null) then -1 else l.Id)) + }) + matchingLinesOrNone ] + +let private groupings () = + [ Scenario.rows + "groupBy a scalar" + (fun db -> query { for c in db.Customers do groupBy c.City into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy _.City) + + Scenario.rows + "groupBy a scalar, groups themselves" + (fun db -> query { for c in db.Customers do groupBy c.City }) + (customers >> Seq.groupBy _.City >> Seq.map (fun (city, group) -> Grouping(city, group) :> IGrouping<_, _>)) + + Scenario.rows + "groupBy a tuple" + (fun db -> query { for c in db.Customers do groupBy (c.City, c.Age > 28) into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28)) + + Scenario.rows + "groupBy a tuple, element of the key" + (fun db -> query { for c in db.Customers do groupBy (c.City, c.Age > 28) into g; select (fst g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28) >> Seq.map (fun ((city, _), count) -> city, count)) + |> Scenario.violating [ Violation.RuntimeHelperCall "Operators.Fst" ] + + Scenario.rows + "groupBy a struct tuple" + (fun db -> query { for c in db.Customers do groupBy (struct (c.City, c.Age > 28)) into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> struct (c.City, c.Age > 28))) + + Scenario.rows + "groupBy a record, fields in declaration order" + (fun db -> query { for c in db.Customers do groupBy { KeyCity = c.City; KeyAge = c.Age / 10 } into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> { KeyCity = c.City; KeyAge = c.Age / 10 })) + + Scenario.rows + "groupBy a record, fields out of declaration order" + (fun db -> query { for c in db.Customers do groupBy { KeyAge = c.Age / 10; KeyCity = c.City } into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> { KeyCity = c.City; KeyAge = c.Age / 10 })) + |> Scenario.violating block + + Scenario.rows + "groupBy an anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do groupBy {| City = c.City; Decade = c.Age / 10 |} into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> {| City = c.City; Decade = c.Age / 10 |})) + + Scenario.rows + "groupBy an anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do groupBy {| Decade = c.Age / 10; City = c.City |} into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy (fun c -> {| City = c.City; Decade = c.Age / 10 |})) + |> Scenario.violating block + + Scenario.rows + "groupValBy a tuple of values" + (fun db -> query { for c in db.Customers do groupValBy (c.Name, c.Age) c.City into g; select (g.Key, g.Count()) }) + (customers >> Seq.countBy _.City) + |> Scenario.violating [ Violation.RuntimeHelperCall "new Grouping`2" ] ] + +let cases () = joins () @ groupings () diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs new file mode 100644 index 00000000000..b8a704a846e --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryMethodSyntaxScenarios + +open System.Linq +open System.Linq.Expressions + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private reading typeName = + [ Violation.ReadsUnnamedMember typeName ] + +let private customers (data: Data) = Seq.ofList data.Customers + +let cases () = + [ Scenario.rows "Select a tuple" (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name))) (customers >> Seq.map (fun c -> c.Id, c.Name)) + + Scenario.rows + "Select a struct tuple" + (fun db -> db.Customers.Select(fun c -> struct (c.Id, c.Name))) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name))) + + Scenario.rows + "Where over a query built earlier" + (fun db -> + let withLargeOrders = db.Orders.Where(fun o -> o.Total > 4.0).Select(fun o -> o.CustomerId) + db.Customers.Where(fun c -> withLargeOrders.Contains c.Id).Select(fun c -> c.Name)) + (customers >> Seq.filter (fun c -> c.Orders |> Seq.exists (fun o -> o.Total > 4.0)) >> Seq.map _.Name) + + Scenario.rows + "Where over a query built earlier with a let" + (fun db -> + let withLargeOrders = db.Orders.Where(fun o -> let total = o.Total in total > 4.0).Select(fun o -> o.CustomerId) + db.Customers.Where(fun c -> withLargeOrders.Contains c.Id).Select(fun c -> c.Name)) + (customers >> Seq.filter (fun c -> c.Orders |> Seq.exists (fun o -> o.Total > 4.0)) >> Seq.map _.Name) + |> Scenario.violating block + + Scenario.rows + "Select a record, fields in declaration order" + (fun db -> db.Customers.Select(fun c -> { PersonName = c.Name; PersonAge = c.Age })) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + + Scenario.rows + "Select a record, fields out of declaration order" + (fun db -> db.Customers.Select(fun c -> { PersonAge = c.Age; PersonName = c.Name })) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "Select an anonymous record, fields in alphabetical order" + (fun db -> db.Customers.Select(fun c -> {| Age = c.Age; Name = c.Name |})) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "Select an anonymous record, fields out of alphabetical order" + (fun db -> db.Customers.Select(fun c -> {| Name = c.Name; Age = c.Age |})) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "Select with a let used twice" + (fun db -> db.Customers.Select(fun c -> let upper = c.Name.ToUpper() in upper + upper)) + (customers >> Seq.map (fun c -> c.Name.ToUpper() + c.Name.ToUpper())) + |> Scenario.violating block + + Scenario.rows + "Where taking a tuple apart" + (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name)).Where(fun (id, _) -> id > 1)) + (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) + |> Scenario.violating (reading "Tuple`2" @ block) + + Scenario.rows + "Where taking the last of nine apart" + (fun db -> + db.Customers + .Select(fun c -> (c.Id + 100, c.Name, c.City, c.Age, c.Id * 7, c.Name.Length, c.City.Length, c.Age * 2, c.Id)) + .Where(fun (_, _, _, _, _, _, _, _, last) -> last > 1) + .Select(fun (_, name, _, _, _, _, _, _, _) -> name)) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating (reading "Tuple`8" @ block) + + Scenario.rows + "Where over a query that selects a tuple" + (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) + (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) + |> Scenario.violating (reading "Tuple`2" @ block) + + Scenario.rows + "Where on a nullable property" + (fun db -> db.Customers.Where(fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) + (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) + |> Scenario.violating block + + Scenario.rows + "Where with a lambda over another source" + (fun db -> db.Customers.Where(fun c -> db.Orders.Any(fun o -> o.CustomerId = c.Id))) + (customers >> Seq.filter (fun c -> c.Orders.Count > 0)) + |> Scenario.violating + [ Violation.RuntimeHelperCall "LeafExpressionConverter.QuotationToLambdaExpression" + Violation.RuntimeHelperCall "LeafExpressionConverter.SubstHelper" ] + + Scenario.rows + "OrderBy, ThenByDescending" + (fun db -> db.Customers.OrderBy(fun c -> c.City).ThenByDescending(fun c -> c.Age).Select(fun c -> c.Name)) + (customers >> Seq.sortBy (fun c -> c.City, -c.Age) >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "GroupBy a tuple" + (fun db -> db.Customers.GroupBy(fun c -> (c.City, c.Age > 28)).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy (fun c -> c.City, c.Age > 28)) + + Scenario.rows + "SelectMany over a navigation collection" + (fun db -> db.Customers.SelectMany(fun c -> c.Orders :> seq).Select(fun o -> o.Id)) + (fun data -> data.Orders |> Seq.map _.Id) ] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs new file mode 100644 index 00000000000..6b2339a996b --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryOperatorScenarios.fs @@ -0,0 +1,197 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryOperatorScenarios + +open System +open System.Linq +open Microsoft.FSharp.Linq +open Microsoft.FSharp.Linq.NullableOperators + +let private customers (data: Data) = Seq.ofList data.Customers + +let private lines (data: Data) = Seq.ofList data.Lines + +let private discountOrLowest (line: OrderLine) = + if line.Discount.HasValue then line.Discount.Value else Int32.MinValue + +let private discounts (data: Data) = + data.Lines |> List.filter _.Discount.HasValue |> List.map _.Discount.Value + +let private sequences () = + [ Scenario.rows + "where" + (fun db -> query { for c in db.Customers do where (c.Age > 26); select c.Name }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) + + Scenario.rows + "where with string methods" + (fun db -> query { for c in db.Customers do where (c.Name.StartsWith "A" || c.City = "Paris"); select c.Name }) + (customers >> Seq.filter (fun c -> c.Name.StartsWith "A" || c.City = "Paris") >> Seq.map _.Name) + + Scenario.rows + "where on a nullable property" + (fun db -> query { for c in db.Customers do where (c.Registered.HasValue && c.Registered.Value.Year > 2020); select c.Name }) + (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020) >> Seq.map _.Name) + + Scenario.rows + "where with a nullable operator" + (fun db -> query { for l in db.Lines do where (l.Discount ?> 1); select l.Id }) + (lines >> Seq.filter (fun l -> l.Discount ?> 1) >> Seq.map _.Id) + + Scenario.rows + "if then" + (fun db -> query { for c in db.Customers do if c.Age > 26 then yield c.Name }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) + + Scenario.rows + "sortBy, thenByDescending" + (fun db -> query { for c in db.Customers do sortBy c.City; thenByDescending c.Age; select c.Name }) + (customers >> Seq.sortBy (fun c -> c.City, -c.Age) >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "ThenByDescending"; "Select" ] + + Scenario.rows + "sortByDescending, thenBy" + (fun db -> query { for c in db.Customers do sortByDescending c.City; thenBy c.Age; select c.Name }) + (customers >> Seq.sortBy _.Age >> Seq.sortByDescending _.City >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.flat [ "OrderByDescending"; "ThenBy"; "Select" ] + + Scenario.rows + "sortByNullable, thenByNullableDescending" + (fun db -> query { for l in db.Lines do sortByNullable l.Discount; thenByNullableDescending (Nullable l.Id); select l.Id }) + (lines >> Seq.sortBy (fun l -> discountOrLowest l, -l.Id) >> Seq.map _.Id) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "ThenByDescending"; "Select" ] + + Scenario.rows + "sortByNullableDescending, thenByNullable" + (fun db -> query { for l in db.Lines do sortByNullableDescending l.Discount; thenByNullable (Nullable l.Id); select l.Id }) + (lines >> Seq.sortBy (fun l -> -(int64 (discountOrLowest l)), l.Id) >> Seq.map _.Id) + |> Scenario.ordered + |> Scenario.flat [ "OrderByDescending"; "ThenBy"; "Select" ] + + Scenario.rows "distinct" (fun db -> query { for c in db.Customers do select c.City; distinct }) (customers >> Seq.map _.City >> Seq.distinct) + + Scenario.rows + "skip, take" + (fun db -> query { for c in db.Customers do sortBy c.Id; skip 1; take 2; select c.Name }) + (customers >> Seq.skip 1 >> Seq.take 2 >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "skipWhile, takeWhile" + (fun db -> query { for c in db.Customers do sortBy c.Id; skipWhile (c.Id < 2); takeWhile (c.Id < 4); select c.Name }) + (customers >> Seq.skipWhile (fun c -> c.Id < 2) >> Seq.takeWhile (fun c -> c.Id < 4) >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "join" + (fun db -> query { for o in db.Orders do join l in db.Lines on (o.Id = l.OrderId); select (o.Id, l.Id) }) + (fun data -> seq { for o in data.Orders do for l in o.Lines -> o.Id, l.Id }) + + Scenario.rows + "groupJoin" + (fun db -> query { for o in db.Orders do groupJoin l in db.Lines on (o.Id = l.OrderId) into lines; select (o.Id, lines.Count()) }) + (fun data -> seq { for o in data.Orders -> o.Id, o.Lines.Count }) + + Scenario.rows + "leftOuterJoin" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on (o.Id = l.OrderId) into lines + for l in lines do + select (o.Id, (if obj.ReferenceEquals(l, null) then -1 else l.Id)) + }) + (fun data -> + seq { + for o in data.Orders do + if o.Lines.Count = 0 then yield o.Id, -1 else for l in o.Lines -> o.Id, l.Id + }) + + Scenario.rows + "leftOuterJoin tested with isNull" + (fun db -> + query { + for o in db.Orders do + leftOuterJoin l in db.Lines on (o.Id = l.OrderId) into lines + for l in lines do + select (o.Id, isNull l) + }) + (fun data -> + seq { + for o in data.Orders do + if o.Lines.Count = 0 then yield o.Id, true else for _ in o.Lines -> o.Id, false + }) + |> Scenario.violating [ Violation.RuntimeHelperCall "Operators.IsNull" ] ] + +let private values () = + [ Scenario.value "count" (fun db -> query { for c in db.Customers do where (c.Age > 26); count }) (fun _ -> 3) + + Scenario.value "exists" (fun db -> query { for c in db.Customers do exists (c.City = "Paris") }) (fun _ -> true) + + Scenario.value "all" (fun db -> query { for c in db.Customers do all (c.Age > 26) }) (fun _ -> false) + + Scenario.value "contains" (fun db -> query { for c in db.Customers do select c.Id; contains 2 }) (fun _ -> true) + + Scenario.value "find" (fun db -> query { for c in db.Customers do find (c.Age < 30) }) (fun data -> data.Customers.[1]) + + Scenario.value "head" (fun db -> query { for c in db.Customers do sortBy c.Id; head }) (fun data -> data.Customers.[0]) + + Scenario.value + "headOrDefault" + (fun db -> query { for c in db.Customers do where (c.Age > 100); headOrDefault }) + (fun _ -> null) + + Scenario.value "last" (fun db -> query { for c in db.Customers do sortBy c.Id; last }) (fun data -> data.Customers.[3]) + + Scenario.value + "lastOrDefault" + (fun db -> query { for c in db.Customers do where (c.Age > 100); lastOrDefault }) + (fun _ -> null) + + Scenario.value + "exactlyOne" + (fun db -> query { for c in db.Customers do where (c.City = "Paris"); exactlyOne }) + (fun data -> data.Customers.[1]) + + Scenario.value + "exactlyOneOrDefault" + (fun db -> query { for c in db.Customers do where (c.City = "Rome"); exactlyOneOrDefault }) + (fun _ -> null) + + Scenario.value "nth" (fun db -> query { for c in db.Customers do sortBy c.Id; nth 2 }) (fun data -> data.Customers.[2]) + + Scenario.value "minBy" (fun db -> query { for c in db.Customers do minBy c.Age }) (customers >> Seq.map _.Age >> Seq.min) + + Scenario.value "maxBy" (fun db -> query { for c in db.Customers do maxBy c.Age }) (customers >> Seq.map _.Age >> Seq.max) + + Scenario.value "sumBy" (fun db -> query { for c in db.Customers do sumBy c.Age }) (customers >> Seq.sumBy _.Age) + + Scenario.value + "averageBy" + (fun db -> query { for c in db.Customers do averageBy (float c.Age) }) + (customers >> Seq.averageBy (fun c -> float c.Age)) + + Scenario.value + "minByNullable" + (fun db -> query { for l in db.Lines do minByNullable l.Discount }) + (discounts >> List.min >> Nullable) + + Scenario.value + "maxByNullable" + (fun db -> query { for l in db.Lines do maxByNullable l.Discount }) + (discounts >> List.max >> Nullable) + + Scenario.value + "sumByNullable" + (fun db -> query { for l in db.Lines do sumByNullable l.Discount }) + (discounts >> List.sum >> Nullable) + + Scenario.value + "averageByNullable" + (fun db -> query { for l in db.Lines do averageByNullable (Nullable.float l.Discount) }) + (discounts >> List.averageBy float >> Nullable) ] + +let cases () = sequences () @ values () diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs new file mode 100644 index 00000000000..44d598d3d13 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -0,0 +1,130 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryProjectionScenarios + +open System.Linq.Expressions + +let private block = [ Violation.Statement ExpressionType.Block ] + +let private customers (data: Data) = Seq.ofList data.Customers + +let cases () = + [ Scenario.rows "scalar" (fun db -> query { for c in db.Customers do select c.Name }) (customers >> Seq.map _.Name) + + Scenario.rows "entity" (fun db -> query { for c in db.Customers do select c }) customers + + Scenario.rows + "computed scalars" + (fun db -> query { for c in db.Customers do select (c.Name + "!", c.Age + 1, (if c.Age > 28 then "senior" else "junior")) }) + (customers >> Seq.map (fun c -> c.Name + "!", c.Age + 1, (if c.Age > 28 then "senior" else "junior"))) + + Scenario.rows + "tuple of two" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name) }) + (customers >> Seq.map (fun c -> c.Id, c.Name)) + + Scenario.rows + "tuple of seven" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2)) + + Scenario.rows + "tuple of eight" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7)) + + Scenario.rows + "tuple of nine" + (fun db -> query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id) }) + (customers >> Seq.map (fun c -> c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id)) + + Scenario.rows + "nested tuple" + (fun db -> query { for c in db.Customers do select ((c.Id, c.Name), c.Age) }) + (customers >> Seq.map (fun c -> (c.Id, c.Name), c.Age)) + + Scenario.rows + "tuple taken apart by the next for" + (fun db -> + query { + for (id, name) in (query { for c in db.Customers do select (c.Id, c.Name) }) do + where (id > 1) + select name + }) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + + Scenario.rows + "record, fields in declaration order" + (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + + Scenario.rows + "record, fields out of declaration order" + (fun db -> query { for c in db.Customers do select { PersonAge = c.Age; PersonName = c.Name } }) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "record of three, fields rotated" + (fun db -> query { for c in db.Customers do select { Town = c.City; FullName = c.Name; Years = c.Age } }) + (customers >> Seq.map (fun c -> { FullName = c.Name; Years = c.Age; Town = c.City })) + |> Scenario.violating block + + Scenario.rows + "record holding a tuple" + (fun db -> query { for c in db.Customers do select {| Age = c.Age; Pair = (c.Id, c.Name) |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Pair = (c.Id, c.Name) |})) + + Scenario.rows + "anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do select {| Age = c.Age; Name = c.Name |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do select {| Name = c.Name; Age = c.Age |} }) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "anonymous record, method calls in fields out of order" + (fun db -> query { for c in db.Customers do select {| Upper = c.Name.ToUpper(); City = c.City.Trim() |} }) + (customers >> Seq.map (fun c -> {| City = c.City.Trim(); Upper = c.Name.ToUpper() |})) + |> Scenario.violating block + + Scenario.rows + "anonymous record inside an anonymous record" + (fun db -> query { for c in db.Customers do select {| Person = {| Name = c.Name; Age = c.Age |} |} }) + (customers >> Seq.map (fun c -> {| Person = {| Age = c.Age; Name = c.Name |} |})) + |> Scenario.violating block + + Scenario.rows + "struct anonymous record, fields in alphabetical order" + (fun db -> query { for c in db.Customers do select (struct {| Age = c.Age; Name = c.Name |}) }) + (customers >> Seq.map (fun c -> struct {| Age = c.Age; Name = c.Name |})) + + Scenario.rows + "struct anonymous record, fields out of alphabetical order" + (fun db -> query { for c in db.Customers do select (struct {| Name = c.Name; Age = c.Age |}) }) + (customers >> Seq.map (fun c -> struct {| Age = c.Age; Name = c.Name |})) + |> Scenario.violating block + + Scenario.rows + "CLIMutable record, fields in declaration order" + (fun db -> query { for c in db.Customers do select { DtoName = c.Name; DtoAge = c.Age } }) + (customers >> Seq.map (fun c -> { DtoName = c.Name; DtoAge = c.Age })) + + Scenario.rows + "CLIMutable record, fields out of declaration order" + (fun db -> query { for c in db.Customers do select { DtoAge = c.Age; DtoName = c.Name } }) + (customers >> Seq.map (fun c -> { DtoName = c.Name; DtoAge = c.Age })) + |> Scenario.violating block + + Scenario.rows + "class with property initializers" + (fun db -> query { for c in db.Customers do select (CustomerView(ViewName = c.Name, ViewAge = c.Age)) }) + (customers >> Seq.map (fun c -> CustomerView(ViewName = c.Name, ViewAge = c.Age))) + + Scenario.rows "option" (fun db -> query { for c in db.Customers do select (Some c.Age) }) (customers >> Seq.map (fun c -> Some c.Age)) + |> Scenario.violating [ Violation.RuntimeHelperCall "FSharpOption`1.Some" ] ] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs new file mode 100644 index 00000000000..bad748d75ab --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -0,0 +1,355 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections +open System.Collections.Generic +open System.Linq +open System.Linq.Expressions +open System.Reflection +open Microsoft.FSharp.Reflection + +type ICapturedQuery = + abstract InMemory: IQueryable + +// EnumerableQuery turns Queryable calls into Enumerable ones only over its own constants; +// over any other IQueryable it calls Queryable again, which here would never end. +type private InMemoryRewriter() = + inherit ExpressionVisitor() + + override _.VisitConstant node = + match node.Value with + | :? ICapturedQuery as captured -> Expression.Constant captured.InMemory + | _ -> node + +/// An in-memory query that stays on the provider that created it and prints its root by name. +[] +type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption) = + + let enumerate () = + match derived with + | ValueSome expression -> provider.Execute>(expression).GetEnumerator() + | ValueNone -> items.GetEnumerator() + + override _.ToString() = + match derived with + | ValueSome expression -> expression.ToString() + | ValueNone -> name + + interface ICapturedQuery with + member _.InMemory = + match derived with + | ValueSome expression -> EnumerableQuery<'T>(InMemoryRewriter().Visit expression) + | ValueNone -> EnumerableQuery<'T> items + + interface IOrderedQueryable<'T> with + member this.Expression = + match derived with + | ValueSome expression -> expression + | ValueNone -> Expression.Constant this + + member _.ElementType = typeof<'T> + member _.Provider = provider + member _.GetEnumerator() : IEnumerator<'T> = enumerate () + member _.GetEnumerator() : IEnumerator = enumerate () + +/// Executes in memory, and records what it was asked to execute. +[] +type CapturingQueryProvider() = + let executed = ResizeArray() + let mutable depth = 0 + + let inMemory () = + EnumerableQuery Seq.empty :> IQueryProvider + + let track (expression: Expression) (run: Expression -> 'TResult) = + if depth = 0 then + executed.Add expression + + depth <- depth + 1 + + try + run (InMemoryRewriter().Visit expression) + finally + depth <- depth - 1 + + /// The expressions executed directly, not from inside another execution. + member _.Executed = executed :> IReadOnlyList + + member this.Source(name: string, items: seq<'T>) : IQueryable<'T> = + CapturedQuery<'T>(this, name, items, ValueNone) + + member this.Derived<'T>(expression: Expression) : IQueryable<'T> = + CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression) + + interface IQueryProvider with + member this.CreateQuery<'T>(expression: Expression) : IQueryable<'T> = this.Derived<'T> expression + + member this.CreateQuery(expression: Expression) : IQueryable = + let elementType = + expression.Type.GetInterfaces() + |> Array.append [| expression.Type |] + |> Array.find (fun ty -> ty.IsGenericType && ty.GetGenericTypeDefinition() = typedefof>) + |> fun ty -> ty.GetGenericArguments().[0] + + typeof + .GetMethod(nameof Unchecked.defaultof.Derived) + .MakeGenericMethod(elementType) + .Invoke(this, [| expression |]) + :?> IQueryable + + member _.Execute<'TResult>(expression: Expression) : 'TResult = + track expression (inMemory ()).Execute<'TResult> + + member _.Execute(expression: Expression) : obj = + track expression (inMemory ()).Execute + +/// Something in an expression tree that a C# expression lambda never contains, so no LINQ provider translates it. +[] +type Violation = + | Statement of ExpressionType + | LambdaInvocation + | UnboundParameter of name: string + | RuntimeHelperCall of methodName: string + | NestedInCollectionSelector of methodName: string + | ReadsUnnamedMember of typeName: string + | LeftProvider + | Throws of exceptionType: string + +module ProviderShape = + + let private fsharpCore = typeof.Assembly + + let private allMembers = BindingFlags.Public ||| BindingFlags.NonPublic + + let private isLinqOperator (call: MethodCallExpression) = + call.Arguments.Count > 0 + && (call.Method.DeclaringType = typeof + || call.Method.DeclaringType = typeof) + + let private isStatement nodeType = + match nodeType with + | ExpressionType.Block + | ExpressionType.Assign + | ExpressionType.Loop + | ExpressionType.Goto + | ExpressionType.Label + | ExpressionType.Try + | ExpressionType.Switch + | ExpressionType.RuntimeVariables + | ExpressionType.DebugInfo -> true + | _ -> false + + let private flattensOrJoins (call: MethodCallExpression) = + isLinqOperator call + && (match call.Method.Name with + | "SelectMany" + | "Join" + | "GroupJoin" -> true + | _ -> false) + + type private ShapeVisitor() = + inherit ExpressionVisitor() + + let found = ResizeArray() + let bound = HashSet(HashIdentity.Reference) + let mutable collectionSelectorDepth = 0 + + let bind (parameters: seq) = + parameters |> Seq.filter bound.Add |> Seq.toArray + + let unbind (parameters: ParameterExpression array) = + for parameter in parameters do + bound.Remove parameter |> ignore + + [] + let (|AssignsBound|_|) (expression: Expression) = + match expression with + | :? BinaryExpression as assign when assign.NodeType = ExpressionType.Assign -> + match assign.Left with + | :? ParameterExpression as variable when bound.Contains variable -> ValueSome assign.Right + | _ -> ValueNone + | _ -> ValueNone + + member _.Violations = found |> Seq.distinct |> List.ofSeq + + override _.VisitLambda<'T>(node: Expression<'T>) = + let added = bind node.Parameters + + try + base.VisitLambda node + finally + unbind added + + // A block is reported once: its variables and their initial assignments are part of it. + override this.VisitBlock node = + found.Add(Violation.Statement ExpressionType.Block) + let added = bind node.Variables + + try + for expression in node.Expressions do + match expression with + | AssignsBound value -> this.Visit value |> ignore + | expression -> this.Visit expression |> ignore + + node + finally + unbind added + + override _.VisitParameter node = + if not (bound.Contains node) then + found.Add(Violation.UnboundParameter node.Name) + + node + + override _.VisitInvocation node = + found.Add Violation.LambdaInvocation + base.VisitInvocation node + + // A query built earlier and used in this one is part of what the provider translates. + override this.VisitConstant node = + match node.Value with + | :? IQueryable as query -> + match query.Expression with + | :? ConstantExpression as root when obj.ReferenceEquals(root.Value, query) -> () + | built -> this.Visit built |> ignore + | _ -> () + + node + + override _.VisitNew node = + if node.Type.Assembly = fsharpCore && not (node.Type.Name.StartsWith("AnonymousObject", StringComparison.Ordinal)) then + found.Add(Violation.RuntimeHelperCall $"new {node.Type.Name}") + + base.VisitNew node + + override this.VisitMethodCall node = + if node.Method.DeclaringType.Assembly = fsharpCore then + found.Add(Violation.RuntimeHelperCall $"{node.Method.DeclaringType.Name}.{node.Method.Name}") + + if collectionSelectorDepth > 0 && flattensOrJoins node then + found.Add(Violation.NestedInCollectionSelector node.Method.Name) + + if isLinqOperator node && node.Method.Name = "SelectMany" then + this.Visit node.Object |> ignore + this.Visit node.Arguments.[0] |> ignore + collectionSelectorDepth <- collectionSelectorDepth + 1 + + try + this.Visit node.Arguments.[1] |> ignore + finally + collectionSelectorDepth <- collectionSelectorDepth - 1 + + for argument in Seq.skip 2 node.Arguments do + this.Visit argument |> ignore + + node + else + base.VisitMethodCall node + + override this.Visit node = + match node with + | null -> null + | node -> + if node.NodeType <> ExpressionType.Block && isStatement node.NodeType then + found.Add(Violation.Statement node.NodeType) + + base.Visit node + + [] + let private (|Lambda|_|) (argument: Expression) = + match argument with + | :? UnaryExpression as quote when quote.NodeType = ExpressionType.Quote -> ValueSome(quote.Operand :?> LambdaExpression) + | :? LambdaExpression as lambda -> ValueSome lambda + | _ -> ValueNone + + /// A tuple or record built without saying which member each argument becomes. + let private buildsUnnamed (body: Expression) = + match body with + | :? NewExpression as construction when construction.Arguments.Count > 0 -> + match construction.Members with + | null -> + FSharpType.IsTuple construction.Type + || FSharpType.IsRecord(construction.Type, allMembers) + | _ -> false + | _ -> false + + type private MemberReadFinder(owner: ParameterExpression) = + inherit ExpressionVisitor() + + member val Found = false with get, set + + override this.VisitMember node = + if obj.ReferenceEquals(node.Expression, owner) then + this.Found <- true + + base.VisitMember node + + let private readsMemberOf (owner: ParameterExpression) (body: Expression) = + let finder = MemberReadFinder owner + finder.Visit body |> ignore + finder.Found + + let private yields (elementType: Type) (source: Expression) = + typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type + + /// Members read from elements that an earlier operator built without naming them: + /// the provider has to take apart an object it was not told how to. + let private unnamedMemberReads (expression: Expression) = + let found = ResizeArray() + + let rec producesUnnamed (expression: Expression) = + match expression with + | :? MethodCallExpression as call when isLinqOperator call -> + let source = call.Arguments.[0] + let sourceUnnamed = producesUnnamed source + + let lambdas = + [ for argument in Seq.skip 1 call.Arguments do + match argument with + | Lambda lambda -> lambda + | _ -> () ] + + if sourceUnnamed then + for lambda in lambdas do + let element = lambda.Parameters.[0] + + if yields element.Type source && readsMemberOf element lambda.Body then + found.Add(Violation.ReadsUnnamedMember element.Type.Name) + + match call.Method.Name with + | "Select" + | "SelectMany" + | "Join" + | "GroupJoin" -> lambdas |> List.last |> _.Body |> buildsUnnamed + | "Where" + | "OrderBy" + | "OrderByDescending" + | "ThenBy" + | "ThenByDescending" + | "Skip" + | "SkipWhile" + | "Take" + | "TakeWhile" + | "Distinct" + | "DefaultIfEmpty" -> sourceUnnamed + | _ -> false + | _ -> false + + producesUnnamed expression |> ignore + List.ofSeq found + + let violations (expression: Expression) = + let visitor = ShapeVisitor() + visitor.Visit expression |> ignore + visitor.Violations @ unnamedMemberReads expression |> List.distinct + + /// The LINQ operators along the source chain, outermost last. + let spine (expression: Expression) = + let rec walk (expression: Expression) operators = + match expression with + | :? MethodCallExpression as call when isLinqOperator call -> walk call.Arguments.[0] (call.Method.Name :: operators) + | _ -> operators + + walk expression [] diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs new file mode 100644 index 00000000000..b2a270042e9 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -0,0 +1,189 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections +open System.Globalization +open System.Linq +open System.Linq.Expressions +open System.Reflection +open Microsoft.FSharp.Reflection + +[] +type Outcome = + | Rows of IQueryable + | Value of obj + +/// One query, what it must return, and what is known to be wrong with its expression tree today. +[] +type Scenario = + { Name: string + Run: Db -> Outcome + Expected: Data -> obj list + Ordered: bool + Spine: string list voption + Violations: Violation list } + + override this.ToString() = this.Name + +module Scenario = + + let private allMembers = BindingFlags.Public ||| BindingFlags.NonPublic + + [] + let private (|Grouping|_|) (value: obj) = + match value.GetType().GetInterface(typedefof>.Name) with + | null -> ValueNone + | grouping -> ValueSome(grouping.GetProperty("Key").GetValue value) + + [] + let private (|Tuple|_|) (value: obj) = + if FSharpType.IsTuple(value.GetType()) then + ValueSome(FSharpValue.GetTupleFields value) + else + ValueNone + + [] + let private (|Record|_|) (value: obj) = + if FSharpType.IsRecord(value.GetType(), allMembers) then + let names = FSharpType.GetRecordFields(value.GetType(), allMembers) |> Seq.map _.Name + ValueSome(Seq.zip names (FSharpValue.GetRecordFields(value, allMembers))) + else + ValueNone + + [] + let private (|Union|_|) (value: obj) = + if FSharpType.IsUnion(value.GetType(), allMembers) then + let case, fields = FSharpValue.GetUnionFields(value, value.GetType(), allMembers) + ValueSome struct (case.Name, fields) + else + ValueNone + + /// Text that is equal for equal results, whichever provider materialized them. + let rec render (value: obj) = + let renderAll (values: seq) = + values |> Seq.map render |> String.concat "; " + + match value with + | null -> "null" + | :? string as text -> $"\"{text}\"" + | :? IFormattable as number -> number.ToString(null, CultureInfo.InvariantCulture) + | Grouping key -> $"{render key} => [{renderAll (Seq.cast (value :?> IEnumerable))}]" + | :? IEnumerable as items -> $"[{renderAll (Seq.cast items)}]" + | Tuple elements -> $"({renderAll elements})" + | Record fields -> + let rendered = fields |> Seq.map (fun (name, field) -> $"{name} = {render field}") + $"""{{ {String.concat "; " rendered} }}""" + | Union(case, fields) -> $"{case}({renderAll fields})" + | value -> string value + + let rows name (run: Db -> IQueryable<'T>) (expected: Data -> seq<'T>) = + { Name = name + Run = fun db -> Outcome.Rows(run db) + Expected = fun data -> [ for row in expected data -> box row ] + Ordered = false + Spine = ValueNone + Violations = [] } + + let value name (run: Db -> 'T) (expected: Data -> 'T) = + { Name = name + Run = fun db -> Outcome.Value(box (run db)) + Expected = fun data -> [ box (expected data) ] + Ordered = true + Spine = ValueNone + Violations = [] } + + let ordered scenario = { scenario with Ordered = true } + + /// The operators the source chain must consist of, outermost last. + let flat spine scenario = + { scenario with + Spine = ValueSome spine } + + /// What is wrong with the expression tree today; the scenario fails once that stops being true. + let violating violations scenario = + { scenario with + Violations = violations } + + [] + type private Observed = + { Expression: Expression + Violations: Violation list + Rows: obj list } + + let rec private rootCause (error: exn) = + match error, error.InnerException with + | :? TargetInvocationException, null -> error + | :? TargetInvocationException, inner -> rootCause inner + | _ -> error + + let private observe (scenario: Scenario) = + let provider = CapturingQueryProvider() + let data = Data() + + let observed = + try + match scenario.Run(Db.inMemory provider data) with + | Outcome.Rows rows -> + let stayed = obj.ReferenceEquals(rows.Provider, provider) + + { Expression = rows.Expression + Violations = (if stayed then [] else [ Violation.LeftProvider ]) + Rows = [ for row in rows -> row ] } + // The value comes from the last execution; whatever ran before it went to the provider too. + | Outcome.Value value when provider.Executed.Count > 0 -> + let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) + + { Expression = provider.Executed.[provider.Executed.Count - 1] + Violations = [ for expression in earlier do yield! ProviderShape.violations expression ] + Rows = [ value ] } + | Outcome.Value value -> + { Expression = Expression.Empty() + Violations = [ Violation.LeftProvider ] + Rows = [ value ] } + with error -> + { Expression = Expression.Empty() + Violations = [ Violation.Throws((rootCause error).GetType().Name) ] + Rows = [] } + + data, observed + + let private inOrder ordered (rows: obj list) = + let rendered = List.map render rows + if ordered then rendered else List.sort rendered + + /// Everything the scenario gets wrong, empty when it behaves as described. + let failures (scenario: Scenario) = + let data, observed = observe scenario + + let violations = + observed.Violations @ ProviderShape.violations observed.Expression + |> List.distinct + + let threw = + violations + |> List.exists (function + | Violation.Throws _ -> true + | _ -> false) + + [ if set violations <> set scenario.Violations then + $"violations: expected %A{scenario.Violations}, got %A{violations}" + + if not threw then + let expected = inOrder scenario.Ordered (scenario.Expected data) + let actual = inOrder scenario.Ordered observed.Rows + + if expected <> actual then + $"rows: expected %A{expected}, got %A{actual}" + + match scenario.Spine with + | ValueSome spine when spine <> ProviderShape.spine observed.Expression -> + $"operators: expected %A{spine}, got %A{ProviderShape.spine observed.Expression}" + | _ -> () ] + |> List.map (fun failure -> $"{scenario.Name}: {failure}\n in {observed.Expression}") + + let check scenario = + match failures scenario with + | [] -> () + | failures -> failwith (String.concat "\n" failures) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs new file mode 100644 index 00000000000..ada7f2ba70f --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenarioDomain.fs @@ -0,0 +1,109 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open System +open System.Collections.Generic +open System.Linq + +[] +type Product() = + member val Id = 0 with get, set + member val Name = "" with get, set + member val Category = "" with get, set + override this.ToString() = $"Product#{this.Id}" + +[] +type OrderLine() = + member val Id = 0 with get, set + member val OrderId = 0 with get, set + member val ProductId = 0 with get, set + member val Quantity = 0 with get, set + member val Price = 0.0 with get, set + member val Discount = Nullable() with get, set + override this.ToString() = $"OrderLine#{this.Id}" + +[] +type Order() = + member val Id = 0 with get, set + member val CustomerId = 0 with get, set + member val Total = 0.0 with get, set + member val Lines: ICollection = List() with get, set + override this.ToString() = $"Order#{this.Id}" + +[] +type Customer() = + member val Id = 0 with get, set + member val Name = "" with get, set + member val City = "" with get, set + member val Age = 0 with get, set + member val Registered = Nullable() with get, set + member val Orders: ICollection = List() with get, set + override this.ToString() = $"Customer#{this.Id}" + +type CustomerView() = + member val ViewName = "" with get, set + member val ViewAge = 0 with get, set + override this.ToString() = $"CustomerView({this.ViewName}, {this.ViewAge})" + +type NameAndAge = { PersonName: string; PersonAge: int } + +type NameAgeCity = + { FullName: string + Years: int + Town: string } + +[] +type NameAndAgeDto = { DtoName: string; DtoAge: int } + +type CityAndAge = { KeyCity: string; KeyAge: int } + +type OrderAndQuantity = { OrderNumber: int; Units: int } + +type Data() = + let line id orderId productId quantity price discount = + OrderLine(Id = id, OrderId = orderId, ProductId = productId, Quantity = quantity, Price = price, Discount = discount) + + let order id customerId total (lines: OrderLine list) = + Order(Id = id, CustomerId = customerId, Total = total, Lines = List lines) + + let customer id name city age registered (orders: Order list) = + Customer(Id = id, Name = name, City = city, Age = age, Registered = registered, Orders = List orders) + + let orders = + [ order 10 1 12.5 [ line 100 10 1 2 2.5 (Nullable 1); line 101 10 2 1 7.5 (Nullable()) ] + order 11 1 3.0 [ line 102 11 3 1 3.0 (Nullable 2) ] + order 12 2 5.0 [ line 103 12 1 2 2.5 (Nullable()) ] + order 13 3 0.0 [] ] + + let ordersOf customerId = + orders |> List.filter (fun order -> order.CustomerId = customerId) + + member val Products = + [ Product(Id = 1, Name = "Pen", Category = "Office") + Product(Id = 2, Name = "Ink", Category = "Office") + Product(Id = 3, Name = "Tea", Category = "Food") ] + + member val Customers = + [ customer 1 "Alice" "London" 30 (Nullable(DateTime(2020, 1, 1))) (ordersOf 1) + customer 2 "Bob" "Paris" 25 (Nullable()) (ordersOf 2) + customer 3 "Carol" "London" 35 (Nullable(DateTime(2021, 6, 15))) (ordersOf 3) + customer 4 "Dave" "Berlin" 28 (Nullable()) [] ] + + member _.Orders = orders + member _.Lines = orders |> List.collect (fun order -> List.ofSeq order.Lines) + +[] +type Db = + { Customers: IQueryable + Orders: IQueryable + Lines: IQueryable + Products: IQueryable } + +module Db = + + let inMemory (provider: CapturingQueryProvider) (data: Data) = + { Customers = provider.Source("Customers", data.Customers) + Orders = provider.Source("Orders", data.Orders) + Lines = provider.Source("Lines", data.Lines) + Products = provider.Source("Products", data.Products) } diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs new file mode 100644 index 00000000000..6e284ed7d4d --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Core.UnitTests.Linq + +open Xunit + +type QueryShapeTests() = + + static let rows (cases: Scenario list) = + cases |> Seq.map (fun scenario -> [| box scenario |]) + + static member Projections = rows (QueryProjectionScenarios.cases ()) + static member Operators = rows (QueryOperatorScenarios.cases ()) + static member Keys = rows (QueryKeyScenarios.cases ()) + static member Structures = rows (QueryStructureScenarios.cases ()) + static member MethodSyntax = rows (QueryMethodSyntaxScenarios.cases ()) + + [] + member _.Projection(scenario: Scenario) = Scenario.check scenario + + [] + member _.Operator(scenario: Scenario) = Scenario.check scenario + + [] + member _.Key(scenario: Scenario) = Scenario.check scenario + + [] + member _.Structure(scenario: Scenario) = Scenario.check scenario + + [] + member _.``Method syntax``(scenario: Scenario) = Scenario.check scenario diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs new file mode 100644 index 00000000000..6f806f4bb43 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -0,0 +1,268 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.QueryStructureScenarios + +open System.Linq + +let private quotedLambda = + [ Violation.RuntimeHelperCall "LeafExpressionConverter.QuotationToLambdaExpression" + Violation.RuntimeHelperCall "LeafExpressionConverter.SubstHelper" ] + +let private nested operators = + operators |> List.map Violation.NestedInCollectionSelector + +let private orDefault (items: seq<'T>) = + if Seq.isEmpty items then Seq.singleton Unchecked.defaultof<'T> else items + +let private idOrNone (order: Order) = + match order with + | null -> -1 + | order -> order.Id + +let private nameOrNone (product: Product) = + match product with + | null -> "" + | product -> product.Name + +let private ordersWithLines (data: Data) = + seq { for c in data.Customers do for o in c.Orders do for l in o.Lines -> c.Name, o.Id, l.Id } + +let private customersWithOrders (data: Data) = + seq { for c in data.Customers do for o in c.Orders -> c.Name, o.Id } + +let private leftJoinedOrders (data: Data) = + seq { for c in data.Customers do for o in orDefault c.Orders -> c.Name, idOrNone o } + +let private leftJoinedOrdersAndProducts (data: Data) = + seq { + for c in data.Customers do + for o in orDefault c.Orders do + for p in orDefault (data.Products |> List.filter (fun p -> p.Id = c.Id)) -> c.Name, idOrNone o, nameOrNone p + } + +let private loops () = + [ Scenario.rows + "two for" + (fun db -> query { for c in db.Customers do for p in db.Products do select (c.Name, p.Name) }) + (fun data -> seq { for c in data.Customers do for p in data.Products -> c.Name, p.Name }) + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "two for, where over both" + (fun db -> query { for c in db.Customers do for o in db.Orders do where (o.CustomerId = c.Id); select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "for, where, for, where" + (fun db -> + query { + for c in db.Customers do + where (c.Age > 26) + for o in db.Orders do + where (o.CustomerId = c.Id) + select (c.Name, o.Id) + }) + (fun data -> seq { for c in data.Customers do if c.Age > 26 then for o in c.Orders -> c.Name, o.Id }) + |> Scenario.flat [ "Where"; "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "for over a navigation collection" + (fun db -> query { for c in db.Customers do for o in c.Orders do select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "for over two levels of navigation collections" + (fun db -> query { for c in db.Customers do for o in c.Orders do for l in o.Lines do select (c.Name, o.Id, l.Id) }) + ordersWithLines + |> Scenario.violating (nested [ "SelectMany" ]) + + Scenario.rows + "three for" + (fun db -> + query { + for c in db.Customers do + for o in db.Orders do + for l in db.Lines do + where (o.CustomerId = c.Id && l.OrderId = o.Id) + select (c.Name, o.Id, l.Id) + }) + ordersWithLines + |> Scenario.violating (nested [ "SelectMany" ]) + + Scenario.rows + "for after sortBy" + (fun db -> query { for c in db.Customers do sortBy c.Age; for p in db.Products do select (c.Name, p.Name) }) + (fun data -> seq { for c in data.Customers |> List.sortBy _.Age do for p in data.Products -> c.Name, p.Name }) + |> Scenario.ordered + |> Scenario.flat [ "OrderBy"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "for after join" + (fun db -> + query { + for c in db.Customers do + join o in db.Orders on (c.Id = o.CustomerId) + for l in db.Lines do + where (l.OrderId = o.Id) + select (c.Name, o.Id, l.Id) + }) + ordersWithLines + |> Scenario.flat [ "Join"; "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "let between for" + (fun db -> + query { for c in db.Customers do let next = c.Id + 9 in for o in db.Orders do where (o.Id = next); select (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do for o in data.Orders do if o.Id = c.Id + 9 then yield c.Name, o.Id }) + |> Scenario.flat [ "SelectMany"; "Where"; "Select"; "Select" ] + + Scenario.rows + "if then around for" + (fun db -> query { for c in db.Customers do if c.Age > 26 then for o in c.Orders do yield (c.Name, o.Id) }) + (fun data -> seq { for c in data.Customers do if c.Age > 26 then for o in c.Orders -> c.Name, o.Id }) + |> Scenario.flat [ "Where"; "SelectMany"; "Select" ] + + Scenario.rows + "for around if then" + (fun db -> query { for c in db.Customers do for o in db.Orders do if o.CustomerId = c.Id then yield (c.Name, o.Id) }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select" ] + + Scenario.rows + "yield! of a navigation collection" + (fun db -> query { for c in db.Customers do yield! c.Orders }) + (fun data -> Seq.ofList data.Orders) + |> Scenario.flat [ "SelectMany" ] + + Scenario.rows + "distinct and take after two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do select c.City; distinct; take 1 }) + (fun _ -> seq { "London" }) + |> Scenario.ordered + |> Scenario.flat [ "SelectMany"; "Select"; "Distinct"; "Take" ] + + Scenario.rows + "groupBy after two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do groupBy c.City into g; select (g.Key, g.Count()) }) + (fun data -> seq { for c in data.Customers do for _ in c.Orders -> c.City } |> Seq.countBy id) + |> Scenario.violating [ Violation.RuntimeHelperCall "new Grouping`2" ] + + Scenario.value "count over two for" (fun db -> query { for c in db.Customers do for o in c.Orders do count }) (fun _ -> 4) + + Scenario.value + "exists over two for" + (fun db -> query { for c in db.Customers do for o in c.Orders do exists (o.Total > 10.0 && c.City = "London") }) + (fun _ -> true) + |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] ] + +let private subqueries () = + [ Scenario.rows + "nested query as the source" + (fun db -> query { for o in (query { for o in db.Orders do where (o.Total > 4.0); select o }) do select o.Id }) + (fun data -> seq { for o in data.Orders do if o.Total > 4.0 then yield o.Id }) + |> Scenario.flat [ "Where"; "Select" ] + + Scenario.rows + "nested query as the source of the second for" + (fun db -> + query { + for c in db.Customers do + for o in (query { for o in db.Orders do where (o.CustomerId = c.Id); select o }) do + select (c.Name, o.Id) + }) + customersWithOrders + |> Scenario.flat [ "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "nested query in select" + (fun db -> + query { for c in db.Customers do select (c.Name, query { for o in db.Orders do where (o.CustomerId = c.Id); select o.Id }) }) + (fun data -> seq { for c in data.Customers -> c.Name, (seq { for o in c.Orders -> o.Id }).AsQueryable() }) + + Scenario.rows + "second for over a source filtered by a lambda" + (fun db -> query { for c in db.Customers do for o in db.Orders.Where(fun o -> o.CustomerId = c.Id) do select (c.Name, o.Id) }) + customersWithOrders + |> Scenario.violating quotedLambda + + Scenario.rows + "where with a lambda over another source" + (fun db -> query { for c in db.Customers do where (db.Orders.Any(fun o -> o.CustomerId = c.Id)); select c.Name }) + (fun data -> seq { for c in data.Customers do if c.Orders.Count > 0 then yield c.Name }) + |> Scenario.violating quotedLambda ] + +let private leftJoins () = + [ Scenario.rows + "left join spelled leftOuterJoin" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id)) + }) + leftJoinedOrders + |> Scenario.flat [ "GroupJoin"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "left join spelled groupJoin and DefaultIfEmpty" + (fun db -> + query { + for c in db.Customers do + groupJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders.DefaultIfEmpty() do + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id)) + }) + leftJoinedOrders + |> Scenario.flat [ "GroupJoin"; "SelectMany"; "Select"; "Select" ] + + Scenario.rows + "two left joins spelled leftOuterJoin" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + leftOuterJoin p in db.Products on (c.Id = p.Id) into products + for p in products do + select + (c.Name, + (if obj.ReferenceEquals(o, null) then -1 else o.Id), + (if obj.ReferenceEquals(p, null) then "" else p.Name)) + }) + leftJoinedOrdersAndProducts + |> Scenario.violating (nested [ "GroupJoin"; "SelectMany" ]) + + Scenario.rows + "two left joins spelled groupJoin and DefaultIfEmpty" + (fun db -> + query { + for c in db.Customers do + groupJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders.DefaultIfEmpty() do + groupJoin p in db.Products on (c.Id = p.Id) into products + for p in products.DefaultIfEmpty() do + select + (c.Name, + (if obj.ReferenceEquals(o, null) then -1 else o.Id), + (if obj.ReferenceEquals(p, null) then "" else p.Name)) + }) + leftJoinedOrdersAndProducts + |> Scenario.violating (nested [ "GroupJoin"; "SelectMany" ]) + + Scenario.rows + "join after a left join" + (fun db -> + query { + for c in db.Customers do + leftOuterJoin o in db.Orders on (c.Id = o.CustomerId) into orders + for o in orders do + join p in db.Products on (c.Id = p.Id) + select (c.Name, (if obj.ReferenceEquals(o, null) then -1 else o.Id), p.Name) + }) + (leftJoinedOrdersAndProducts >> Seq.filter (fun (_, _, product) -> product <> "")) + |> Scenario.violating (nested [ "Join" ]) ] + +let cases () = loops () @ subqueries () @ leftJoins () From 8bd4cd28dd4c73ce7c97ea77f2248b2ceae615df Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 04:29:30 +0200 Subject: [PATCH 02/13] Translate struct tuples in queries Selecting a struct tuple failed with "unreachable": the tuple was replaced by an anonymous object, but only reference tuples had an anonymous object type to be replaced by, and the way back always built a reference tuple. Both kinds now share the anonymous object of their arity, and the way back builds the kind the query asked for. Taking a struct tuple apart in a lambda failed with a tuple index out of range, because a struct tuple has fields where a reference tuple has properties. Fixes #15522 Co-Authored-By: Claude Fable 5.1 --- docs/release-notes/.FSharp.Core/11.0.200.md | 1 + src/FSharp.Core/Linq.fs | 9 ++++- src/FSharp.Core/Query.fs | 5 +-- src/FSharp.Core/QueryExtensions.fs | 35 ++++++------------- .../QueryMethodSyntaxScenarios.fs | 16 +++++++++ .../QueryProjectionScenarios.fs | 32 +++++++++++++++++ 6 files changed, 71 insertions(+), 27 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 49e81a0efdf..fd72f4da417 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,5 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) ### Changed diff --git a/src/FSharp.Core/Linq.fs b/src/FSharp.Core/Linq.fs index b8025b39bb4..431f6fd9514 100644 --- a/src/FSharp.Core/Linq.fs +++ b/src/FSharp.Core/Linq.fs @@ -475,7 +475,14 @@ module LeafExpressionConverter = | TupleGet(arg, n) -> let argP = ConvExprToLinqInContext env arg - let rec build ty argP n = + let rec build (ty: Type) argP n = + // A struct tuple holds its elements in fields, which PreComputeTuplePropertyInfo cannot return + if ty.IsValueType then + if n < 7 then + Expression.Field(argP, ty.GetField("Item" + string (n + 1))) |> asExpr + else + build (ty.GetGenericArguments().[7]) (Expression.Field(argP, ty.GetField "Rest") |> asExpr) (n - 7) + else match FSharpValue.PreComputeTuplePropertyInfo(ty, n) with | propInfo, None -> Expression.Property(argP, propInfo) |> asExpr diff --git a/src/FSharp.Core/Query.fs b/src/FSharp.Core/Query.fs index 2f84efd0eee..56171c629f9 100644 --- a/src/FSharp.Core/Query.fs +++ b/src/FSharp.Core/Query.fs @@ -1156,8 +1156,9 @@ module Query = /// immutable-to-mutable conversion information, convert it back to an expression involving immutable tuples or records. let rec ConvMutableToImmutable conv mutExpr = match conv with - | TupleConv convs -> - Expr.NewTuple (convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i)))) + | TupleConv (isStruct, convs) -> + let elements = convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i))) + if isStruct then Expr.NewStructTuple elements else Expr.NewTuple elements | RecordConv (typ, convs) -> Expr.NewRecord (typ, convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i)))) diff --git a/src/FSharp.Core/QueryExtensions.fs b/src/FSharp.Core/QueryExtensions.fs index eafdae2d187..18edd4b9b5b 100644 --- a/src/FSharp.Core/QueryExtensions.fs +++ b/src/FSharp.Core/QueryExtensions.fs @@ -108,14 +108,6 @@ module internal Adapters = let anonObjectTypes = tupleTypes |> Array.map snd - let tupleToAnonTypeMap = - let t = Dictionary() - - for k, v in tupleTypes do - t.[k] <- v - - t - let anonToTupleTypeMap = let t = Dictionary() @@ -173,18 +165,13 @@ module internal Adapters = walk i e e.Type + /// Reference and struct tuples of the same arity share one anonymous object type. let RewriteTupleType (ty: Type) conv = - // Tuples are generic, so lookup only for generic types - assert ty.IsGenericType - let generic = ty.GetGenericTypeDefinition() - - match tupleToAnonTypeMap.TryGetValue generic with - | true, mutableTupleType -> - // Recursively transform type arguments - mutableTupleType.MakeGenericType(ty.GetGenericArguments() |> Array.toList |> conv |> Array.ofList) - | _ -> - assert false - failwith "unreachable" + assert (FSharpType.IsTuple ty) + let elementTypes = ty.GetGenericArguments() + let mutableTupleType = anonObjectTypes.[elementTypes.Length - 1] + // Recursively transform type arguments + mutableTupleType.MakeGenericType(elementTypes |> Array.toList |> conv |> Array.ofList) let (|RecordFieldGetSimplification|_|) (expr: Expr) = match expr with @@ -206,7 +193,7 @@ module internal Adapters = /// that we can invert the process in final query results. [] type ConversionDescription = - | TupleConv of ConversionDescription list + | TupleConv of isStruct: bool * ConversionDescription list | RecordConv of Type * ConversionDescription list | GroupingConv (* origKeyType: *) of Type (* origElemType: *) * Type * ConversionDescription | SeqConv of ConversionDescription @@ -216,18 +203,18 @@ module internal Adapters = /// "yield" or "select", convert it to a type involving anonymous objects according to the conversion data. let rec ConvImmutableTypeToMutableType conv ty = match conv with - | TupleConv convs -> + | TupleConv(isStruct, convs) -> assert (FSharpType.IsTuple ty) match convs with | x1 :: x2 :: x3 :: x4 :: x5 :: x6 :: x7 :: x8 :: tail -> - let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(x8 :: tail) ] + let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(isStruct, x8 :: tail) ] RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType els) | _ -> RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType convs) | RecordConv(_, convs) -> assert (isPartiallyImmutableRecord ty) let types = [| for f in FSharpType.GetRecordFields ty -> f.PropertyType |] - ConvImmutableTypeToMutableType (TupleConv convs) (FSharpType.MakeTupleType types) + ConvImmutableTypeToMutableType (TupleConv(false, convs)) (FSharpType.MakeTupleType types) | GroupingConv(_keyTy, _elemTy, conv) -> assert ty.IsGenericType assert (ty.GetGenericTypeDefinition() = typedefof>) @@ -324,7 +311,7 @@ module internal Adapters = let argExprsNow, argScripts = exprs |> List.map (ProduceMoreMutables tipf) |> List.unzip - NewAnonymousObject argExprsNow, TupleConv argScripts + NewAnonymousObject argExprsNow, TupleConv(expr.Type.IsValueType, argScripts) // Replace immutable records by anonymous objects | Patterns.NewRecord(typ, args) when isPartiallyImmutableRecord typ -> diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index b8a704a846e..5325268456b 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -69,6 +69,12 @@ let cases () = (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) |> Scenario.violating (reading "Tuple`2" @ block) + Scenario.rows + "Where taking a struct tuple apart" + (fun db -> db.Customers.Select(fun c -> struct (c.Id, c.Name)).Where(fun (struct (id, _)) -> id > 1)) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name)) >> Seq.filter (fun (struct (id, _)) -> id > 1)) + |> Scenario.violating (reading "ValueTuple`2" @ block) + Scenario.rows "Where taking the last of nine apart" (fun db -> @@ -79,6 +85,16 @@ let cases () = (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) |> Scenario.violating (reading "Tuple`8" @ block) + Scenario.rows + "Where taking the last of a struct nine apart" + (fun db -> + db.Customers + .Select(fun c -> struct (c.Id + 100, c.Name, c.City, c.Age, c.Id * 7, c.Name.Length, c.City.Length, c.Age * 2, c.Id)) + .Where(fun (struct (_, _, _, _, _, _, _, _, last)) -> last > 1) + .Select(fun (struct (_, name, _, _, _, _, _, _, _)) -> name)) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating (reading "ValueTuple`8" @ block) + Scenario.rows "Where over a query that selects a tuple" (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs index 44d598d3d13..8c1e684614e 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -43,6 +43,27 @@ let cases () = (fun db -> query { for c in db.Customers do select ((c.Id, c.Name), c.Age) }) (customers >> Seq.map (fun c -> (c.Id, c.Name), c.Age)) + Scenario.rows + "struct tuple of two" + (fun db -> query { for c in db.Customers do select (struct (c.Id, c.Name)) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name))) + + Scenario.rows + "struct tuple of nine" + (fun db -> + query { for c in db.Customers do select (struct (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id)) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name, c.City, c.Age, c.Id + 100, c.Name.Length, c.Age * 2, c.Id * 7, c.Age - c.Id))) + + Scenario.rows + "struct tuple inside a tuple" + (fun db -> query { for c in db.Customers do select (struct (c.Id, c.Name), c.Age) }) + (customers >> Seq.map (fun c -> struct (c.Id, c.Name), c.Age)) + + Scenario.rows + "tuple inside a struct tuple" + (fun db -> query { for c in db.Customers do select (struct ((c.Id, c.Name), c.Age)) }) + (customers >> Seq.map (fun c -> struct ((c.Id, c.Name), c.Age))) + Scenario.rows "tuple taken apart by the next for" (fun db -> @@ -54,6 +75,17 @@ let cases () = (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] + Scenario.rows + "struct tuple taken apart by the next for" + (fun db -> + query { + for struct (id, name) in (query { for c in db.Customers do select (struct (c.Id, c.Name)) }) do + where (id > 1) + select name + }) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + |> Scenario.violating [ Violation.ReadsUnnamedMember "ValueTuple`2" ] + Scenario.rows "record, fields in declaration order" (fun db -> query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }) From c3ec9ebb8d5301eb5130e53ed435ada4de190adc Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:12:20 +0100 Subject: [PATCH 03/13] Link the release note to the pull request Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index fd72f4da417..0aa52c42d5a 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,6 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) ### Changed From 526db2687cc2969dad617756d27d52727c487530 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:55:55 +0100 Subject: [PATCH 04/13] Follow every query chain when checking for unnamed member reads The shape tests followed only the first source of each operator. Each lambda parameter is now tied to the sequence it stands for, so the check also covers the inner input of a join, the second parameter of a join result selector, a collection selector of SelectMany, an identity Select, a chain inside the lambda of another operator and a query captured from earlier. A value scenario now has to end in a Queryable operator that gives the value: a count made over the rows of the provider leaves it. The validator itself is tested on trees built by hand. Co-Authored-By: Claude Sonnet 5.5 --- .../QueryProviderShape.fs | 171 +++++++++++-- .../Microsoft.FSharp.Linq/QueryScenario.fs | 12 +- .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 231 ++++++++++++++++++ 3 files changed, 389 insertions(+), 25 deletions(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index bad748d75ab..ae25aa7b1d6 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -294,35 +294,70 @@ module ProviderShape = let private yields (elementType: Type) (source: Expression) = typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type - /// Members read from elements that an earlier operator built without naming them: - /// the provider has to take apart an object it was not told how to. - let private unnamedMemberReads (expression: Expression) = + /// Whether the lambda gives its first parameter back as it is. + let private returnsParameter (lambda: LambdaExpression) = + lambda.Parameters.Count > 0 && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[0]) + + /// A query that is its own root, as opposed to one built from another by operators. + let private isRoot (query: IQueryable) = + match query.Expression with + | :? ConstantExpression as root -> obj.ReferenceEquals(root.Value, query) + | _ -> false + + /// Finds members read from elements that an earlier operator built without naming them: the provider has to + /// take apart an object it was not told how to. Every chain of operators is followed, the ones inside lambdas + /// and the ones of a query captured from earlier too, and each parameter of a lambda is tied to the sequence + /// it stands for, which is not always the first source of the operator. + type private UnnamedMemberFinder() = + inherit ExpressionVisitor() + let found = ResizeArray() - let rec producesUnnamed (expression: Expression) = - match expression with + // Whether the elements are unnamed, for the parameters that stand for a sequence: the group of a GroupJoin + let sequences = Dictionary(HashIdentity.Reference) + + let lambdaAt (call: MethodCallExpression) index = + if index < call.Arguments.Count then + match call.Arguments.[index] with + | Lambda lambda -> ValueSome lambda + | _ -> ValueNone + else + ValueNone + + // The parameter of the lambda stands for an element of the sequence, which was built without names + let report (lambda: LambdaExpression) index (sequence: Expression) (unnamed: bool) = + if unnamed && index < lambda.Parameters.Count then + let parameter = lambda.Parameters.[index] + + if yields parameter.Type sequence && readsMemberOf parameter lambda.Body then + found.Add(Violation.ReadsUnnamedMember parameter.Type.Name) + + member _.Found = found |> Seq.distinct |> List.ofSeq + + /// Whether the elements of the sequence were built without naming their members. + member this.Unnamed(sequence: Expression) : bool = + match sequence with | :? MethodCallExpression as call when isLinqOperator call -> let source = call.Arguments.[0] - let sourceUnnamed = producesUnnamed source let lambdas = - [ for argument in Seq.skip 1 call.Arguments do - match argument with - | Lambda lambda -> lambda - | _ -> () ] - - if sourceUnnamed then - for lambda in lambdas do - let element = lambda.Parameters.[0] - - if yields element.Type source && readsMemberOf element lambda.Body then - found.Add(Violation.ReadsUnnamedMember element.Type.Name) + [ for index in 1 .. call.Arguments.Count - 1 do + match lambdaAt call index with + | ValueSome lambda -> lambda + | ValueNone -> () ] match call.Method.Name with - | "Select" - | "SelectMany" + | "Select" -> + lambdas + |> List.tryLast + |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameter selector && this.Unnamed source)) + | "SelectMany" -> + match lambdas with + | [ collection ] -> this.Unnamed collection.Body + | [ _; result ] -> buildsUnnamed result.Body + | _ -> false | "Join" - | "GroupJoin" -> lambdas |> List.last |> _.Body |> buildsUnnamed + | "GroupJoin" -> lambdas |> List.tryLast |> Option.exists (fun result -> buildsUnnamed result.Body) | "Where" | "OrderBy" | "OrderByDescending" @@ -333,18 +368,108 @@ module ProviderShape = | "Take" | "TakeWhile" | "Distinct" - | "DefaultIfEmpty" -> sourceUnnamed + | "DefaultIfEmpty" + | "Reverse" + | "AsQueryable" + | "AsEnumerable" -> this.Unnamed source + | "Concat" + | "Union" + | "Intersect" + | "Except" -> this.Unnamed source || (call.Arguments.Count > 1 && this.Unnamed call.Arguments.[1]) + | _ -> false + | :? ConstantExpression as constant -> + match constant.Value with + | :? IQueryable as query when not (isRoot query) -> this.Unnamed query.Expression + | _ -> false + | :? ParameterExpression as parameter -> + match sequences.TryGetValue parameter with + | true, unnamed -> unnamed | _ -> false + | :? UnaryExpression as conversion when conversion.NodeType = ExpressionType.Convert -> this.Unnamed conversion.Operand | _ -> false - producesUnnamed expression |> ignore - List.ofSeq found + /// Reports the members that the lambdas of the operator read from elements built without names. + member this.Check(call: MethodCallExpression) = + let source = call.Arguments.[0] + + match call.Method.Name with + | "Join" + | "GroupJoin" when call.Arguments.Count >= 5 -> + let outer, inner = call.Arguments.[0], call.Arguments.[1] + let outerUnnamed, innerUnnamed = this.Unnamed outer, this.Unnamed inner + + lambdaAt call 2 |> ValueOption.iter (fun key -> report key 0 outer outerUnnamed) + lambdaAt call 3 |> ValueOption.iter (fun key -> report key 0 inner innerUnnamed) + + lambdaAt call 4 + |> ValueOption.iter (fun result -> + report result 0 outer outerUnnamed + + if call.Method.Name = "Join" then + report result 1 inner innerUnnamed + elif result.Parameters.Count > 1 then + // The second parameter is the group of the matching inner elements, not one of them + sequences.[result.Parameters.[1]] <- innerUnnamed) + | "SelectMany" when call.Arguments.Count >= 3 -> + // A collection selector, then a result selector over the element and an element of the collection + let sourceUnnamed = this.Unnamed source + let collection = lambdaAt call 1 + collection |> ValueOption.iter (fun selector -> report selector 0 source sourceUnnamed) + + lambdaAt call 2 + |> ValueOption.iter (fun result -> + report result 0 source sourceUnnamed + + collection + |> ValueOption.iter (fun selector -> report result 1 selector.Body (this.Unnamed selector.Body))) + | "Zip" when call.Arguments.Count = 3 -> + let second = call.Arguments.[1] + + lambdaAt call 2 + |> ValueOption.iter (fun result -> + report result 0 source (this.Unnamed source) + report result 1 second (this.Unnamed second)) + // The first parameter of the function is the accumulated value + | "Aggregate" -> () + | _ -> + let sourceUnnamed = this.Unnamed source + + for index in 1 .. call.Arguments.Count - 1 do + lambdaAt call index |> ValueOption.iter (fun lambda -> report lambda 0 source sourceUnnamed) + + override this.VisitMethodCall node = + if isLinqOperator node then + this.Check node + + base.VisitMethodCall node + + // A query built earlier and used in this one is part of what the provider translates. + override this.VisitConstant node = + match node.Value with + | :? IQueryable as query when not (isRoot query) -> this.Visit query.Expression |> ignore + | _ -> () + + node + + let private unnamedMemberReads (expression: Expression) = + let finder = UnnamedMemberFinder() + finder.Visit expression |> ignore + finder.Found let violations (expression: Expression) = let visitor = ShapeVisitor() visitor.Visit expression |> ignore visitor.Violations @ unnamedMemberReads expression |> List.distinct + /// Whether the expression calls a Queryable operator that gives a value and not a query, as a provider is asked + /// to execute when a query is counted, summed or searched. + let isTerminalOperator (expression: Expression) = + match expression with + | :? MethodCallExpression as call -> + call.Method.DeclaringType = typeof + && not (typeof.IsAssignableFrom call.Type) + | _ -> false + /// The LINQ operators along the source chain, outermost last. let spine (expression: Expression) = let rec walk (expression: Expression) operators = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index b2a270042e9..094519b89e0 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -133,10 +133,18 @@ module Scenario = Rows = [ for row in rows -> row ] } // The value comes from the last execution; whatever ran before it went to the provider too. | Outcome.Value value when provider.Executed.Count > 0 -> + let last = provider.Executed.[provider.Executed.Count - 1] let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) - { Expression = provider.Executed.[provider.Executed.Count - 1] - Violations = [ for expression in earlier do yield! ProviderShape.violations expression ] + { Expression = last + Violations = + [ for expression in earlier do + yield! ProviderShape.violations expression + + // Enumerating the rows of a query is an execution too, so the last one has to be the + // operator that gives the value: a count made over those rows is made by the client. + if not (ProviderShape.isTerminalOperator last) then + Violation.LeftProvider ] Rows = [ value ] } | Outcome.Value value -> { Expression = Expression.Empty() diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 6e284ed7d4d..0b2ce330374 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -2,6 +2,11 @@ namespace FSharp.Core.UnitTests.Linq +open System +open System.Collections.Generic +open System.Linq +open System.Linq.Expressions +open System.Reflection open Xunit type QueryShapeTests() = @@ -29,3 +34,229 @@ type QueryShapeTests() = [] member _.``Method syntax``(scenario: Scenario) = Scenario.check scenario + +/// Trees built by hand, the way a C# compiler builds them, to check the validator itself. They do not depend on how +/// FSharp.Core translates a query, so they mean the same in every branch. +module private ValidatorTrees = + + let customers = Queryable.AsQueryable Seq.empty + let orders = Queryable.AsQueryable Seq.empty + + let customerPair = typeof> + let orderPair = typeof> + + let call (name: string) (typeArguments: Type list) (arguments: Expression list) = + Expression.Call(typeof, name, Array.ofList typeArguments, Array.ofList arguments) :> Expression + + let quote (lambda: LambdaExpression) = Expression.Quote lambda :> Expression + + let pair (tuple: Type) (arguments: Expression list) (named: bool) = + let constructor = tuple.GetConstructors().[0] + + if named then + Expression.New(constructor, arguments, [| tuple.GetProperty "Item1" :> MemberInfo; tuple.GetProperty "Item2" :> MemberInfo |]) + else + Expression.New(constructor, arguments) + + /// customers.Select(c => new Tuple(c.Id, c.Name)), with or without naming the members + let customerPairs (named: bool) = + let c = Expression.Parameter(typeof, "c") + let body = pair customerPair [ Expression.Property(c, "Id"); Expression.Property(c, "Name") ] named + + call "Select" [ typeof; customerPair ] [ customers.Expression; quote (Expression.Lambda(body, c)) ] + + /// orders.Select(o => new Tuple(o.Id, o.Total)), with or without naming the members + let orderPairs (named: bool) = + let o = Expression.Parameter(typeof, "o") + let body = pair orderPair [ Expression.Property(o, "Id"); Expression.Property(o, "Total") ] named + + call "Select" [ typeof; orderPair ] [ orders.Expression; quote (Expression.Lambda(body, o)) ] + + /// source.Where(t => t.Item1 > 0) + let whereItem1Positive (source: Expression) (tuple: Type) = + let t = Expression.Parameter(tuple, "t") + let predicate = Expression.Lambda(Expression.GreaterThan(Expression.Property(t, "Item1"), Expression.Constant(0)), t) + + call "Where" [ tuple ] [ source; quote predicate ] + + let item1 (p: ParameterExpression) = Expression.Property(p, "Item1") :> Expression + let item2 (p: ParameterExpression) = Expression.Property(p, "Item2") :> Expression + let zero (_: ParameterExpression) = Expression.Constant(0) :> Expression + + /// outer.Join(inner, o => outerKey(o), i => innerKey(i), (o, i) => result(o, i)), with int keys + let join + (outer: Expression) + (outerElement: Type) + (inner: Expression) + (innerElement: Type) + (outerKey: ParameterExpression -> Expression) + (innerKey: ParameterExpression -> Expression) + (result: ParameterExpression -> ParameterExpression -> Expression) + = + let parameter (ty: Type) name = Expression.Parameter(ty, name) + let o1 = parameter outerElement "o" + let i1 = parameter innerElement "i" + let o2 = parameter outerElement "o" + let i2 = parameter innerElement "i" + let resultBody = result o2 i2 + + call + "Join" + [ outerElement; innerElement; typeof; resultBody.Type ] + [ outer + inner + quote (Expression.Lambda(outerKey o1, o1)) + quote (Expression.Lambda(innerKey i1, i1)) + quote (Expression.Lambda(resultBody, o2, i2)) ] + +type ProviderShapeValidatorTests() = + + static let unnamedRead = [ Violation.ReadsUnnamedMember "Tuple`2" ] + + static let reported tree = ProviderShape.violations tree + + [] + member _.``A member read from an unnamed projection is reported``() = + let unnamed = ValidatorTrees.whereItem1Positive (ValidatorTrees.customerPairs false) ValidatorTrees.customerPair + let named = ValidatorTrees.whereItem1Positive (ValidatorTrees.customerPairs true) ValidatorTrees.customerPair + + Assert.Equal(unnamedRead, reported unnamed) + Assert.Empty(reported named) + + [] + member _.``A chain inside the lambda of another operator is checked``() = + let nested named = + let inner = ValidatorTrees.whereItem1Positive (ValidatorTrees.orderPairs named) ValidatorTrees.orderPair + let any = ValidatorTrees.call "Any" [ ValidatorTrees.orderPair ] [ inner ] + let c = Expression.Parameter(typeof, "c") + + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(any, c)) ] + + Assert.Equal(unnamedRead, reported (nested false)) + Assert.Empty(reported (nested true)) + + [] + member _.``A query captured from earlier is checked where it is used``() = + let capturing named = + let captured = ValidatorTrees.customers.Provider.CreateQuery>(ValidatorTrees.customerPairs named) + let o = Expression.Parameter(typeof, "o") + let t = Expression.Parameter(ValidatorTrees.customerPair, "t") + let predicate = Expression.Lambda(Expression.GreaterThan(Expression.Property(t, "Item1"), Expression.Property(o, "Id")), t) + let any = ValidatorTrees.call "Any" [ ValidatorTrees.customerPair ] [ Expression.Constant(captured) :> Expression; ValidatorTrees.quote predicate ] + + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.orders.Expression; ValidatorTrees.quote (Expression.Lambda(any, o)) ] + + Assert.Equal(unnamedRead, reported (capturing false)) + Assert.Empty(reported (capturing true)) + + [] + member _.``An identity Select keeps the elements unnamed``() = + let t = Expression.Parameter(ValidatorTrees.customerPair, "t") + + let identity = + ValidatorTrees.call + "Select" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs false; ValidatorTrees.quote (Expression.Lambda(t, t)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive identity ValidatorTrees.customerPair)) + + [] + member _.``The elements of a collection selector stay unnamed after SelectMany``() = + let flattened named = + let c = Expression.Parameter(typeof, "c") + let collectionType = typedefof>.MakeGenericType(typeof, typeof>>) + let collection = Expression.Lambda(collectionType, ValidatorTrees.orderPairs named, c) + + ValidatorTrees.call + "SelectMany" + [ typeof; ValidatorTrees.orderPair ] + [ ValidatorTrees.customers.Expression; ValidatorTrees.quote collection ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (flattened false) ValidatorTrees.orderPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (flattened true) ValidatorTrees.orderPair)) + + [] + member _.``A named input of a join is not blamed for the other one being unnamed``() = + // The two inputs have the same tuple type, and only the outer one is unnamed. Neither key reads a member of it. + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Empty(reported tree) + + [] + member _.``An unnamed inner input of a join is checked against the inner key``() = + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``An unnamed inner input of a join is checked against the second parameter of the result``() = + let tree = + ValidatorTrees.join + (ValidatorTrees.customerPairs true) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs false) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.zero + (fun _ i -> ValidatorTrees.item2 i) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``An unnamed inner input of a join is checked when the outer input is a plain source``() = + let tree = + ValidatorTrees.join + ValidatorTrees.customers.Expression + typeof + (ValidatorTrees.orderPairs false) + ValidatorTrees.orderPair + (fun c -> Expression.Property(c, "Id") :> Expression) + ValidatorTrees.item1 + (fun _ _ -> ValidatorTrees.zero null) + + Assert.Equal(unnamedRead, reported tree) + + [] + member _.``A selector that is not a lambda is passed over``() = + let selector = + Expression.Constant( + Expression.Lambda>(Expression.Constant(0), Expression.Parameter(typeof, "c")), + typeof>> + ) + + let tree = + ValidatorTrees.call "Select" [ typeof; typeof ] [ ValidatorTrees.customers.Expression; selector :> Expression ] + + Assert.Empty(reported tree) + + [] + member _.``A count made by the provider passes``() = + Scenario.value "count by the provider" (fun db -> db.Customers.Where(fun c -> c.Age > 26).Count()) (fun _ -> 3) + |> Scenario.check + + [] + member _.``A count made over the rows of the provider leaves it``() = + Scenario.value "count on the client" (fun db -> db.Customers.Where(fun c -> c.Age > 26).AsEnumerable().Count()) (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check + + Scenario.value "count by Enumerable" (fun db -> Enumerable.Count(db.Customers.Where(fun c -> c.Age > 26))) (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check From 521e8300b0b76d62761543a093005f4c589e62c8 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 14:24:09 +0100 Subject: [PATCH 05/13] Count every execution and every violation when checking a scenario A value scenario was accepted when the last execution at the provider was an operator that gives a value. A count made by the client over rows taken from the provider passed whenever a function it ran asked the provider for a count of its own, because that count came last. Now every execution has to be such an operator, and so does the enumeration of a source taken as it is, which is recorded as an execution too. The violations of a tree are compared with every occurrence: a tree with one block was equal to a tree with two. The scenarios that have several of a kind are pinned with their number. The validator tests build a tree with an invoked lambda and one with an unbound parameter, which no scenario has, and the two client counts. The release note names the error a struct tuple gave on main. Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 2 +- .../QueryKeyScenarios.fs | 8 ++- .../QueryMethodSyntaxScenarios.fs | 14 +++-- .../QueryProviderShape.fs | 30 ++++++---- .../Microsoft.FSharp.Linq/QueryScenario.fs | 22 ++++--- .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 59 +++++++++++++++++++ 6 files changed, 107 insertions(+), 28 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 0aa52c42d5a..74a8bab02a3 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -6,6 +6,6 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) ### Changed diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs index 03011a4d99f..0c7e32749cb 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryKeyScenarios.fs @@ -8,6 +8,10 @@ open Microsoft.FSharp.Linq.RuntimeHelpers let private block = [ Violation.Statement ExpressionType.Block ] +/// A tree is pinned with every block it has: one that goes from three to two is a change as well +let private blocks count = + List.replicate count (Violation.Statement ExpressionType.Block) + let private customers (data: Data) = Seq.ofList data.Customers /// Orders paired with the lines that share both the order number and, as quantity, the customer number. @@ -68,7 +72,7 @@ let private joins () = select (o.Id, l.Id) }) matchingLines - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "join on an anonymous record, fields in alphabetical order" @@ -89,7 +93,7 @@ let private joins () = select (o.Id, l.Id) }) matchingLines - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "groupJoin on a tuple" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index 5325268456b..760c9b78a49 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -7,6 +7,10 @@ open System.Linq.Expressions let private block = [ Violation.Statement ExpressionType.Block ] +/// A tree is pinned with every block it has: one that goes from three to two is a change as well +let private blocks count = + List.replicate count (Violation.Statement ExpressionType.Block) + let private reading typeName = [ Violation.ReadsUnnamedMember typeName ] @@ -67,7 +71,7 @@ let cases () = "Where taking a tuple apart" (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name)).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ block) + |> Scenario.violating (reading "Tuple`2" @ blocks 2) Scenario.rows "Where taking a struct tuple apart" @@ -83,7 +87,7 @@ let cases () = .Where(fun (_, _, _, _, _, _, _, _, last) -> last > 1) .Select(fun (_, name, _, _, _, _, _, _, _) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "Tuple`8" @ block) + |> Scenario.violating (reading "Tuple`8" @ reading "Tuple`8" @ blocks 18) Scenario.rows "Where taking the last of a struct nine apart" @@ -93,19 +97,19 @@ let cases () = .Where(fun (struct (_, _, _, _, _, _, _, _, last)) -> last > 1) .Select(fun (struct (_, name, _, _, _, _, _, _, _)) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "ValueTuple`8" @ block) + |> Scenario.violating (reading "ValueTuple`8" @ reading "ValueTuple`8" @ blocks 2) Scenario.rows "Where over a query that selects a tuple" (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ block) + |> Scenario.violating (reading "Tuple`2" @ blocks 2) Scenario.rows "Where on a nullable property" (fun db -> db.Customers.Where(fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) (customers >> Seq.filter (fun c -> c.Registered.HasValue && c.Registered.Value.Year > 2020)) - |> Scenario.violating block + |> Scenario.violating (blocks 3) Scenario.rows "Where with a lambda over another source" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index ae25aa7b1d6..20665171824 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -25,12 +25,15 @@ type private InMemoryRewriter() = /// An in-memory query that stays on the provider that created it and prints its root by name. [] -type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption) = +type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, derived: Expression voption, scanned: Expression -> unit) = - let enumerate () = + let enumerate (self: Expression) = match derived with | ValueSome expression -> provider.Execute>(expression).GetEnumerator() - | ValueNone -> items.GetEnumerator() + | ValueNone -> + // No operator runs at the provider here: whoever enumerates the source takes all of its rows + scanned self + items.GetEnumerator() override _.ToString() = match derived with @@ -51,8 +54,8 @@ type CapturedQuery<'T>(provider: IQueryProvider, name: string, items: seq<'T>, d member _.ElementType = typeof<'T> member _.Provider = provider - member _.GetEnumerator() : IEnumerator<'T> = enumerate () - member _.GetEnumerator() : IEnumerator = enumerate () + member this.GetEnumerator() : IEnumerator<'T> = enumerate (Expression.Constant this) + member this.GetEnumerator() : IEnumerator = enumerate (Expression.Constant this) /// Executes in memory, and records what it was asked to execute. [] @@ -74,14 +77,19 @@ type CapturingQueryProvider() = finally depth <- depth - 1 - /// The expressions executed directly, not from inside another execution. + /// The expressions executed directly, not from inside another execution, and the sources that were enumerated. member _.Executed = executed :> IReadOnlyList + /// A source that is enumerated as it is, without an operator + member _.Scanned(source: Expression) = + if depth = 0 then + executed.Add source + member this.Source(name: string, items: seq<'T>) : IQueryable<'T> = - CapturedQuery<'T>(this, name, items, ValueNone) + CapturedQuery<'T>(this, name, items, ValueNone, this.Scanned) member this.Derived<'T>(expression: Expression) : IQueryable<'T> = - CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression) + CapturedQuery<'T>(this, "", Seq.empty, ValueSome expression, this.Scanned) interface IQueryProvider with member this.CreateQuery<'T>(expression: Expression) : IQueryable<'T> = this.Derived<'T> expression @@ -172,7 +180,7 @@ module ProviderShape = | _ -> ValueNone | _ -> ValueNone - member _.Violations = found |> Seq.distinct |> List.ofSeq + member _.Violations = List.ofSeq found override _.VisitLambda<'T>(node: Expression<'T>) = let added = bind node.Parameters @@ -332,7 +340,7 @@ module ProviderShape = if yields parameter.Type sequence && readsMemberOf parameter lambda.Body then found.Add(Violation.ReadsUnnamedMember parameter.Type.Name) - member _.Found = found |> Seq.distinct |> List.ofSeq + member _.Found = List.ofSeq found /// Whether the elements of the sequence were built without naming their members. member this.Unnamed(sequence: Expression) : bool = @@ -459,7 +467,7 @@ module ProviderShape = let violations (expression: Expression) = let visitor = ShapeVisitor() visitor.Visit expression |> ignore - visitor.Violations @ unnamedMemberReads expression |> List.distinct + visitor.Violations @ unnamedMemberReads expression /// Whether the expression calls a Queryable operator that gives a value and not a query, as a provider is asked /// to execute when a query is counted, summed or searched. diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 094519b89e0..042b108c16a 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -131,19 +131,22 @@ module Scenario = { Expression = rows.Expression Violations = (if stayed then [] else [ Violation.LeftProvider ]) Rows = [ for row in rows -> row ] } - // The value comes from the last execution; whatever ran before it went to the provider too. + // A query that is counted, summed or searched is executed by the operator that gives the value. + // Enumerating the rows of a query is an execution too, and so is every query that a function on the + // client runs while it goes through those rows: the provider made the value only if every + // execution is such an operator. The last one is the expression that is checked as a whole; whatever + // ran before it went to the provider too. | Outcome.Value value when provider.Executed.Count > 0 -> - let last = provider.Executed.[provider.Executed.Count - 1] - let earlier = provider.Executed |> Seq.take (provider.Executed.Count - 1) + let executed = List.ofSeq provider.Executed + let last = List.last executed + let earlier = List.take (executed.Length - 1) executed { Expression = last Violations = [ for expression in earlier do yield! ProviderShape.violations expression - // Enumerating the rows of a query is an execution too, so the last one has to be the - // operator that gives the value: a count made over those rows is made by the client. - if not (ProviderShape.isTerminalOperator last) then + if not (List.forall ProviderShape.isTerminalOperator executed) then Violation.LeftProvider ] Rows = [ value ] } | Outcome.Value value -> @@ -165,9 +168,10 @@ module Scenario = let failures (scenario: Scenario) = let data, observed = observe scenario + // Every occurrence counts: a tree with two blocks is not a tree with one let violations = observed.Violations @ ProviderShape.violations observed.Expression - |> List.distinct + |> List.sort let threw = violations @@ -175,8 +179,8 @@ module Scenario = | Violation.Throws _ -> true | _ -> false) - [ if set violations <> set scenario.Violations then - $"violations: expected %A{scenario.Violations}, got %A{violations}" + [ if violations <> List.sort scenario.Violations then + $"violations: expected %A{List.sort scenario.Violations}, got %A{violations}" if not threw then let expected = inOrder scenario.Ordered (scenario.Expected data) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 0b2ce330374..35c106eedab 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -246,6 +246,48 @@ type ProviderShapeValidatorTests() = Assert.Empty(reported tree) + [] + member _.``A block is reported once for each block``() = + let c = Expression.Parameter(typeof, "c") + + let block (name: string) = + let x = Expression.Variable(typeof, name) + Expression.Block([| x |], Expression.Assign(x, Expression.Property(c, "Id")), Expression.GreaterThan(x, Expression.Constant 0)) :> Expression + + let where (body: Expression) = + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(body, c)) ] + + let blockViolation = Violation.Statement ExpressionType.Block + + Assert.Equal([ blockViolation ], reported (where (block "x"))) + Assert.Equal([ blockViolation; blockViolation ], reported (where (Expression.AndAlso(block "x", block "y")))) + + [] + member _.``A lambda that is invoked is reported``() = + let c = Expression.Parameter(typeof, "c") + let n = Expression.Parameter(typeof, "n") + let positive = Expression.Lambda>(Expression.GreaterThan(n, Expression.Constant 0), n) + let tree = Expression.Invoke(positive, Expression.Property(c, "Id")) + + let where = + ValidatorTrees.call "Where" [ typeof ] [ ValidatorTrees.customers.Expression; ValidatorTrees.quote (Expression.Lambda(tree, c)) ] + + Assert.Equal([ Violation.LambdaInvocation ], reported where) + + [] + member _.``A parameter that no lambda binds is reported``() = + let c = Expression.Parameter(typeof, "c") + let n = Expression.Parameter(typeof, "n") + + let where = + ValidatorTrees.call + "Where" + [ typeof ] + [ ValidatorTrees.customers.Expression + ValidatorTrees.quote (Expression.Lambda(Expression.GreaterThan(Expression.Property(c, "Id"), n), c)) ] + + Assert.Equal([ Violation.UnboundParameter "n" ], reported where) + [] member _.``A count made by the provider passes``() = Scenario.value "count by the provider" (fun db -> db.Customers.Where(fun c -> c.Age > 26).Count()) (fun _ -> 3) @@ -260,3 +302,20 @@ type ProviderShapeValidatorTests() = Scenario.value "count by Enumerable" (fun db -> Enumerable.Count(db.Customers.Where(fun c -> c.Age > 26))) (fun _ -> 3) |> Scenario.violating [ Violation.LeftProvider ] |> Scenario.check + + [] + member _.``A count made by the client over counts made by the provider leaves it``() = + // The provider counts the orders once for each customer, and the client counts the customers + Scenario.value + "client count with a provider count in its projection" + (fun db -> db.Customers.Where(fun c -> c.Age > 26).AsEnumerable().Select(fun _ -> db.Orders.Count()).Count()) + (fun _ -> 3) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check + + Scenario.value + "client count with an inner provider count" + (fun db -> Enumerable.Count(db.Customers.Select(fun _ -> db.Orders.Count()))) + (fun data -> data.Customers.Length) + |> Scenario.violating [ Violation.LeftProvider ] + |> Scenario.check From b418d09d7657b552a834fbfb4d834fa78cf09b1a Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 17:37:24 +0100 Subject: [PATCH 06/13] Check the elements a result selector hands on, and compare entities by what they hold The shape tests did not see an unnamed element that passes through a result selector giving one of its parameters back, in SelectMany, Join and Zip, and they skipped the element parameter of Aggregate together with the accumulated value. Both are checked now. A function with a seed takes the accumulated value and an element, and one without a seed takes two elements. A scenario compared an entity by the text it makes of itself, which names its key only: a result with the right key and the wrong name or age passed. An object of a class of the scenarios is now compared by every property that gives a plain value. Co-Authored-By: Claude Sonnet 5.5 --- .../QueryProviderShape.fs | 38 ++++- .../Microsoft.FSharp.Linq/QueryScenario.fs | 32 ++++ .../Microsoft.FSharp.Linq/QueryShapeTests.fs | 146 ++++++++++++++++++ 3 files changed, 208 insertions(+), 8 deletions(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs index 20665171824..8709087cd59 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProviderShape.fs @@ -302,9 +302,9 @@ module ProviderShape = let private yields (elementType: Type) (source: Expression) = typedefof>.MakeGenericType(elementType).IsAssignableFrom source.Type - /// Whether the lambda gives its first parameter back as it is. - let private returnsParameter (lambda: LambdaExpression) = - lambda.Parameters.Count > 0 && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[0]) + /// Whether the lambda gives the parameter at the index back as it is. + let private returnsParameterAt index (lambda: LambdaExpression) = + index < lambda.Parameters.Count && obj.ReferenceEquals(lambda.Body, lambda.Parameters.[index]) /// A query that is its own root, as opposed to one built from another by operators. let private isRoot (query: IQueryable) = @@ -354,18 +354,30 @@ module ProviderShape = | ValueSome lambda -> lambda | ValueNone -> () ] + // A function of an element of each of two sequences gives unnamed elements when it builds them without + // names, and when it gives back one of its parameters of which the sequence has unnamed elements + let unnamedResult (result: LambdaExpression) (second: Expression) = + buildsUnnamed result.Body + || (returnsParameterAt 0 result && this.Unnamed source) + || (returnsParameterAt 1 result && this.Unnamed second) + match call.Method.Name with | "Select" -> lambdas |> List.tryLast - |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameter selector && this.Unnamed source)) + |> Option.exists (fun selector -> buildsUnnamed selector.Body || (returnsParameterAt 0 selector && this.Unnamed source)) | "SelectMany" -> match lambdas with | [ collection ] -> this.Unnamed collection.Body - | [ _; result ] -> buildsUnnamed result.Body + | [ collection; result ] -> unnamedResult result collection.Body | _ -> false | "Join" - | "GroupJoin" -> lambdas |> List.tryLast |> Option.exists (fun result -> buildsUnnamed result.Body) + | "Zip" -> lambdas |> List.tryLast |> Option.exists (fun result -> unnamedResult result call.Arguments.[1]) + // The second parameter is the group of the matching inner elements, which no element is + | "GroupJoin" -> + lambdas + |> List.tryLast + |> Option.exists (fun result -> buildsUnnamed result.Body || (returnsParameterAt 0 result && this.Unnamed source)) | "Where" | "OrderBy" | "OrderByDescending" @@ -437,8 +449,18 @@ module ProviderShape = |> ValueOption.iter (fun result -> report result 0 source (this.Unnamed source) report result 1 second (this.Unnamed second)) - // The first parameter of the function is the accumulated value - | "Aggregate" -> () + // The function takes the accumulated value, then an element. With no seed the accumulated value starts as + // an element, and a seed is of a type of its own, which is no element. + | "Aggregate" -> + let sourceUnnamed = this.Unnamed source + let seeded = call.Arguments.Count > 2 + + lambdaAt call (if seeded then 2 else 1) + |> ValueOption.iter (fun func -> + if not seeded then + report func 0 source sourceUnnamed + + report func 1 source sourceUnnamed) | _ -> let sourceUnnamed = this.Unnamed source diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 042b108c16a..9ae260a7226 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -60,6 +60,35 @@ module Scenario = else ValueNone + let rec private isPlain (ty: Type) = + ty.IsPrimitive + || ty.IsEnum + || ty = typeof + || ty = typeof + || ty = typeof + || ty = typeof + || ty = typeof + || (match Nullable.GetUnderlyingType ty with + | null -> false + | underlying -> isPlain underlying) + + /// An object of a class of the scenarios, with what it holds in the properties that give plain values. Its own + /// text says no more than the key of an entity, and a result that is wrong in any other property is not equal to + /// the right one. + [] + let private (|Entity|_|) (value: obj) = + let ty = value.GetType() + + if ty.IsClass && String.Equals(ty.Namespace, typeof.Namespace, StringComparison.Ordinal) then + ValueSome( + ty.GetProperties(BindingFlags.Public ||| BindingFlags.Instance) + |> Seq.filter (fun property -> property.GetIndexParameters().Length = 0 && isPlain property.PropertyType) + |> Seq.map (fun property -> struct (property.Name, property.GetValue value)) + |> Seq.toList + ) + else + ValueNone + /// Text that is equal for equal results, whichever provider materialized them. let rec render (value: obj) = let renderAll (values: seq) = @@ -76,6 +105,9 @@ module Scenario = let rendered = fields |> Seq.map (fun (name, field) -> $"{name} = {render field}") $"""{{ {String.concat "; " rendered} }}""" | Union(case, fields) -> $"{case}({renderAll fields})" + | Entity properties -> + let rendered = properties |> Seq.map (fun (struct (name, property)) -> $"{name} = {render property}") + $"""{value.GetType().Name} {{ {String.concat "; " rendered} }}""" | value -> string value let rows name (run: Db -> IQueryable<'T>) (expected: Data -> seq<'T>) = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 35c106eedab..94b9d25edf6 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -319,3 +319,149 @@ type ProviderShapeValidatorTests() = (fun data -> data.Customers.Length) |> Scenario.violating [ Violation.LeftProvider ] |> Scenario.check + + [] + member _.``A result selector that gives back an unnamed element keeps the elements unnamed``() = + let item = Expression.Parameter(typeof, "item") + + let flattened named = + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let collectionType = typedefof>.MakeGenericType(ValidatorTrees.customerPair, typeof>) + let collection = Expression.Lambda(collectionType, Expression.NewArrayInit(typeof, Expression.Constant 0), pair) + let first = Expression.Parameter(ValidatorTrees.customerPair, "pair") + + ValidatorTrees.call + "SelectMany" + [ ValidatorTrees.customerPair; typeof; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs named + ValidatorTrees.quote collection + ValidatorTrees.quote (Expression.Lambda(first, first, item)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (flattened false) ValidatorTrees.customerPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (flattened true) ValidatorTrees.customerPair)) + + [] + member _.``The second parameter of a SelectMany result selector is an element of the collection``() = + let collected named = + let customer = Expression.Parameter(typeof, "c") + let collectionType = typedefof>.MakeGenericType(typeof, typeof>>) + let collection = Expression.Lambda(collectionType, ValidatorTrees.orderPairs named, customer) + let owner = Expression.Parameter(typeof, "c") + let order = Expression.Parameter(ValidatorTrees.orderPair, "o") + + ValidatorTrees.call + "SelectMany" + [ typeof; ValidatorTrees.orderPair; ValidatorTrees.orderPair ] + [ ValidatorTrees.customers.Expression + ValidatorTrees.quote collection + ValidatorTrees.quote (Expression.Lambda(order, owner, order)) ] + + Assert.Equal(unnamedRead, reported (ValidatorTrees.whereItem1Positive (collected false) ValidatorTrees.orderPair)) + Assert.Empty(reported (ValidatorTrees.whereItem1Positive (collected true) ValidatorTrees.orderPair)) + + [] + member _.``A join or a zip that gives back an unnamed element keeps the elements unnamed``() = + let joined outerNamed innerNamed (result: ParameterExpression -> ParameterExpression -> Expression) = + ValidatorTrees.join + (ValidatorTrees.customerPairs outerNamed) + ValidatorTrees.customerPair + (ValidatorTrees.customerPairs innerNamed) + ValidatorTrees.customerPair + ValidatorTrees.zero + ValidatorTrees.zero + result + + let check tree = reported (ValidatorTrees.whereItem1Positive tree ValidatorTrees.customerPair) + + Assert.Equal(unnamedRead, check (joined false true (fun o _ -> o))) + Assert.Empty(check (joined true false (fun o _ -> o))) + Assert.Equal(unnamedRead, check (joined true false (fun _ i -> i))) + Assert.Empty(check (joined false true (fun _ i -> i))) + + [] + member _.``A zip that gives back an unnamed element keeps the elements unnamed``() = + let zipped firstNamed secondNamed = + let a = Expression.Parameter(ValidatorTrees.customerPair, "a") + let b = Expression.Parameter(ValidatorTrees.customerPair, "b") + + ValidatorTrees.whereItem1Positive + (ValidatorTrees.call + "Zip" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs firstNamed + ValidatorTrees.customerPairs secondNamed + ValidatorTrees.quote (Expression.Lambda(a, a, b)) ]) + ValidatorTrees.customerPair + + Assert.Equal(unnamedRead, reported (zipped false true)) + Assert.Empty(reported (zipped true false)) + + [] + member _.``The element an Aggregate function takes is checked, and a seed that is accumulated is not``() = + // customers.Select(c => new Tuple(c.Id, c.Name)).Aggregate(0, (acc, pair) => acc + pair.Item1) + let summed named = + let acc = Expression.Parameter(typeof, "acc") + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let func = Expression.Lambda(Expression.Add(acc, ValidatorTrees.item1 pair), acc, pair) + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair; typeof ] + [ ValidatorTrees.customerPairs named; Expression.Constant 0; ValidatorTrees.quote func ] + + Assert.Equal(unnamedRead, reported (summed false)) + Assert.Empty(reported (summed true)) + + // The accumulated value has the type of the elements here, and a member read from it is no read from an element + let counted = + let acc = Expression.Parameter(ValidatorTrees.customerPair, "acc") + let pair = Expression.Parameter(ValidatorTrees.customerPair, "pair") + let seed = ValidatorTrees.pair ValidatorTrees.customerPair [ Expression.Constant 0; Expression.Constant "" ] true + let count = Expression.Add(ValidatorTrees.item1 acc, Expression.Constant 1) + let next = ValidatorTrees.pair ValidatorTrees.customerPair [ count; Expression.Constant "" ] true + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair; ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs false; seed :> Expression; ValidatorTrees.quote (Expression.Lambda(next, acc, pair)) ] + + Assert.Empty(reported counted) + + [] + member _.``Both parameters of an Aggregate function with no seed are elements``() = + // customers.Select(c => new Tuple(c.Id, c.Name)).Aggregate((a, b) => new Tuple(a.Item1 + b.Item1, a.Item2)) + let combined named = + let a = Expression.Parameter(ValidatorTrees.customerPair, "a") + let b = Expression.Parameter(ValidatorTrees.customerPair, "b") + let sum = Expression.Add(ValidatorTrees.item1 a, ValidatorTrees.item1 b) + let body = ValidatorTrees.pair ValidatorTrees.customerPair [ sum; ValidatorTrees.item2 a ] true + + ValidatorTrees.call + "Aggregate" + [ ValidatorTrees.customerPair ] + [ ValidatorTrees.customerPairs named; ValidatorTrees.quote (Expression.Lambda(body, a, b)) ] + + Assert.Equal(unnamedRead @ unnamedRead, reported (combined false)) + Assert.Empty(reported (combined true)) + + [] + member _.``An entity that is wrong in a property other than its key does not pass``() = + let fails (build: Customer -> Customer) = + let scenario = + Scenario.value + "payload" + (fun db -> + query { + for c in db.Customers do + select (build c) into p + head + }) + (fun data -> data.Customers.[0]) + + Scenario.failures scenario |> List.exists (fun failure -> failure.Contains("rows: expected", StringComparison.Ordinal)) + + let copy (c: Customer) = Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age, Registered = c.Registered) + + Assert.True(fails (fun c -> Customer(Id = c.Id, Name = "CORRUPTED", City = c.City, Age = -1, Registered = c.Registered))) + Assert.True(fails (fun c -> Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age))) + Assert.False(fails copy) From 5ba518829ca80048f99dafc8320702d44fd0a321 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Wed, 4 Nov 2026 10:23:54 +0100 Subject: [PATCH 07/13] Spell the check for a wrong row with IndexOf, which net472 has String.Contains(string, StringComparison) does not exist on .NET Framework, so the test project failed to compile for net472 and all the Windows legs of the CI went red. IndexOf with an ordinal comparison does the same on both. Co-Authored-By: Claude Sonnet 5.5 --- .../FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs index 94b9d25edf6..a9cc9d9535f 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryShapeTests.fs @@ -458,7 +458,7 @@ type ProviderShapeValidatorTests() = }) (fun data -> data.Customers.[0]) - Scenario.failures scenario |> List.exists (fun failure -> failure.Contains("rows: expected", StringComparison.Ordinal)) + Scenario.failures scenario |> List.exists (fun failure -> failure.IndexOf("rows: expected", StringComparison.Ordinal) >= 0) let copy (c: Customer) = Customer(Id = c.Id, Name = c.Name, City = c.City, Age = c.Age, Registered = c.Registered) From f7f6b4d7a6683b1c63340a10b5d18baa6820f840 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Wed, 4 Nov 2026 10:39:14 +0100 Subject: [PATCH 08/13] Render the properties of an entity in the order of their names Reflection promises no order of the properties of a class. On .NET Framework one scenario listed the properties of the first order it rendered in another order than those of the three after it, so the expected and the actual rows of the same orders differed as text. The names decide the order now. Co-Authored-By: Claude Sonnet 5.5 --- .../FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs index 9ae260a7226..51d51d47696 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryScenario.fs @@ -83,6 +83,8 @@ module Scenario = ValueSome( ty.GetProperties(BindingFlags.Public ||| BindingFlags.Instance) |> Seq.filter (fun property -> property.GetIndexParameters().Length = 0 && isPlain property.PropertyType) + // Reflection promises no order of the properties, and on .NET Framework two calls for one class gave two orders + |> Seq.sortBy (fun property -> property.Name) |> Seq.map (fun property -> struct (property.Name, property.GetValue value)) |> Seq.toList ) From 6e5197e3a04c922aefbafd974a82b4c7ee8888f6 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 27 Sep 2026 20:31:46 +0200 Subject: [PATCH 09/13] Name the members of tuples and records built in expression trees A tuple, struct tuple, record or anonymous record was built with Expression.New(ctor, args) and no Members. A provider can materialize such an object at the end of a query, but it cannot tell which argument a later read of Item1 or of a field refers to, so EF Core failed on any query composed over a tuple or record projection - a Where on the result of select (c.Id, c.Name), or a record read by the next for - and Cosmos rejected every such projection as a constructor call. The constructions now name the member each argument becomes, as C# anonymous types do: the ItemN and Rest properties of System.Tuple, the fields of System.ValueTuple, and the fields of a record, whose getters have to be public for that; otherwise the record is built as before. AnonymousObject members are taken in Item1..ItemN order rather than in whatever order reflection returns them. Co-Authored-By: Claude Opus 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 3 + .../release-notes/PENDING_BREAKING_CHANGES.md | 12 +++ src/FSharp.Core/Linq.fs | 28 +++++-- .../FSharp.Core.UnitTests.fsproj | 1 + .../ExpressionNewMembersTests.fs | 65 +++++++++++++++ .../QueryMethodSyntaxScenarios.fs | 80 +++++++++++++++++-- .../QueryProjectionScenarios.fs | 33 +++++++- .../QueryStructureScenarios.fs | 3 +- 8 files changed, 208 insertions(+), 17 deletions(-) create mode 100644 tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 74a8bab02a3..cbab1a059c7 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -7,5 +7,8 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) +* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) +* Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40)) ### Changed diff --git a/docs/release-notes/PENDING_BREAKING_CHANGES.md b/docs/release-notes/PENDING_BREAKING_CHANGES.md index df80c994bea..5f4ac528707 100644 --- a/docs/release-notes/PENDING_BREAKING_CHANGES.md +++ b/docs/release-notes/PENDING_BREAKING_CHANGES.md @@ -52,6 +52,18 @@ a.Equals(b) // Was: false, Now: true // New: if (node.NodeType == ExpressionType.ArrayIndex) ``` +### Tuple and record constructions name their members + +Constructing a tuple, struct tuple, record or anonymous record now gives a `NewExpression` whose `Members` say which property (or `ValueTuple` field) each argument becomes, as C# anonymous types do, so providers can bind a later read such as `t.Item1` or `r.Name` to the argument. + +```fsharp +<@ fun (c: Customer) -> (c.Id, c.Name) @> +// Was: new Tuple`2(c.Id, c.Name) +// Now: new Tuple`2(Item1 = c.Id, Item2 = c.Name) +``` + +A record whose fields have no public getter is still built without `Members`. + --- ## IQueryable Type Preservation 🟡 diff --git a/src/FSharp.Core/Linq.fs b/src/FSharp.Core/Linq.fs index 431f6fd9514..2ee5b47bbff 100644 --- a/src/FSharp.Core/Linq.fs +++ b/src/FSharp.Core/Linq.fs @@ -78,6 +78,18 @@ module LeafExpressionConverter = let getNonNullableType typ = match Nullable.GetUnderlyingType typ with null -> typ | t -> t + let tupleMemberNames = [| "Item1"; "Item2"; "Item3"; "Item4"; "Item5"; "Item6"; "Item7"; "Rest" |] + + /// The properties of a System.Tuple, or the fields of a System.ValueTuple, that its constructor arguments become. + let tupleMembers (tupleTy: Type) = + [| for name in tupleMemberNames.[.. tupleTy.GetGenericArguments().Length - 1] -> + if tupleTy.IsValueType then tupleTy.GetField name :> MemberInfo else tupleTy.GetProperty name :> MemberInfo |] + + let hasPublicGetter (property: PropertyInfo) = + match property.GetGetMethod() with + | null -> false + | _ -> true + // https://github.com/dotnet/runtime/blob/fa779e8cb2b5868a0ac2fd4215f39ffb91f0dab0/src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/TypeUtils.cs#L72 /// Can LINQ Expressions' BinaryExpression's (Left/Right)Shift construct a SimpleBinaryExpression from the type in question? Otherwise, use the F# operator as the user-defined method. let isLinqExpressionsInteger typ = @@ -522,8 +534,8 @@ module LeafExpressionConverter = // Anonymous type initialization generated by LinqQueries | NewAnonymousObjectQ(ctor, args) -> let argsR = ConvExprsToLinq env args - let props = ctor.DeclaringType.GetProperties() - Expression.New(ctor, argsR, [| for p in props -> (p :> MemberInfo) |]) |> asExpr + let anonTy = ctor.DeclaringType + Expression.New(ctor, argsR, [| for i in 1 .. args.Length -> anonTy.GetProperty("Item" + string i) :> MemberInfo |]) |> asExpr // Do the same thing as C# compiler for string addition | PlusQ (_, GenericArgs [|ty1; ty2; ty3|], [x1; x2]) @@ -707,7 +719,13 @@ module LeafExpressionConverter = // Expr.New* | NewRecord(recdTy, args) -> let ctorInfo = FSharpValue.PreComputeRecordConstructorInfo(recdTy, showAll) - Expression.New(ctorInfo, ConvExprsToLinq env args) |> asExpr + let argsP = ConvExprsToLinq env args + let fields = FSharpType.GetRecordFields(recdTy, showAll) + // Expression.New rejects a member it cannot read publicly, such as a field of a record with a private representation + if fields |> Array.forall hasPublicGetter then + Expression.New(ctorInfo, argsP, [| for field in fields -> field :> MemberInfo |]) |> asExpr + else + Expression.New(ctorInfo, argsP) |> asExpr | NewArray(ty, args) -> Expression.NewArrayInit(ty, ConvExprsToLinq env args) |> asExpr @@ -757,10 +775,10 @@ module LeafExpressionConverter = let argsP = ConvExprsToLinq env args let rec build ty (argsP: Expression array) = match FSharpValue.PreComputeTupleConstructorInfo ty with - | ctorInfo, None -> Expression.New(ctorInfo, argsP) |> asExpr + | ctorInfo, None -> Expression.New(ctorInfo, argsP, tupleMembers ty) |> asExpr | ctorInfo, Some nestedTy -> let n = ctorInfo.GetParameters().Length - 1 - Expression.New(ctorInfo, Array.append argsP.[0..n-1] [| build nestedTy argsP.[n..] |]) |> asExpr + Expression.New(ctorInfo, Array.append argsP.[0..n-1] [| build nestedTy argsP.[n..] |], tupleMembers ty) |> asExpr build tupTy argsP | IfThenElse(g, t, e) -> diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj index 3a20b60f7e4..ffe3e88f9aa 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj +++ b/tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj @@ -101,6 +101,7 @@ + diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs new file mode 100644 index 00000000000..4783ea20035 --- /dev/null +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Core.UnitTests.Linq.ExpressionNewMembersTests + +open System +open System.Linq.Expressions +open Xunit +open Microsoft.FSharp.Linq.RuntimeHelpers + +type PrivateRepresentation = private { Hidden: int } + +let private construction (lambda: Expression<'T>) = + match lambda.Body with + | :? NewExpression as construction -> construction + | body -> failwith $"Expected a New, got {body}" + +let private memberNames (construction: NewExpression) = + [ for m in construction.Members -> m.Name ] + +[] +let ``Tuple construction names the property each argument becomes`` () = + let tuple = construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun x -> (x, "a")) @>) + Assert.Equal([ "Item1"; "Item2" ], memberNames tuple) + Assert.All(tuple.Members, fun m -> Assert.Equal(Reflection.MemberTypes.Property, m.MemberType)) + +[] +let ``Struct tuple construction names the field each argument becomes`` () = + let tuple = + construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun x -> struct (x, "a")) @>) + + Assert.Equal([ "Item1"; "Item2" ], memberNames tuple) + Assert.All(tuple.Members, fun m -> Assert.Equal(Reflection.MemberTypes.Field, m.MemberType)) + +[] +let ``Tuple of nine names Rest and the members of the nested tuple`` () = + let tuple = + construction ( + LeafExpressionConverter.QuotationToLambdaExpression + <@ Func(fun x -> (x, x, x, x, x, x, x, x, x)) @> + ) + + Assert.Equal([ "Item1"; "Item2"; "Item3"; "Item4"; "Item5"; "Item6"; "Item7"; "Rest" ], memberNames tuple) + + match tuple.Arguments.[7] with + | :? NewExpression as rest -> Assert.Equal([ "Item1"; "Item2" ], memberNames rest) + | rest -> Assert.Fail $"Expected Rest to be a New, got {rest}" + +[] +let ``Record construction names the field each argument becomes`` () = + let record = + construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun c -> { PersonName = c.Name; PersonAge = c.Age }) @>) + + Assert.Equal([ "PersonName"; "PersonAge" ], memberNames record) + +[] +let ``Anonymous record construction names the field each argument becomes`` () = + let record = + construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun c -> {| Age = c.Age; Name = c.Name |}) @>) + + Assert.Equal([ "Age"; "Name" ], memberNames record) + +[] +let ``Record whose fields cannot be read publicly is built without members`` () = + let record = construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun x -> { Hidden = x }) @>) + Assert.Null record.Members diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index 760c9b78a49..8b12e720a26 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -11,9 +11,6 @@ let private block = [ Violation.Statement ExpressionType.Block ] let private blocks count = List.replicate count (Violation.Statement ExpressionType.Block) -let private reading typeName = - [ Violation.ReadsUnnamedMember typeName ] - let private customers (data: Data) = Seq.ofList data.Customers let cases () = @@ -71,13 +68,75 @@ let cases () = "Where taking a tuple apart" (fun db -> db.Customers.Select(fun c -> (c.Id, c.Name)).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ blocks 2) + |> Scenario.violating (blocks 2) Scenario.rows "Where taking a struct tuple apart" (fun db -> db.Customers.Select(fun c -> struct (c.Id, c.Name)).Where(fun (struct (id, _)) -> id > 1)) (customers >> Seq.map (fun c -> struct (c.Id, c.Name)) >> Seq.filter (fun (struct (id, _)) -> id > 1)) - |> Scenario.violating (reading "ValueTuple`2" @ block) + |> Scenario.violating block + + Scenario.rows + "OrderBy taking a tuple apart" + (fun db -> db.Customers.Select(fun c -> (c.Name, c.Age)).OrderBy(fun (_, age) -> age).Select(fun (name, _) -> name)) + (customers >> Seq.sortBy _.Age >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.violating block + + Scenario.rows + "OrderBy taking a struct tuple apart" + (fun db -> + db.Customers.Select(fun c -> struct (c.Name, c.Age)).OrderBy(fun (struct (_, age)) -> age).Select(fun (struct (name, _)) -> name)) + (customers >> Seq.sortBy _.Age >> Seq.map _.Name) + |> Scenario.ordered + |> Scenario.violating block + + Scenario.rows + "GroupBy taking a tuple apart" + (fun db -> db.Customers.Select(fun c -> (c.City, c.Age)).GroupBy(fun (city, _) -> city).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy _.City) + |> Scenario.violating block + + Scenario.rows + "GroupBy taking a struct tuple apart" + (fun db -> + db.Customers.Select(fun c -> struct (c.City, c.Age)).GroupBy(fun (struct (city, _)) -> city).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy _.City) + |> Scenario.violating block + + Scenario.rows + "Where reading a record" + (fun db -> db.Customers.Select(fun c -> { PersonName = c.Name; PersonAge = c.Age }).Where(fun p -> p.PersonAge > 26)) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age }) >> Seq.filter (fun p -> p.PersonAge > 26)) + + Scenario.rows + "OrderBy reading a record" + (fun db -> + db.Customers.Select(fun c -> { PersonName = c.Name; PersonAge = c.Age }).OrderBy(fun p -> p.PersonAge).Select(fun p -> p.PersonName)) + (customers >> Seq.sortBy _.Age >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "GroupBy reading a record" + (fun db -> + db.Customers.Select(fun c -> { KeyCity = c.City; KeyAge = c.Age }).GroupBy(fun k -> k.KeyCity).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy _.City) + + Scenario.rows + "Where reading an anonymous record" + (fun db -> db.Customers.Select(fun c -> {| Age = c.Age; Name = c.Name |}).Where(fun p -> p.Age > 26)) + (customers >> Seq.map (fun c -> {| Age = c.Age; Name = c.Name |}) >> Seq.filter (fun p -> p.Age > 26)) + + Scenario.rows + "OrderBy reading an anonymous record" + (fun db -> db.Customers.Select(fun c -> {| Age = c.Age; Name = c.Name |}).OrderBy(fun p -> p.Age).Select(fun p -> p.Name)) + (customers >> Seq.sortBy _.Age >> Seq.map _.Name) + |> Scenario.ordered + + Scenario.rows + "GroupBy reading an anonymous record" + (fun db -> db.Customers.Select(fun c -> {| Age = c.Age; City = c.City |}).GroupBy(fun p -> p.City).Select(fun g -> (g.Key, g.Count()))) + (customers >> Seq.countBy _.City) Scenario.rows "Where taking the last of nine apart" @@ -87,7 +146,7 @@ let cases () = .Where(fun (_, _, _, _, _, _, _, _, last) -> last > 1) .Select(fun (_, name, _, _, _, _, _, _, _) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "Tuple`8" @ reading "Tuple`8" @ blocks 18) + |> Scenario.violating (blocks 18) Scenario.rows "Where taking the last of a struct nine apart" @@ -97,13 +156,18 @@ let cases () = .Where(fun (struct (_, _, _, _, _, _, _, _, last)) -> last > 1) .Select(fun (struct (_, name, _, _, _, _, _, _, _)) -> name)) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating (reading "ValueTuple`8" @ reading "ValueTuple`8" @ blocks 2) + |> Scenario.violating (blocks 2) Scenario.rows "Where over a query that selects a tuple" (fun db -> (query { for c in db.Customers do select (c.Id, c.Name) }).Where(fun (id, _) -> id > 1)) (customers >> Seq.map (fun c -> c.Id, c.Name) >> Seq.filter (fun (id, _) -> id > 1)) - |> Scenario.violating (reading "Tuple`2" @ blocks 2) + |> Scenario.violating (blocks 2) + + Scenario.rows + "Where over a query that selects a record" + (fun db -> (query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }).Where(fun p -> p.PersonAge > 26)) + (customers >> Seq.map (fun c -> { PersonName = c.Name; PersonAge = c.Age }) >> Seq.filter (fun p -> p.PersonAge > 26)) Scenario.rows "Where on a nullable property" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs index 8c1e684614e..c967f951119 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -73,7 +73,6 @@ let cases () = select name }) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] Scenario.rows "struct tuple taken apart by the next for" @@ -84,7 +83,37 @@ let cases () = select name }) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) - |> Scenario.violating [ Violation.ReadsUnnamedMember "ValueTuple`2" ] + + Scenario.rows + "tuple of nine taken apart by the next for" + (fun db -> + query { + for (_, name, _, _, _, _, _, _, last) in + (query { for c in db.Customers do select (c.Id, c.Name, c.City, c.Age, c.Id, c.Name, c.City, c.Age, c.Id) }) do + where (last > 1) + select name + }) + (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + + Scenario.rows + "record read by the next for" + (fun db -> + query { + for p in (query { for c in db.Customers do select { PersonName = c.Name; PersonAge = c.Age } }) do + where (p.PersonAge > 26) + select p.PersonName + }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) + + Scenario.rows + "anonymous record read by the next for" + (fun db -> + query { + for p in (query { for c in db.Customers do select {| Age = c.Age; Name = c.Name |} }) do + where (p.Age > 26) + select p.Name + }) + (customers >> Seq.filter (fun c -> c.Age > 26) >> Seq.map _.Name) Scenario.rows "record, fields in declaration order" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs index 6f806f4bb43..8c35edb3bb1 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryStructureScenarios.fs @@ -154,8 +154,7 @@ let private loops () = Scenario.value "exists over two for" (fun db -> query { for c in db.Customers do for o in c.Orders do exists (o.Total > 10.0 && c.City = "London") }) - (fun _ -> true) - |> Scenario.violating [ Violation.ReadsUnnamedMember "Tuple`2" ] ] + (fun _ -> true) ] let private subqueries () = [ Scenario.rows From f7bdf97574239803fad79016b16de1512360ab3a Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:15:24 +0100 Subject: [PATCH 10/13] Link the release note to the pull request Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index cbab1a059c7..09bb217df8a 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -9,6 +9,6 @@ * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) * Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) * Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) -* Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40)) +* Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40), [PR #20897](https://github.com/dotnet/fsharp/pull/20897)) ### Changed From 37f42caf6a1d3c3a9cb3b787c295cc636db35e46 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 22:18:00 +0100 Subject: [PATCH 11/13] Remove the duplicated release note line Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 09bb217df8a..558ac82f86a 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -8,7 +8,6 @@ * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) * Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522)) * Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40), [PR #20897](https://github.com/dotnet/fsharp/pull/20897)) ### Changed From 0b7c10ff940b65489e4bd8fc9d5485dbe63fbf3b Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 1 Nov 2026 23:11:38 +0100 Subject: [PATCH 12/13] Name only the members of FSharp.Core's own anonymous objects NewAnonymousObjectHelper is public, and a constructor of any type can be marked with it. The members are named Item1, Item2 and so on only for the AnonymousObject types of FSharp.Core; the constructor of any other type keeps the properties its type has, as it did before. Co-Authored-By: Claude Sonnet 5.5 --- src/FSharp.Core/Linq.fs | 16 ++++++++++++++- .../ExpressionNewMembersTests.fs | 20 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/FSharp.Core/Linq.fs b/src/FSharp.Core/Linq.fs index 2ee5b47bbff..750867b1995 100644 --- a/src/FSharp.Core/Linq.fs +++ b/src/FSharp.Core/Linq.fs @@ -85,6 +85,13 @@ module LeafExpressionConverter = [| for name in tupleMemberNames.[.. tupleTy.GetGenericArguments().Length - 1] -> if tupleTy.IsValueType then tupleTy.GetField name :> MemberInfo else tupleTy.GetProperty name :> MemberInfo |] + /// FSharp.Core's own AnonymousObject types, which the query translation builds, and which name their members + /// Item1, Item2 and so on + let isAnonymousObject (ty: Type) = + ty.IsGenericType + && obj.ReferenceEquals (ty.Assembly, typeof.Assembly) + && ty.Name.StartsWith ("AnonymousObject", StringComparison.Ordinal) + let hasPublicGetter (property: PropertyInfo) = match property.GetGetMethod() with | null -> false @@ -535,7 +542,14 @@ module LeafExpressionConverter = | NewAnonymousObjectQ(ctor, args) -> let argsR = ConvExprsToLinq env args let anonTy = ctor.DeclaringType - Expression.New(ctor, argsR, [| for i in 1 .. args.Length -> anonTy.GetProperty("Item" + string i) :> MemberInfo |]) |> asExpr + // The arguments become the items in the order of their numbers. The constructor of any other type, which + // NewAnonymousObjectHelper can mark as well, keeps the properties its type has. + let members = + if isAnonymousObject anonTy then + [| for i in 1 .. args.Length -> anonTy.GetProperty("Item" + string i) :> MemberInfo |] + else + [| for p in anonTy.GetProperties() -> p :> MemberInfo |] + Expression.New(ctor, argsR, members) |> asExpr // Do the same thing as C# compiler for string addition | PlusQ (_, GenericArgs [|ty1; ty2; ty3|], [x1; x2]) diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs index 4783ea20035..dba8babe29c 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/ExpressionNewMembersTests.fs @@ -9,6 +9,9 @@ open Microsoft.FSharp.Linq.RuntimeHelpers type PrivateRepresentation = private { Hidden: int } +type OnlyId(id: int) = + member _.Id = id + let private construction (lambda: Expression<'T>) = match lambda.Body with | :? NewExpression as construction -> construction @@ -63,3 +66,20 @@ let ``Anonymous record construction names the field each argument becomes`` () = let ``Record whose fields cannot be read publicly is built without members`` () = let record = construction (LeafExpressionConverter.QuotationToLambdaExpression <@ Func(fun x -> { Hidden = x }) @>) Assert.Null record.Members + +[] +let ``Constructor marked as an anonymous object keeps the properties of its type`` () = + let lambda = + LeafExpressionConverter.QuotationToLambdaExpression + <@ Func(fun x -> LeafExpressionConverter.NewAnonymousObjectHelper(OnlyId x)) @> + + Assert.Equal([ "Id" ], memberNames (construction lambda)) + Assert.Equal(3, lambda.Compile().Invoke(3).Id) + +[] +let ``Anonymous object of the query translation names its items`` () = + let lambda = + LeafExpressionConverter.QuotationToLambdaExpression + <@ Func(fun x -> LeafExpressionConverter.NewAnonymousObjectHelper(AnonymousObject(x, "a"))) @> + + Assert.Equal([ "Item1"; "Item2" ], memberNames (construction lambda)) From ab61bbb6bfde8a16f02e801f6108aa6c01a60b74 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Mon, 2 Nov 2026 14:46:52 +0100 Subject: [PATCH 13/13] Note what else the named members change, and cover distinct The note no longer credits #3782: the record with fields out of order is fixed by #20895. It names the distinct that returned a city once for each id, because the provider applied DISTINCT to both columns of the earlier projection. The note on breaking changes says that the SQL now names its columns, and that a groupBy on an item of a selected tuple needs #20899. A scenario takes one item of a tuple apart in the next for and applies distinct to it. Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.FSharp.Core/11.0.200.md | 4 ++-- docs/release-notes/PENDING_BREAKING_CHANGES.md | 6 ++++++ .../QueryMethodSyntaxScenarios.fs | 6 +++--- .../Microsoft.FSharp.Linq/QueryProjectionScenarios.fs | 11 +++++++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/11.0.200.md b/docs/release-notes/.FSharp.Core/11.0.200.md index 558ac82f86a..3da46561208 100644 --- a/docs/release-notes/.FSharp.Core/11.0.200.md +++ b/docs/release-notes/.FSharp.Core/11.0.200.md @@ -7,7 +7,7 @@ * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815)) * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814)) * Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) -* Fix struct tuples in query expressions and in lambdas converted to LINQ expression trees: selecting one no longer fails with "unreachable", and taking one apart no longer fails with a tuple index out of range. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894)) -* Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40), [PR #20897](https://github.com/dotnet/fsharp/pull/20897)) +* Fix LINQ providers failing on queries composed over a tuple or record projection (for example the tuples of a nested query taken apart by the next `for`, or a record read by a later `Where`): tuple, struct tuple, record and anonymous record constructions now name the member each argument becomes, as C# anonymous types do. ([Issue #15133](https://github.com/dotnet/fsharp/issues/15133), [Issue #40](https://github.com/dotnet/fsharp/issues/40), [PR #20897](https://github.com/dotnet/fsharp/pull/20897)) +* Fix `distinct` after a member of a tuple or record that an earlier operator selected: `for (city, _) in (select (b.Address.City, b.Id)) do select city; distinct` returned a city once for each id, because the provider applied `DISTINCT` to both columns of the earlier projection, and now returns each city once. ([PR #20897](https://github.com/dotnet/fsharp/pull/20897)) ### Changed diff --git a/docs/release-notes/PENDING_BREAKING_CHANGES.md b/docs/release-notes/PENDING_BREAKING_CHANGES.md index 5f4ac528707..d6ed9a40368 100644 --- a/docs/release-notes/PENDING_BREAKING_CHANGES.md +++ b/docs/release-notes/PENDING_BREAKING_CHANGES.md @@ -64,6 +64,12 @@ Constructing a tuple, struct tuple, record or anonymous record now gives a `NewE A record whose fields have no public getter is still built without `Members`. +The SQL that a provider writes names its columns after the members (`SELECT "b"."Id" AS "Item1", ...`), which shows in tests that compare SQL text. + +A `groupBy` on an item of a tuple that an earlier operator selected, `for (city, id) in (select (b.Address.City, b.Id)) do groupBy city into g; select (g.Key, g.Count())`, failed in EF Core with "could not be translated" and now fails inside EF Core with `KeyNotFoundException: The given key 'Item1' was not present in the dictionary`. It translates together with the change to groupings in #20899. + +Together with the change to F# operators in #20900, a `leftOuterJoin` that follows two `for`s and a `join`, and tests the joined row with `isNull`, fails in EF Core 10 for SQL Server with "Nullable object must have a value": `for c in db.Customers do for o in c.Orders do join d in db.Customers on (c.City = d.City); leftOuterJoin p in db.Products on (d.Id = p.Id) into products; for p in products do select (c.Name, o.Id, d.Name, (if isNull p then "" else p.Name))`. EF Core translates it with either change alone. + --- ## IQueryable Type Preservation 🟡 diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs index 8b12e720a26..497dbee8015 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryMethodSyntaxScenarios.fs @@ -81,7 +81,7 @@ let cases () = (fun db -> db.Customers.Select(fun c -> (c.Name, c.Age)).OrderBy(fun (_, age) -> age).Select(fun (name, _) -> name)) (customers >> Seq.sortBy _.Age >> Seq.map _.Name) |> Scenario.ordered - |> Scenario.violating block + |> Scenario.violating (blocks 4) Scenario.rows "OrderBy taking a struct tuple apart" @@ -89,13 +89,13 @@ let cases () = db.Customers.Select(fun c -> struct (c.Name, c.Age)).OrderBy(fun (struct (_, age)) -> age).Select(fun (struct (name, _)) -> name)) (customers >> Seq.sortBy _.Age >> Seq.map _.Name) |> Scenario.ordered - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "GroupBy taking a tuple apart" (fun db -> db.Customers.Select(fun c -> (c.City, c.Age)).GroupBy(fun (city, _) -> city).Select(fun g -> (g.Key, g.Count()))) (customers >> Seq.countBy _.City) - |> Scenario.violating block + |> Scenario.violating (blocks 2) Scenario.rows "GroupBy taking a struct tuple apart" diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs index c967f951119..aedb17acf40 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Linq/QueryProjectionScenarios.fs @@ -74,6 +74,17 @@ let cases () = }) (customers >> Seq.filter (fun c -> c.Id > 1) >> Seq.map _.Name) + // The provider has to know which column of the tuple is the city, or it applies DISTINCT to both + Scenario.rows + "distinct of an item of a tuple taken apart by the next for" + (fun db -> + query { + for (city, _) in (query { for c in db.Customers do select (c.City, c.Id) }) do + select city + distinct + }) + (customers >> Seq.map _.City >> Seq.distinct) + Scenario.rows "struct tuple taken apart by the next for" (fun db ->