Type checker: don't suppress errors while checking expressions (#18311)

Author: auduchinok

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

@@ -32,6 +32,7 @@
 * Add support for C# `Experimental` attribute. ([PR #18253](https://github.com/dotnet/fsharp/pull/18253))
 * Nullness warnings are issued for signature<>implementation conformance ([PR #18186](https://github.com/dotnet/fsharp/pull/18186))
 * Symbols: Add FSharpAssembly.IsFSharp ([PR #18290](https://github.com/dotnet/fsharp/pull/18290))
+* Type checker: don't suppress errors while checking expressions ([PR #18311](https://github.com/dotnet/fsharp/pull/18311))
 * Type parameter constraint `null` in generic code will now automatically imply `not struct` ([Issue #18320](https://github.com/dotnet/fsharp/issues/18320), [PR #18323](https://github.com/dotnet/fsharp/pull/18323))
 * Add a switch to determine whether to generate a default implementation body for overridden method when completing. [PR #18341](https://github.com/dotnet/fsharp/pull/18341)
 * Use a more accurate range for CE Combine methods. [PR #18394](https://github.com/dotnet/fsharp/pull/18394)

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -5960,7 +5960,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
 
     | SynExpr.FromParseError (expr1, m) ->
         //SolveTypeAsError cenv env.DisplayEnv m overallTy
-        let _, tpenv = suppressErrorReporting (fun () -> TcExpr cenv overallTy env tpenv expr1)
+        let _, tpenv = TcExpr cenv overallTy env tpenv expr1
         mkDefault(m, overallTy.Commit), tpenv
 
     | SynExpr.Sequential (sp, dir, synExpr1, synExpr2, m, _) ->
@@ -6489,9 +6489,7 @@ and TcIteratedLambdas (cenv: cenv) isFirst (env: TcEnv) overallTy takenNames tpe
 
     | e ->
         let env = { env with eIsControlFlow = true }
-        // Dive into the expression to check for syntax errors and suppress them if they show.
-        conditionallySuppressErrorReporting (not isFirst && synExprContainsError e) (fun () ->
-            TcExpr cenv overallTy env tpenv e)
+        TcExpr cenv overallTy env tpenv e
 
 and TcTyparExprThen (cenv: cenv) overallTy env tpenv synTypar m delayed =
     match delayed with
@@ -11103,39 +11101,33 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt
         // At each module binding, dive into the expression to check for syntax errors and suppress them if they show.
         // Don't do this for lambdas, because we always check for suppression for all lambda bodies in TcIteratedLambdas
         let rhsExprChecked, tpenv =
-            let atTopNonLambdaDefn =
-                declKind.IsModuleOrMemberOrExtensionBinding &&
-                (match rhsExpr with SynExpr.Lambda _ -> false | _ -> true) &&
-                synExprContainsError rhsExpr
-
-            conditionallySuppressErrorReporting atTopNonLambdaDefn (fun () ->
-
-                // Save the arginfos away to match them up in the lambda
-                let (PrelimValReprInfo(argInfos, _)) = prelimValReprInfo
-
-                // The right-hand-side is control flow (has an implicit debug point) in any situation where we
-                // haven't extended the debug point to include the 'let', that is, there is a debug point noted
-                // at the binding.
-                //
-                // This includes
-                //    let _ = expr
-                //    let () = expr
-                // which are transformed to sequential expressions in TcLetBinding
-                //
-                let rhsIsControlFlow =
-                    match pat with
-                    | SynPat.Wild _
-                    | SynPat.Const (SynConst.Unit, _)
-                    | SynPat.Paren (SynPat.Const (SynConst.Unit, _), _) -> true
-                    | _ ->
-                    match debugPoint with
-                    | DebugPointAtBinding.Yes _ -> false
-                    | _ -> true
+            // Save the arginfos away to match them up in the lambda
+            let (PrelimValReprInfo(argInfos, _)) = prelimValReprInfo
+
+            // The right-hand-side is control flow (has an implicit debug point) in any situation where we
+            // haven't extended the debug point to include the 'let', that is, there is a debug point noted
+            // at the binding.
+            //
+            // This includes
+            //    let _ = expr
+            //    let () = expr
+            // which are transformed to sequential expressions in TcLetBinding
+            //
+            let rhsIsControlFlow =
+                match pat with
+                | SynPat.Wild _
+                | SynPat.Const (SynConst.Unit, _)
+                | SynPat.Paren (SynPat.Const (SynConst.Unit, _), _) -> true
+                | _ ->
+
+                match debugPoint with
+                | DebugPointAtBinding.Yes _ -> false
+                | _ -> true
 
-                let envinner = { envinner with eLambdaArgInfos = argInfos; eIsControlFlow = rhsIsControlFlow }
+            let envinner = { envinner with eLambdaArgInfos = argInfos; eIsControlFlow = rhsIsControlFlow }
 
-                if isCtor then TcExprThatIsCtorBody (safeThisValOpt, safeInitInfo) cenv (MustEqual overallExprTy) envinner tpenv rhsExpr
-                else TcExprThatCantBeCtorBody cenv (MustConvertTo (false, overallExprTy)) envinner tpenv rhsExpr)
+            if isCtor then TcExprThatIsCtorBody (safeThisValOpt, safeInitInfo) cenv (MustEqual overallExprTy) envinner tpenv rhsExpr
+            else TcExprThatCantBeCtorBody cenv (MustConvertTo (false, overallExprTy)) envinner tpenv rhsExpr
 
         if kind = SynBindingKind.StandaloneExpression && not cenv.isScript then
             UnifyUnitType cenv env mBinding overallPatTy rhsExprChecked |> ignore<bool>

tests/FSharp.Compiler.Service.Tests/Common.fs

@@ -119,7 +119,6 @@ let mkProjectCommandLineArgsForScript (dllName, fileNames) =
         yield "--doc:test.xml"
         yield "--warn:3"
         yield "--fullpaths"
-        yield "--flaterrors"
         yield "--target:library"
         for x in fileNames do
             yield x
@@ -370,11 +369,17 @@ let inline dumpDiagnostics (results: FSharpCheckFileResults) =
     |> Array.map (fun e ->
         let message =
             e.Message.Split('\n')
-            |> Array.map (fun s -> s.Trim())
+            |> Array.map _.Trim()
+            |> Array.filter (fun s -> s.Length > 0)
             |> String.concat " "
         sprintf "%s: %s" (e.Range.ToString()) message)
     |> List.ofArray
 
+let inline dumpDiagnosticNumbers (results: FSharpCheckFileResults) =
+    results.Diagnostics
+    |> Array.map (fun e -> e.Range.ToString(), e.ErrorNumber)
+    |> List.ofArray
+
 let getSymbolUses (results: FSharpCheckFileResults) =
     results.GetAllUsesOfAllSymbolsInFile()
 

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj

@@ -26,6 +26,7 @@
       <Link>XunitSetup.fs</Link>
     </Compile>
     <Compile Include="Common.fs" />
+    <Compile Include="TypeChecker\TypeCheckerRecoveryTests.fs" />
     <Compile Include="GeneratedCodeSymbolsTests.fs" />
     <Compile Include="AssemblyReaderShim.fs" />
     <Compile Include="ModuleReaderCancellationTests.fs" />