From 77bcbe73861a9e5d3039e6805e0ce144028eae1f Mon Sep 17 00:00:00 2001
From: Ryan Coy <ryan@efbnet.com>
Date: Tue, 1 Dec 2020 21:19:29 +0100
Subject: [PATCH] Resolve issue with implicit yields requiring Zero (#10556)

---
 src/fsharp/CheckComputationExpressions.fs     |  8 +-
 .../FSharp.Compiler.ComponentTests.fsproj     |  5 +-
 .../Language/ComputationExpressionTests.fs    | 95 +++++++++++++++++++
 3 files changed, 103 insertions(+), 5 deletions(-)
 create mode 100644 tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs

diff --git a/src/fsharp/CheckComputationExpressions.fs b/src/fsharp/CheckComputationExpressions.fs
index c8ac4cd1f9a..fa413a52a02 100644
--- a/src/fsharp/CheckComputationExpressions.fs
+++ b/src/fsharp/CheckComputationExpressions.fs
@@ -949,8 +949,14 @@ let TcComputationExpression cenv env overallTy tpenv (mWhole, interpExpr: Expr,
         | SynExpr.Paren (_, _, _, m) -> 
             error(Error(FSComp.SR.tcConstructIsAmbiguousInComputationExpression(), m))
 
+        // In some cases the node produced by `mkSynCall "Zero" m []` may be discarded in the case
+        // of implicit yields - for example "list { 1; 2 }" when each expression checks as an implicit yield.
+        // If it is not discarded, the syntax node will later be checked and the existence/non-existence of the Zero method
+        // will be checked/reported appropriately (though the error message won't mention computation expressions
+        // like our other error messages for missing methods).
         | SynExpr.ImplicitZero m -> 
-            if isNil (TryFindIntrinsicOrExtensionMethInfo ResultCollectionSettings.AtMostOneResult cenv env m ad "Zero" builderTy) then error(Error(FSComp.SR.tcRequireBuilderMethod("Zero"), m))
+            if (not enableImplicitYield) && 
+               isNil (TryFindIntrinsicOrExtensionMethInfo ResultCollectionSettings.AtMostOneResult cenv env m ad "Zero" builderTy) then error(Error(FSComp.SR.tcRequireBuilderMethod("Zero"), m))
             Some (translatedCtxt (mkSynCall "Zero" m []))
             
         | OptionalSequential (JoinOrGroupJoinOrZipClause (_, _, _, _, _, mClause), _) 
diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj
index 440a4e8cf94..3f8fc44becd 100644
--- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj
+++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj
@@ -45,6 +45,7 @@
     <Compile Include="Language\XmlComments.fs" />
     <Compile Include="Language\CompilerDirectiveTests.fs" />
     <Compile Include="Language\CodeQuotationTests.fs" />
+    <Compile Include="Language\ComputationExpressionTests.fs" />
     <Compile Include="ConstraintSolver\PrimitiveConstraints.fs" />
     <Compile Include="ConstraintSolver\MemberConstraints.fs" />
     <Compile Include="Interop\SimpleInteropTests.fs" />
@@ -57,8 +58,4 @@
     <ProjectReference Include="$(FSharpSourcesRoot)\fsharp\FSharp.Core\FSharp.Core.fsproj" />
     <ProjectReference Include="$(FSharpTestsRoot)\FSharp.Test.Utilities\FSharp.Test.Utilities.fsproj" />
   </ItemGroup>
-
-  <ItemGroup>
-    <Folder Include="NewFolder\" />
-  </ItemGroup>
 </Project>
diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs
new file mode 100644
index 00000000000..0b567ee9a9c
--- /dev/null
+++ b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs
@@ -0,0 +1,95 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace FSharp.Compiler.ComponentTests.Language
+
+open Xunit
+open FSharp.Test.Utilities.Compiler
+
+module ComputationExpressionTests =
+    [<Fact>]
+    let ``A CE not using Zero does not require Zero``() =
+        FSharp """
+module ComputationExpressionTests
+type ListBuilder () =
+    member __.Combine (a: List<'T>, b) = a @ b
+    member __.Yield  x = List.singleton x
+    member __.Delay expr = expr () :  List<'T>
+
+let lb = ListBuilder ()
+
+let x = lb {1; 2;}
+        """
+        |> compile
+        |> shouldSucceed
+        |> ignore
+
+    [<Fact>]
+    let ``A CE explicitly using Zero fails without a defined Zero``() =
+        FSharp """
+module ComputationExpressionTests
+type ListBuilder () =
+    member __.Combine (a: List<'T>, b) = a @ b
+    member __.Yield  x = List.singleton x
+    member __.Delay expr = expr () :  List<'T>
+
+let lb = ListBuilder ()
+
+let x = lb {1; 2;()}
+        """
+        |> compile
+        |> shouldFail
+        |> withSingleDiagnostic (Error 39, Line 10, Col 18, Line 10, Col 20, "The type 'ListBuilder' does not define the field, constructor or member 'Zero'.")
+        |> ignore
+
+    [<Fact>]
+    let ``A CE explicitly using Zero succeeds with a defined Zero``() =
+        FSharp """
+module ComputationExpressionTests
+type ListBuilder () =
+    member __.Zero () = []
+    member __.Combine (a: List<'T>, b) = a @ b
+    member __.Yield  x = List.singleton x
+    member __.Delay expr = expr () :  List<'T>
+
+let lb = ListBuilder ()
+
+let x = lb {1; 2;()}
+        """
+        |> compile
+        |> shouldSucceed
+        |> ignore
+
+    [<Fact>]
+    let ``A CE with a complete if-then expression does not require Zero``() =
+        FSharp """
+module ComputationExpressionTests
+type ListBuilder () =
+    member __.Combine (a: List<'T>, b) = a @ b
+    member __.Yield  x = List.singleton x
+    member __.Delay expr = expr () :  List<'T>
+
+let lb = ListBuilder ()
+
+let x = lb {1; 2; if true then 3 else 4;}
+        """
+        |> compile
+        |> shouldSucceed
+        |> ignore
+
+    [<Fact>]
+    let ``A CE with a missing/empty else branch implicitly requires Zero``() =
+        FSharp """
+module ComputationExpressionTests
+type ListBuilder () =
+    member __.Combine (a: List<'T>, b) = a @ b
+    member __.Yield  x = List.singleton x
+    member __.Delay expr = expr () :  List<'T>
+
+let lb = ListBuilder ()
+
+let x = lb {1; 2; if true then 3;}
+        """
+        |> compile
+        |> shouldFail
+        |> withSingleDiagnostic (Error 708, Line 10, Col 19, Line 10, Col 31, "This control construct may only be used if the computation expression builder defines a 'Zero' method")
+        |> ignore
\ No newline at end of file