Better error reporting for CE do! (dotnet#17779)

* Better error reporting for `CE` do bang * Use SynExpr.DoBang * update surface area * format code * release notes * update tests * Remove no intended case
vzarytovskii · Sep 25, 2024 · 6cdcc3c · 6cdcc3c
1 parent cd736c4
commit 6cdcc3c
Show file tree

Hide file tree

Showing 19 changed files with 53 additions and 24 deletions.
diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
@@ -8,5 +8,6 @@
 
 * Make ILTypeDef interface impls calculation lazy. ([PR #17392](https://github.com/dotnet/fsharp/pull/17392))
 * Better ranges for CE `let!` and `use!` error reporting. ([PR #17712](https://github.com/dotnet/fsharp/pull/17712))
+* Better ranges for CE `do!` error reporting. ([PR #17779](https://github.com/dotnet/fsharp/pull/17779))
 
 ### Breaking Changes
diff --git a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs
@@ -1592,7 +1592,7 @@ let rec TryTranslateComputationExpression
                 if ceenv.isQuery && not (innerComp1.IsArbExprAndThusAlreadyReportedError) then
                     match innerComp1 with
                     | SynExpr.JoinIn _ -> ()
-                    | SynExpr.DoBang(range = m) -> errorR (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), m))
+                    | SynExpr.DoBang(trivia = { DoBangKeyword = m }) -> errorR (Error(FSComp.SR.tcBindMayNotBeUsedInQueries (), m))
                     | _ -> errorR (Error(FSComp.SR.tcUnrecognizedQueryOperator (), innerComp1.RangeOfFirstPortion))
 
                 match
@@ -1657,7 +1657,7 @@ let rec TryTranslateComputationExpression
                 | None ->
                     // "do! expr; cexpr" is treated as { let! () = expr in cexpr }
                     match innerComp1 with
-                    | SynExpr.DoBang(rhsExpr, m) ->
+                    | SynExpr.DoBang(expr = rhsExpr; range = m) ->
                         let sp =
                             match sp with
                             | DebugPointAtSequential.SuppressExpr -> DebugPointAtBinding.NoneAtDo
@@ -2867,7 +2867,7 @@ and TranslateComputationExpression (ceenv: ComputationExpressionContext<'a>) fir
             // This only occurs in final position in a sequence
             match comp with
             // "do! expr;" in final position is treated as { let! () = expr in return () } when Return is provided (and no Zero with Default attribute is available) or as { let! () = expr in zero } otherwise
-            | SynExpr.DoBang(rhsExpr, m) ->
+            | SynExpr.DoBang(expr = rhsExpr; trivia = { DoBangKeyword = m }) ->
                 let mUnit = rhsExpr.Range
                 let rhsExpr = mkSourceExpr rhsExpr ceenv.sourceMethInfo ceenv.builderValName
 

diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs
@@ -6003,10 +6003,10 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
     | SynExpr.ImplicitZero m ->
         error(Error(FSComp.SR.tcConstructRequiresSequenceOrComputations(), m))
 
-    | SynExpr.DoBang (_, m)
+    | SynExpr.DoBang (trivia = { DoBangKeyword = m })
     | SynExpr.MatchBang (range = m)
     | SynExpr.WhileBang (range = m)
-    | SynExpr.LetOrUseBang (range = m) ->
+    | SynExpr.LetOrUseBang (trivia = { LetOrUseBangKeyword = m }) ->
         error(Error(FSComp.SR.tcConstructRequiresComputationExpression(), m))
 
     | SynExpr.IndexFromEnd (rightExpr, m) ->

diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs
@@ -168,7 +168,7 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
 
         | SynExpr.ImplicitZero m -> Some(mkSeqEmpty cenv env m genOuterTy, tpenv)
 
-        | SynExpr.DoBang(_rhsExpr, m) -> error (Error(FSComp.SR.tcDoBangIllegalInSequenceExpression (), m))
+        | SynExpr.DoBang(trivia = { DoBangKeyword = m }) -> error (Error(FSComp.SR.tcDoBangIllegalInSequenceExpression (), m))
 
         | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m, _) ->
             let env1 =

diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs
@@ -523,7 +523,7 @@ let visitSynExpr (e: SynExpr) : FileContentEntry list =
             visit expr (fun exprNodes ->
                 [ yield! exprNodes; yield! List.collect visitSynMatchClause clauses ]
                 |> continuation)
-        | SynExpr.DoBang(expr, _) -> visit expr continuation
+        | SynExpr.DoBang(expr = expr) -> visit expr continuation
         | SynExpr.WhileBang(whileExpr = whileExpr; doExpr = doExpr) ->
             visit whileExpr (fun whileNodes -> visit doExpr (fun doNodes -> whileNodes @ doNodes |> continuation))
         | SynExpr.LibraryOnlyILAssembly(typeArgs = typeArgs; args = args; retTy = retTy) ->

diff --git a/src/Compiler/Service/FSharpParseFileResults.fs b/src/Compiler/Service/FSharpParseFileResults.fs
@@ -578,7 +578,7 @@ type FSharpParseFileResults(diagnostics: FSharpDiagnostic[], input: ParsedInput,
                             yield! walkExpr false e
 
                         | SynExpr.YieldOrReturnFrom(_, e, _)
-                        | SynExpr.DoBang(e, _) ->
+                        | SynExpr.DoBang(expr = e) ->
                             yield! checkRange e.Range
                             yield! walkExpr false e
 

diff --git a/src/Compiler/Service/ServiceInterfaceStubGenerator.fs b/src/Compiler/Service/ServiceInterfaceStubGenerator.fs
@@ -952,9 +952,9 @@ module InterfaceStubGenerator =
                 | SynExpr.Null _range
                 | SynExpr.ImplicitZero _range -> None
 
-                | SynExpr.YieldOrReturn(_, synExpr, _range)
-                | SynExpr.YieldOrReturnFrom(_, synExpr, _range)
-                | SynExpr.DoBang(synExpr, _range) -> walkExpr synExpr
+                | SynExpr.YieldOrReturn(expr = synExpr)
+                | SynExpr.YieldOrReturnFrom(expr = synExpr)
+                | SynExpr.DoBang(expr = synExpr) -> walkExpr synExpr
 
                 | SynExpr.LetOrUseBang(rhs = synExpr1; andBangs = synExprAndBangs; body = synExpr2) ->
                     [

diff --git a/src/Compiler/Service/ServiceStructure.fs b/src/Compiler/Service/ServiceStructure.fs
@@ -254,7 +254,7 @@ module Structure =
                 rcheck Scope.YieldOrReturnBang Collapse.Below r r
                 parseExpr e
 
-            | SynExpr.DoBang(e, r) ->
+            | SynExpr.DoBang(expr = e; range = r) ->
                 rcheck Scope.Do Collapse.Below r <| Range.modStart 3 r
                 parseExpr e
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fs b/src/Compiler/SyntaxTree/SyntaxTree.fs
@@ -726,7 +726,7 @@ type SynExpr =
         range: range *
         trivia: SynExprMatchBangTrivia
 
-    | DoBang of expr: SynExpr * range: range
+    | DoBang of expr: SynExpr * range: range * trivia: SynExprDoBangTrivia
 
     | WhileBang of whileDebugPoint: DebugPointAtWhile * whileExpr: SynExpr * doExpr: SynExpr * range: range
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fsi b/src/Compiler/SyntaxTree/SyntaxTree.fsi
@@ -909,7 +909,7 @@ type SynExpr =
 
     /// F# syntax: do! expr
     /// Computation expressions only
-    | DoBang of expr: SynExpr * range: range
+    | DoBang of expr: SynExpr * range: range * trivia: SynExprDoBangTrivia
 
     /// F# syntax: 'while! ... do ...'
     | WhileBang of whileDebugPoint: DebugPointAtWhile * whileExpr: SynExpr * doExpr: SynExpr * range: range

diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs
@@ -885,7 +885,7 @@ let rec synExprContainsError inpExpr =
         | SynExpr.TraitCall(_, _, e, _)
         | SynExpr.YieldOrReturn(_, e, _)
         | SynExpr.YieldOrReturnFrom(_, e, _)
-        | SynExpr.DoBang(e, _)
+        | SynExpr.DoBang(e, _, _)
         | SynExpr.Fixed(e, _)
         | SynExpr.DebugPoint(_, _, e)
         | SynExpr.Paren(e, _, _, _) -> walkExpr e

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fs b/src/Compiler/SyntaxTree/SyntaxTrivia.fs
@@ -117,6 +117,9 @@ type SynExprMatchBangTrivia =
         WithKeyword: range
     }
 
+[<NoEquality; NoComparison>]
+type SynExprDoBangTrivia = { DoBangKeyword: range }
+
 [<NoEquality; NoComparison>]
 type SynExprAnonRecdTrivia = { OpeningBraceRange: range }
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fsi b/src/Compiler/SyntaxTree/SyntaxTrivia.fsi
@@ -169,6 +169,14 @@ type SynExprMatchBangTrivia =
         WithKeyword: range
     }
 
+/// Represents additional information for SynExpr.DoBang
+[<NoEquality; NoComparison>]
+type SynExprDoBangTrivia =
+    {
+        /// The syntax range of the `do!` keyword
+        DoBangKeyword: range
+    }
+
 /// Represents additional information for SynExpr.AnonRecd
 [<NoEquality; NoComparison>]
 type SynExprAnonRecdTrivia =

diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy
@@ -4442,11 +4442,14 @@ declExpr:
 
   | DO_BANG typedSequentialExpr IN opt_OBLOCKSEP typedSequentialExprBlock %prec expr_let
      { let spBind = DebugPointAtBinding.NoneAtDo
-       let trivia: SynExprLetOrUseBangTrivia = { LetOrUseBangKeyword = Range.Zero; EqualsRange = None }
-       SynExpr.LetOrUseBang(spBind, false, true, SynPat.Const(SynConst.Unit, $2.Range), $2, [], $5, unionRanges (rhs parseState 1) $5.Range, trivia) }
+       let trivia: SynExprDoBangTrivia = { DoBangKeyword = rhs parseState 1 }
+       let m = unionRanges (rhs parseState 1) $5.Range
+       SynExpr.DoBang($2, m, trivia) }
 
   | ODO_BANG typedSequentialExprBlock hardwhiteDefnBindingsTerminator %prec expr_let
-     { SynExpr.DoBang($2, unionRanges (rhs parseState 1) $2.Range) }
+     {  let trivia: SynExprDoBangTrivia = { DoBangKeyword = rhs parseState 1 }
+        let m = unionRanges (rhs parseState 1) $2.Range
+        SynExpr.DoBang($2, m, trivia) }
 
   | FIXED declExpr
      { SynExpr.Fixed($2, (unionRanges (rhs parseState 1) $2.Range)) }

diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ComputationExpressionTests.fs
@@ -330,7 +330,7 @@ query {
         |> typecheck
         |> shouldFail
         |> withDiagnostics [
-            (Error 3143, Line 3, Col 5, Line 3, Col 20, "'let!', 'use!' and 'do!' expressions may not be used in queries")
+            (Error 3143, Line 3, Col 5, Line 3, Col 8, "'let!', 'use!' and 'do!' expressions may not be used in queries")
         ]
 
     [<Fact>]
@@ -392,7 +392,7 @@ query {
         |> typecheck
         |> shouldFail
         |> withDiagnostics [
-            (Error 3143, Line 4, Col 5, Line 4, Col 20, "'let!', 'use!' and 'do!' expressions may not be used in queries")  
+            (Error 3143, Line 4, Col 5, Line 4, Col 8, "'let!', 'use!' and 'do!' expressions may not be used in queries")  
         ]
 
     [<Fact>]

diff --git a/...FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl b/...FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl
@@ -7606,7 +7606,13 @@ FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewConst(FSharp.C
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDebugPoint(FSharp.Compiler.Syntax.DebugPointAtLeafExpr, Boolean, FSharp.Compiler.Syntax.SynExpr)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDiscardAfterMissingQualificationAfterDot(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDo(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range)
-FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDoBang(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range)
+FSharp.Compiler.Syntax.SynExpr+DoBang: FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia get_trivia()
+FSharp.Compiler.Syntax.SynExpr+DoBang: FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia trivia
+FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDoBang(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia)
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: FSharp.Compiler.Text.Range DoBangKeyword
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: FSharp.Compiler.Text.Range get_DoBangKeyword()
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: System.String ToString()
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: Void .ctor(FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotGet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Syntax.SynLongIdent, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotIndexedGet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotIndexedSet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)

diff --git a/...harp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl b/...harp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl
@@ -7606,7 +7606,13 @@ FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewConst(FSharp.C
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDebugPoint(FSharp.Compiler.Syntax.DebugPointAtLeafExpr, Boolean, FSharp.Compiler.Syntax.SynExpr)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDiscardAfterMissingQualificationAfterDot(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDo(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range)
-FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDoBang(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range)
+FSharp.Compiler.Syntax.SynExpr+DoBang: FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia get_trivia()
+FSharp.Compiler.Syntax.SynExpr+DoBang: FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia trivia
+FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDoBang(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia)
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: FSharp.Compiler.Text.Range DoBangKeyword
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: FSharp.Compiler.Text.Range get_DoBangKeyword()
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: System.String ToString()
+FSharp.Compiler.SyntaxTrivia.SynExprDoBangTrivia: Void .ctor(FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotGet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Syntax.SynLongIdent, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotIndexedGet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)
 FSharp.Compiler.Syntax.SynExpr: FSharp.Compiler.Syntax.SynExpr NewDotIndexedSet(FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Syntax.SynExpr, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range)

diff --git a/tests/fsharp/typecheck/sigs/neg61.bsl b/tests/fsharp/typecheck/sigs/neg61.bsl
@@ -61,7 +61,7 @@ neg61.fs(92,13,92,70): typecheck error FS3142: 'use' expressions may not be used
 
 neg61.fs(97,13,97,17): typecheck error FS3143: 'let!', 'use!' and 'do!' expressions may not be used in queries
 
-neg61.fs(102,13,102,28): typecheck error FS3143: 'let!', 'use!' and 'do!' expressions may not be used in queries
+neg61.fs(102,13,102,16): typecheck error FS3143: 'let!', 'use!' and 'do!' expressions may not be used in queries
 
 neg61.fs(107,13,107,21): typecheck error FS3144: 'return' and 'return!' may not be used in queries
 

diff --git a/tests/service/data/SyntaxTree/Expression/SynExprDoContainsTheRangeOfTheDoKeyword.fs.bsl b/tests/service/data/SyntaxTree/Expression/SynExprDoContainsTheRangeOfTheDoKeyword.fs.bsl
@@ -14,7 +14,9 @@ ImplFile
                   Named (SynIdent (a, None), false, None, (2,4--2,5)), None,
                   Sequential
                     (SuppressNeither, true, Do (Ident foobar, (3,4--4,14)),
-                     DoBang (Ident foobarBang, (5,4--6,18)), (3,4--6,18),
+                     DoBang
+                       (Ident foobarBang, (5,4--6,18),
+                        { DoBangKeyword = (5,4--5,7) }), (3,4--6,18),
                      { SeparatorRange = None }), (2,4--2,5), NoneAtLet,
                   { LeadingKeyword = Let (2,0--2,3)
                     InlineKeyword = None