WIP : FSharp.Core 'query' related fixes

BREAKING_CHANGES.md

@@ -0,0 +1,67 @@
+# Breaking Changes in Query Expression Fixes
+
+## AnonymousObject Structural Equality 🔴
+
+`AnonymousObject<T>` types now implement `Equals`/`GetHashCode` with structural equality.
+
+**Impact:** Code using these as Dictionary keys with identity semantics will see different behavior.
+
+```fsharp
+let a = AnonymousObject(1, 2)
+let b = AnonymousObject(1, 2)
+a.Equals(b)  // Was: false, Now: true
+```
+
+**Why:** Fixes #7885, #47 - join/groupBy with tuple keys now work correctly.
+
+---
+
+## Expression Tree Structure Changes 🟡
+
+### Let-bindings use Block instead of Lambda.Invoke
+
+```fsharp
+<@ let x = 1 in x + 1 @>
+// Was: Lambda(x => x+1).Invoke(1)
+// Now: Block({ x = 1; x + 1 })
+```
+
+**Migration:** If you detected let-bindings via `Lambda.Invoke`:
+```csharp
+// Old: if (node.Method.Name == "Invoke" && node.Object is LambdaExpression)
+// New: if (node.NodeType == ExpressionType.Block)
+```
+
+### Array access uses ArrayIndex
+
+```fsharp
+<@ arr.[0] @>
+// Was: Call(GetArray, arr, 0)
+// Now: ArrayIndex(arr, 0)
+```
+
+**Migration:**
+```csharp
+// Old: if (node.Method.Name == "GetArray")
+// New: if (node.NodeType == ExpressionType.ArrayIndex)
+```
+
+---
+
+## IQueryable Type Preservation 🟡
+
+Tuple projections now preserve the provider's `IQueryable` type.
+
+```fsharp
+query { for p in db.Products do select (p.Id, p.Name) }
+// Was: EnumerableQuery<_>
+// Now: Provider's IQueryable<_>
+```
+
+**Impact:** Code checking `is EnumerableQuery<_>` may need adjustment.
+
+---
+
+## FCS API: Query Variables 🟢
+
+Query variables now report `IsCompilerGenerated = true`. Fixes #422 (FS1182 false positives).

docs/release-notes/.FSharp.Compiler.Service/10.0.300.md

@@ -1,5 +1,7 @@
 ### Fixed
 
+* Fix false FS1182 (unused variable) warning for query expression variables used in where, let, join, and select clauses. ([Issue #422](https://github.com/dotnet/fsharp/issues/422))
+
 ### Added
 
 ### Changed

docs/release-notes/.FSharp.Core/10.0.300.md

@@ -1,5 +1,12 @@
 ### Fixed
 
+* Fix anonymous record field ordering in LINQ expression conversion to produce consistent expression trees regardless of field declaration order. ([Issue #11131](https://github.com/dotnet/fsharp/issues/11131), [Issue #15648](https://github.com/dotnet/fsharp/issues/15648))
+* Fix array indexing in LINQ expressions to generate proper array index expressions instead of GetArray method calls, enabling LINQ providers like Azure Cosmos DB to translate array access. ([Issue #16918](https://github.com/dotnet/fsharp/issues/16918))
+* Fix tuple join conditions and groupBy operations to properly compare tuple keys using structural equality. AnonymousObject types now implement Equals and GetHashCode, enabling inline tuple joins like `join b on ((a.Id1, a.Id2) = (b.Id1, b.Id2))` to work correctly. ([Issue #7885](https://github.com/dotnet/fsharp/issues/7885), [Issue #47](https://github.com/dotnet/fsharp/issues/47))
+* Fix tuple/multi-value projections in queries to use Queryable.Select instead of Enumerable.Select when the source is IQueryable, preserving query composition and enabling async operations like ToListAsync() in Entity Framework Core. ([Issue #3782](https://github.com/dotnet/fsharp/issues/3782), [Issue #15133](https://github.com/dotnet/fsharp/issues/15133))
+* Fix EvaluateQuotation to handle Sequential expressions, void method calls (unit return), and other patterns that were previously throwing NotSupportedException. Also properly handles unit-returning expressions by using Action delegates instead of Func delegates. ([Issue #19099](https://github.com/dotnet/fsharp/issues/19099))
+* Fix query conditionals without else branch (if-then only) that were causing type mismatch errors. Now properly extracts element type from IQueryable for creating empty sequences. ([Issue #3445](https://github.com/dotnet/fsharp/issues/3445))
+
 ### Added
 
 ### Changed

src/Compiler/Checking/Expressions/CheckComputationExpressions.fs

@@ -123,11 +123,29 @@ let mkSimplePatForVarSpace m (patvs: Val list) =
     let spats =
         match patvs with
         | [] -> []
-        | [ v ] -> [ mkSynSimplePatVar false v.Id ]
-        | vs -> vs |> List.map (fun v -> mkSynSimplePatVar false v.Id)
+        // Use mkSynCompGenSimplePatVar to mark these synthetic lambda parameters as compiler-generated.
+        // This prevents false FS1182 warnings for query variables that are logically used in query clauses.
+        // See Issue #422.
+        | [ v ] -> [ mkSynCompGenSimplePatVar v.Id ]
+        | vs -> vs |> List.map (fun v -> mkSynCompGenSimplePatVar v.Id)
 
     SynSimplePats.SimplePats(spats, [], m)
 
+/// Mark all SynSimplePat.Id nodes in a SynSimplePats as compiler-generated.
+/// This is used to prevent false FS1182 warnings for join/groupJoin/zip patterns
+/// that are used in synthetic lambdas for key selectors.
+let rec markSimplePatAsCompilerGenerated (pat: SynSimplePat) =
+    match pat with
+    | SynSimplePat.Id(ident, altNameRefCell, _isCompilerGenerated, isThisVal, isOptional, range) ->
+        SynSimplePat.Id(ident, altNameRefCell, true, isThisVal, isOptional, range)
+    | SynSimplePat.Typed(p, ty, range) -> SynSimplePat.Typed(markSimplePatAsCompilerGenerated p, ty, range)
+    | SynSimplePat.Attrib(p, attribs, range) -> SynSimplePat.Attrib(markSimplePatAsCompilerGenerated p, attribs, range)
+
+let markSimplePatsAsCompilerGenerated (pats: SynSimplePats) =
+    match pats with
+    | SynSimplePats.SimplePats(patList, commaRanges, range) ->
+        SynSimplePats.SimplePats(patList |> List.map markSimplePatAsCompilerGenerated, commaRanges, range)
+
 let mkPatForVarSpace m (patvs: Val list) =
     match patvs with
     | [] -> SynPat.Const(SynConst.Unit, m)
@@ -1063,10 +1081,12 @@ let rec TryTranslateComputationExpression
                     | None -> varSpace
 
                 let firstSourceSimplePats, later1 =
-                    SimplePatsOfPat cenv.synArgNameGenerator firstSourcePat
+                    let pats, later = SimplePatsOfPat cenv.synArgNameGenerator firstSourcePat
+                    markSimplePatsAsCompilerGenerated pats, later
 
                 let secondSourceSimplePats, later2 =
-                    SimplePatsOfPat cenv.synArgNameGenerator secondSourcePat
+                    let pats, later = SimplePatsOfPat cenv.synArgNameGenerator secondSourcePat
+                    markSimplePatsAsCompilerGenerated pats, later
 
                 if Option.isSome later1 then
                     errorR (Error(FSComp.SR.tcJoinMustUseSimplePattern nm.idText, firstSourcePat.Range))
@@ -1152,7 +1172,8 @@ let rec TryTranslateComputationExpression
                         // groupJoin
                         | Some secondResultPat, Some relExpr when customOperationIsLikeGroupJoin ceenv nm ->
                             let secondResultSimplePats, later3 =
-                                SimplePatsOfPat cenv.synArgNameGenerator secondResultPat
+                                let pats, later = SimplePatsOfPat cenv.synArgNameGenerator secondResultPat
+                                markSimplePatsAsCompilerGenerated pats, later
 
                             if Option.isSome later3 then
                                 errorR (Error(FSComp.SR.tcJoinMustUseSimplePattern nm.idText, secondResultPat.Range))

src/FSharp.Core/Linq.fs

@@ -34,10 +34,21 @@ module LeafExpressionConverter =
         {   varEnv : Map<Var, Expression> }
     let asExpr x = (x :> Expression)
 
-    let instanceBindingFlags = BindingFlags.Instance ||| BindingFlags.Public ||| BindingFlags.NonPublic ||| BindingFlags.DeclaredOnly
+    let _instanceBindingFlags = BindingFlags.Instance ||| BindingFlags.Public ||| BindingFlags.NonPublic ||| BindingFlags.DeclaredOnly
 
     let isNamedType(typ:Type) = not (typ.IsArray || typ.IsByRef || typ.IsPointer)
 
+    /// Determines if a property access on a derived union case type needs to be coerced to the declaring type.
+    /// This handles the case where a property is defined on a specific union case type that inherits from the union type.
+    let getUnionCaseCoercionType (objOpt: Expr option) (declaringType: Type) =
+        if objOpt.IsSome && 
+           FSharpType.IsUnion declaringType && 
+           not (isNull declaringType.BaseType) &&
+           FSharpType.IsUnion declaringType.BaseType then
+            Some declaringType
+        else
+            None
+
     let equivHeadTypes (ty1:Type) (ty2:Type) =
         isNamedType(ty1) &&
         if ty1.IsGenericType then
@@ -475,11 +486,7 @@ module LeafExpressionConverter =
              build arg.Type argP n
 
         | PropertyGet(objOpt, propInfo, args) ->
-            let coerceTo =
-                if objOpt.IsSome && FSharpType.IsUnion propInfo.DeclaringType && FSharpType.IsUnion propInfo.DeclaringType.BaseType then
-                    Some propInfo.DeclaringType
-                else
-                    None
+            let coerceTo = getUnionCaseCoercionType objOpt propInfo.DeclaringType
             match args with
             | [] ->
                 Expression.Property(ConvObjArg env objOpt coerceTo, propInfo) |> asExpr
@@ -623,7 +630,10 @@ module LeafExpressionConverter =
             | CheckedConvUInt16Q (_, _, [x]) | CheckedConvUInt32Q (_, _, [x]) | CheckedConvUInt64Q (_, _, [x]) | CheckedConvIntPtrQ (_, _, [x]) -> transConv inp env true x
             | CheckedConvUIntPtrQ (_, _, [x]) -> transConv inp env true x
 
-            | ArrayLookupQ (_, GenericArgs [|_; _; _|], [x1; x2]) ->
+            // Issue #16918: ArrayLookupQ pattern expects GenericArgs [|_|] (1 type param) not [|_; _; _|] (3 params)
+            // because GetArray only has 1 type parameter. This enables proper Expression.ArrayIndex generation
+            // that LINQ providers can translate, instead of falling through to method call handling.
+            | ArrayLookupQ (_, GenericArgs [|_|], [x1; x2]) ->
                 Expression.ArrayIndex(ConvExprToLinqInContext env x1, ConvExprToLinqInContext env x2) |> asExpr
 
             // Throw away markers inserted to satisfy C#'s design where they pass an argument
@@ -770,13 +780,16 @@ module LeafExpressionConverter =
                     |> asExpr
 
         | Let (v, e, b) ->
+            // Use Expression.Block to properly scope the variable. This:
+            // 1. Evaluates the expression exactly once (preserving reference semantics)
+            // 2. Avoids Lambda.Invoke which EF Core cannot translate
+            // 3. Works for both mutable and immutable variables
             let vP = ConvVarToLinq v
+            let eP = ConvExprToLinqInContext env e
             let envinner = { varEnv = Map.add v (vP |> asExpr) env.varEnv }
             let bodyP = ConvExprToLinqInContext envinner b
-            let eP = ConvExprToLinqInContext env e
-            let ty = Expression.GetFuncType [| v.Type; b.Type |]
-            let lam = Expression.Lambda(ty, bodyP, [| vP |]) |> asExpr
-            Expression.Call(lam, ty.GetMethod("Invoke", instanceBindingFlags), [| eP |]) |> asExpr
+            // Create a block with the variable declaration, initial assignment, and body
+            Expression.Block([| vP |], Expression.Assign(vP, eP), bodyP) |> asExpr
 
         | Lambda(v, body) ->
             let vP = ConvVarToLinq v
@@ -792,6 +805,41 @@ module LeafExpressionConverter =
             let convType = lambdaTy.MakeGenericType tyargs
             let convDelegate = Expression.Lambda(convType, bodyP, [| vP |]) |> asExpr
             Expression.Call(typeof<FuncConvert>, "ToFSharpFunc", tyargs, [| convDelegate |]) |> asExpr
+
+        // Issue #19099: Handle Sequential (e1; e2) expressions
+        | Sequential(e1, e2) ->
+            let e1P = ConvExprToLinqInContext env e1
+            let e2P = ConvExprToLinqInContext env e2
+            Expression.Block(e1P, e2P) |> asExpr
+
+        // Issue #19099: Handle VarSet (v <- value) expressions  
+        | VarSet(v, value) ->
+            let vP = 
+                try Map.find v env.varEnv
+                with :? KeyNotFoundException -> invalidOp ("The variable '"+ v.Name + "' was not found in the translation context'")
+            let valueP = ConvExprToLinqInContext env value
+            Expression.Assign(vP, valueP) |> asExpr
+
+        // Issue #19099: Handle FieldSet (obj.field <- value) expressions
+        | FieldSet(objOpt, fieldInfo, value) ->
+            let objP = ConvObjArg env objOpt None
+            let valueP = ConvExprToLinqInContext env value
+            Expression.Assign(Expression.Field(objP, fieldInfo), valueP) |> asExpr
+
+        // Issue #19099: Handle PropertySet (obj.prop <- value) expressions
+        | PropertySet(objOpt, propInfo, args, value) ->
+            let coerceTo = getUnionCaseCoercionType objOpt propInfo.DeclaringType
+            let valueP = ConvExprToLinqInContext env value
+            match args with
+            | [] ->
+                let propExpr = Expression.Property(ConvObjArg env objOpt coerceTo, propInfo)
+                Expression.Assign(propExpr, valueP) |> asExpr
+            | _ ->
+                // For indexed property sets, use the setter method directly
+                let argsP = ConvExprsToLinq env args
+                let allArgsP = Array.append argsP [| valueP |]
+                Expression.Call(ConvObjArg env objOpt coerceTo, propInfo.GetSetMethod(true), allArgsP) |> asExpr
+
         | _ ->
             failConvert inp
 
@@ -910,11 +958,23 @@ module LeafExpressionConverter =
        | Value (obj, _) -> obj
        | _ ->
        let ty = e.Type
-       let e = Expr.NewDelegate (Expression.GetFuncType([|typeof<unit>; ty |]), [new Var("unit", typeof<unit>)], e)
-       let linqExpr = (ConvExprToLinq e:?> LambdaExpression)
-       let d = linqExpr.Compile ()
-       try
-           d.DynamicInvoke [| box () |]
-       with :? TargetInvocationException as exn ->
-           raise exn.InnerException
+       // Helper to compile and invoke a delegate, re-raising inner exceptions
+       let compileAndInvoke (delegateExpr: Expr) =
+           let linqExpr = ConvExprToLinq delegateExpr :?> LambdaExpression
+           let d = linqExpr.Compile()
+           try
+               d.DynamicInvoke [| box () |]
+           with :? TargetInvocationException as exn ->
+               raise exn.InnerException
+
+       // Issue #19099: Handle unit/void return types by wrapping in an Action instead of Func
+       // When the expression returns unit, the LINQ expression will have type System.Void which cannot
+       // be a return type of Func<unit, unit>. We use Action<unit> instead and return box().
+       if ty = typeof<unit> then
+           let unitVar = new Var("unit", typeof<unit>)
+           compileAndInvoke (Expr.NewDelegate(typeof<Action<unit>>, [unitVar], e)) |> ignore
+           box ()
+       else
+           let unitVar = new Var("unit", typeof<unit>)
+           compileAndInvoke (Expr.NewDelegate(Expression.GetFuncType([|typeof<unit>; ty|]), [unitVar], e))
 #endif