Better error reporting for return, yield, return! and yield! (#17792)

* Better error reporting for `yield` and `yield!` * format code * Update SyntaxTree Tests * release notes * Update baselines * update test * update tests * update tests * update tests * fix * extra tests * fantomas * update tests * update release notes * more tests * more updates * more tests * more tests * more tests * format code * update tests * more tests --------- Co-authored-by: Petr <psfinaki@users.noreply.github.com>
dotnet · Oct 7, 2024 · e017a71 · e017a71
1 parent b187b80
commit e017a71
Show file tree

Hide file tree

Showing 45 changed files with 435 additions and 111 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
@@ -16,6 +16,7 @@
 * Remove non-functional useSyntaxTreeCache option. ([PR #17768](https://github.com/dotnet/fsharp/pull/17768))
 * 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))
+* Better ranges for CE `return, yield, return! and yield!` error reporting. ([PR #17792](https://github.com/dotnet/fsharp/pull/17792))
 * Better ranges for CE `match!`. ([PR #17789](https://github.com/dotnet/fsharp/pull/17789))
 
 ### Breaking Changes
diff --git a/docs/release-notes/.VisualStudio/17.12.md b/docs/release-notes/.VisualStudio/17.12.md
@@ -5,9 +5,10 @@
 ### Added
 
 ### Changed
+* Fix unwanted navigation on hover [PR #17592](https://github.com/dotnet/fsharp/pull/17592)
+* Update `RemoveReturnOrYield` code fix range calculation [PR #17792](https://github.com/dotnet/fsharp/pull/17792)
 * Fix unwanted navigation on hover [PR #17592](https://github.com/dotnet/fsharp/pull/17592))
 * Remove non-functional useSyntaxTreeCache option. ([PR #17768](https://github.com/dotnet/fsharp/pull/17768))
 
-
 ### Breaking Changes
 * Enable FSharp 9.0 Language Version ([Issue #17497](https://github.com/dotnet/fsharp/issues/17438)), [PR](https://github.com/dotnet/fsharp/pull/17500)))
diff --git a/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs b/src/Compiler/Checking/Expressions/CheckComputationExpressions.fs
@@ -1545,7 +1545,9 @@ let rec TryTranslateComputationExpression
 
             let dataCompPrior =
                 translatedCtxt (
-                    TranslateComputationExpressionNoQueryOps ceenv (SynExpr.YieldOrReturn((true, false), varSpaceExpr, mClause))
+                    TranslateComputationExpressionNoQueryOps
+                        ceenv
+                        (SynExpr.YieldOrReturn((true, false), varSpaceExpr, mClause, SynExprYieldOrReturnTrivia.Zero))
                 )
 
             // Rebind using for ...
@@ -1576,7 +1578,9 @@ let rec TryTranslateComputationExpression
                     let isYield = not (customOperationMaintainsVarSpaceUsingBind ceenv nm)
 
                     translatedCtxt (
-                        TranslateComputationExpressionNoQueryOps ceenv (SynExpr.YieldOrReturn((isYield, false), varSpaceExpr, mClause))
+                        TranslateComputationExpressionNoQueryOps
+                            ceenv
+                            (SynExpr.YieldOrReturn((isYield, false), varSpaceExpr, mClause, SynExprYieldOrReturnTrivia.Zero))
                     )
 
                 // Now run the consumeCustomOpClauses
@@ -2374,7 +2378,7 @@ let rec TryTranslateComputationExpression
 
             Some(translatedCtxt callExpr)
 
-        | SynExpr.YieldOrReturnFrom((true, _), synYieldExpr, m) ->
+        | SynExpr.YieldOrReturnFrom((true, _), synYieldExpr, _, { YieldOrReturnFromKeyword = m }) ->
             let yieldFromExpr =
                 mkSourceExpr synYieldExpr ceenv.sourceMethInfo ceenv.builderValName
 
@@ -2392,7 +2396,8 @@ let rec TryTranslateComputationExpression
             then
                 error (Error(FSComp.SR.tcRequireBuilderMethod ("YieldFrom"), m))
 
-            let yieldFromCall = mkSynCall "YieldFrom" m [ yieldFromExpr ] ceenv.builderValName
+            let yieldFromCall =
+                mkSynCall "YieldFrom" synYieldExpr.Range [ yieldFromExpr ] ceenv.builderValName
 
             let yieldFromCall =
                 if IsControlFlowExpression synYieldExpr then
@@ -2402,7 +2407,7 @@ let rec TryTranslateComputationExpression
 
             Some(translatedCtxt yieldFromCall)
 
-        | SynExpr.YieldOrReturnFrom((false, _), synReturnExpr, m) ->
+        | SynExpr.YieldOrReturnFrom((false, _), synReturnExpr, _, { YieldOrReturnFromKeyword = m }) ->
             let returnFromExpr =
                 mkSourceExpr synReturnExpr ceenv.sourceMethInfo ceenv.builderValName
 
@@ -2424,7 +2429,7 @@ let rec TryTranslateComputationExpression
                 error (Error(FSComp.SR.tcRequireBuilderMethod ("ReturnFrom"), m))
 
             let returnFromCall =
-                mkSynCall "ReturnFrom" m [ returnFromExpr ] ceenv.builderValName
+                mkSynCall "ReturnFrom" synReturnExpr.Range [ returnFromExpr ] ceenv.builderValName
 
             let returnFromCall =
                 if IsControlFlowExpression synReturnExpr then
@@ -2434,7 +2439,7 @@ let rec TryTranslateComputationExpression
 
             Some(translatedCtxt returnFromCall)
 
-        | SynExpr.YieldOrReturn((isYield, _), synYieldOrReturnExpr, m) ->
+        | SynExpr.YieldOrReturn((isYield, _), synYieldOrReturnExpr, _, { YieldOrReturnKeyword = m }) ->
             let methName = (if isYield then "Yield" else "Return")
 
             if ceenv.isQuery && not isYield then
@@ -2452,10 +2457,10 @@ let rec TryTranslateComputationExpression
                         ceenv.builderTy
                 )
             then
-                error (Error(FSComp.SR.tcRequireBuilderMethod (methName), m))
+                error (Error(FSComp.SR.tcRequireBuilderMethod methName, m))
 
             let yieldOrReturnCall =
-                mkSynCall methName m [ synYieldOrReturnExpr ] ceenv.builderValName
+                mkSynCall methName synYieldOrReturnExpr.Range [ synYieldOrReturnExpr ] ceenv.builderValName
 
             let yieldOrReturnCall =
                 if IsControlFlowExpression synYieldOrReturnExpr then
@@ -2759,7 +2764,7 @@ and TranslateComputationExpressionBind
 /// The inner option indicates if a custom operation is involved inside
 and convertSimpleReturnToExpr (ceenv: ComputationExpressionContext<'a>) comp varSpace innerComp =
     match innerComp with
-    | SynExpr.YieldOrReturn((false, _), returnExpr, m) ->
+    | SynExpr.YieldOrReturn((false, _), returnExpr, m, _) ->
         let returnExpr = SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes m, false, returnExpr)
         Some(returnExpr, None)
 
@@ -2901,7 +2906,7 @@ and TranslateComputationExpression (ceenv: ComputationExpressionContext<'a>) fir
                         with
                         | minfo :: _ when MethInfoHasAttribute ceenv.cenv.g m ceenv.cenv.g.attrib_DefaultValueAttribute minfo ->
                             SynExpr.ImplicitZero m
-                        | _ -> SynExpr.YieldOrReturn((false, true), SynExpr.Const(SynConst.Unit, m), m)
+                        | _ -> SynExpr.YieldOrReturn((false, true), SynExpr.Const(SynConst.Unit, m), m, SynExprYieldOrReturnTrivia.Zero)
 
                 let letBangBind =
                     SynExpr.LetOrUseBang(

diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs
@@ -5991,16 +5991,16 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
         CallExprHasTypeSink cenv.tcSink (m, env.NameEnv, overallTy.Commit, env.AccessRights)
         TcQuotationExpr cenv overallTy env tpenv (oper, raw, ast, isFromQueryExpression, m)
 
-    | SynExpr.YieldOrReturn ((isTrueYield, _), _, m)
-    | SynExpr.YieldOrReturnFrom ((isTrueYield, _), _, m) when isTrueYield ->
+    | SynExpr.YieldOrReturn ((isTrueYield, _), _, _m, { YieldOrReturnKeyword = m })
+    | SynExpr.YieldOrReturnFrom ((isTrueYield, _), _, _m, { YieldOrReturnFromKeyword = m }) when isTrueYield ->
         error(Error(FSComp.SR.tcConstructRequiresListArrayOrSequence(), m))
 
-    | SynExpr.YieldOrReturn ((_, isTrueReturn), _, m)
-    | SynExpr.YieldOrReturnFrom ((_, isTrueReturn), _, m) when isTrueReturn ->
+    | SynExpr.YieldOrReturn ((_, isTrueReturn), _, _m, { YieldOrReturnKeyword = m })
+    | SynExpr.YieldOrReturnFrom ((_, isTrueReturn), _, _m, { YieldOrReturnFromKeyword = m }) when isTrueReturn ->
         error(Error(FSComp.SR.tcConstructRequiresComputationExpressions(), m))
 
-    | SynExpr.YieldOrReturn (_, _, m)
-    | SynExpr.YieldOrReturnFrom (_, _, m)
+    | SynExpr.YieldOrReturn (trivia = { YieldOrReturnKeyword = m })
+    | SynExpr.YieldOrReturnFrom (trivia = { YieldOrReturnFromKeyword = m })
     | SynExpr.ImplicitZero m ->
         error(Error(FSComp.SR.tcConstructRequiresSequenceOrComputations(), m))
 

diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs
@@ -353,43 +353,44 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
 
             Some(combinatorExpr, tpenv)
 
-        | SynExpr.YieldOrReturnFrom((isYield, _), synYieldExpr, m) ->
+        | SynExpr.YieldOrReturnFrom(flags = (isYield, _); expr = synYieldExpr; trivia = { YieldOrReturnFromKeyword = m }) ->
             let env = { env with eIsControlFlow = false }
             let resultExpr, genExprTy, tpenv = TcExprOfUnknownType cenv env tpenv synYieldExpr
 
             if not isYield then
                 errorR (Error(FSComp.SR.tcUseYieldBangForMultipleResults (), m))
 
-            AddCxTypeMustSubsumeType ContextInfo.NoContext env.DisplayEnv cenv.css m NoTrace genOuterTy genExprTy
+            AddCxTypeMustSubsumeType ContextInfo.NoContext env.DisplayEnv cenv.css synYieldExpr.Range NoTrace genOuterTy genExprTy
 
-            let resultExpr = mkCoerceExpr (resultExpr, genOuterTy, m, genExprTy)
+            let resultExpr =
+                mkCoerceExpr (resultExpr, genOuterTy, synYieldExpr.Range, genExprTy)
 
             let resultExpr =
                 if IsControlFlowExpression synYieldExpr then
                     resultExpr
                 else
-                    mkDebugPoint m resultExpr
+                    mkDebugPoint resultExpr.Range resultExpr
 
             Some(resultExpr, tpenv)
 
-        | SynExpr.YieldOrReturn((isYield, _), synYieldExpr, m) ->
+        | SynExpr.YieldOrReturn(flags = (isYield, _); expr = synYieldExpr; trivia = { YieldOrReturnKeyword = m }) ->
             let env = { env with eIsControlFlow = false }
             let genResultTy = NewInferenceType g
 
             if not isYield then
                 errorR (Error(FSComp.SR.tcSeqResultsUseYield (), m))
 
-            UnifyTypes cenv env m genOuterTy (mkSeqTy cenv.g genResultTy)
+            UnifyTypes cenv env synYieldExpr.Range genOuterTy (mkSeqTy cenv.g genResultTy)
 
             let resultExpr, tpenv = TcExprFlex cenv flex true genResultTy env tpenv synYieldExpr
 
-            let resultExpr = mkCallSeqSingleton cenv.g m genResultTy resultExpr
+            let resultExpr = mkCallSeqSingleton cenv.g synYieldExpr.Range genResultTy resultExpr
 
             let resultExpr =
                 if IsControlFlowExpression synYieldExpr then
                     resultExpr
                 else
-                    mkDebugPoint m resultExpr
+                    mkDebugPoint synYieldExpr.Range resultExpr
 
             Some(resultExpr, tpenv)
 

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

diff --git a/src/Compiler/Service/ServiceStructure.fs b/src/Compiler/Service/ServiceStructure.fs
@@ -246,11 +246,11 @@ module Structure =
                 rcheck Scope.New Collapse.Below r e.Range
                 parseExpr e
 
-            | SynExpr.YieldOrReturn(_, e, r) ->
+            | SynExpr.YieldOrReturn(_, e, r, _) ->
                 rcheck Scope.YieldOrReturn Collapse.Below r r
                 parseExpr e
 
-            | SynExpr.YieldOrReturnFrom(_, e, r) ->
+            | SynExpr.YieldOrReturnFrom(_, e, r, _) ->
                 rcheck Scope.YieldOrReturnBang Collapse.Below r r
                 parseExpr e
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fs b/src/Compiler/SyntaxTree/SyntaxTree.fs
@@ -704,9 +704,9 @@ type SynExpr =
 
     | SequentialOrImplicitYield of debugPoint: DebugPointAtSequential * expr1: SynExpr * expr2: SynExpr * ifNotStmt: SynExpr * range: range
 
-    | YieldOrReturn of flags: (bool * bool) * expr: SynExpr * range: range
+    | YieldOrReturn of flags: (bool * bool) * expr: SynExpr * range: range * trivia: SynExprYieldOrReturnTrivia
 
-    | YieldOrReturnFrom of flags: (bool * bool) * expr: SynExpr * range: range
+    | YieldOrReturnFrom of flags: (bool * bool) * expr: SynExpr * range: range * trivia: SynExprYieldOrReturnFromTrivia
 
     | LetOrUseBang of
         bindDebugPoint: DebugPointAtBinding *

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fsi b/src/Compiler/SyntaxTree/SyntaxTree.fsi
@@ -877,12 +877,12 @@ type SynExpr =
     /// F# syntax: yield expr
     /// F# syntax: return expr
     /// Computation expressions only
-    | YieldOrReturn of flags: (bool * bool) * expr: SynExpr * range: range
+    | YieldOrReturn of flags: (bool * bool) * expr: SynExpr * range: range * trivia: SynExprYieldOrReturnTrivia
 
     /// F# syntax: yield! expr
     /// F# syntax: return! expr
     /// Computation expressions only
-    | YieldOrReturnFrom of flags: (bool * bool) * expr: SynExpr * range: range
+    | YieldOrReturnFrom of flags: (bool * bool) * expr: SynExpr * range: range * trivia: SynExprYieldOrReturnFromTrivia
 
     /// F# syntax: let! pat = expr in expr
     /// F# syntax: use! pat = expr in expr

diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs
@@ -883,8 +883,8 @@ let rec synExprContainsError inpExpr =
         | SynExpr.InferredDowncast(e, _)
         | SynExpr.Lazy(e, _)
         | SynExpr.TraitCall(_, _, e, _)
-        | SynExpr.YieldOrReturn(_, e, _)
-        | SynExpr.YieldOrReturnFrom(_, e, _)
+        | SynExpr.YieldOrReturn(_, e, _, _)
+        | SynExpr.YieldOrReturnFrom(_, e, _, _)
         | SynExpr.DoBang(e, _, _)
         | SynExpr.Fixed(e, _)
         | SynExpr.DebugPoint(_, _, e)

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fs b/src/Compiler/SyntaxTree/SyntaxTrivia.fs
@@ -117,6 +117,25 @@ type SynExprMatchBangTrivia =
         WithKeyword: range
     }
 
+[<NoEquality; NoComparison>]
+type SynExprYieldOrReturnTrivia =
+    {
+        YieldOrReturnKeyword: range
+    }
+
+    static member Zero: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = Range.Zero }
+
+[<NoEquality; NoComparison>]
+type SynExprYieldOrReturnFromTrivia =
+    {
+        YieldOrReturnFromKeyword: range
+    }
+
+    static member Zero: SynExprYieldOrReturnFromTrivia =
+        {
+            YieldOrReturnFromKeyword = Range.Zero
+        }
+
 [<NoEquality; NoComparison>]
 type SynExprDoBangTrivia = { DoBangKeyword: range }
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fsi b/src/Compiler/SyntaxTree/SyntaxTrivia.fsi
@@ -177,6 +177,24 @@ type SynExprDoBangTrivia =
         DoBangKeyword: range
     }
 
+/// Represents additional information for SynExpr.YieldOrReturn
+[<NoEquality; NoComparison>]
+type SynExprYieldOrReturnTrivia =
+    {
+        /// The syntax range of the `yield` or `return` keyword.
+        YieldOrReturnKeyword: range
+    }
+
+    static member Zero: SynExprYieldOrReturnTrivia
+
+/// Represents additional information for SynExpr.YieldOrReturnFrom
+[<NoEquality; NoComparison>]
+type SynExprYieldOrReturnFromTrivia =
+    {
+        /// The syntax range of the `yield!` or `return!` keyword.
+        YieldOrReturnFromKeyword: range
+    }
+
 /// Represents additional information for SynExpr.AnonRecd
 [<NoEquality; NoComparison>]
 type SynExprAnonRecdTrivia =

diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy
@@ -4402,18 +4402,22 @@ declExpr:
         exprFromParseError (SynExpr.ForEach(spFor, spIn, SeqExprOnly false, true, $2, arbExpr ("forLoopCollection", mFor), arbExpr ("forLoopBody3", mForLoopBodyArb), mForLoopAll)) }
 
   | YIELD declExpr
-     { SynExpr.YieldOrReturn(($1, not $1), $2, unionRanges (rhs parseState 1) $2.Range) }
+     { let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = rhs parseState 1 }
+       SynExpr.YieldOrReturn(($1, not $1), $2, (unionRanges (rhs parseState 1) $2.Range), trivia) }
 
   | YIELD_BANG declExpr
-     { SynExpr.YieldOrReturnFrom(($1, not $1), $2, unionRanges (rhs parseState 1) $2.Range) }
+     {  let trivia: SynExprYieldOrReturnFromTrivia = { YieldOrReturnFromKeyword = rhs parseState 1 }
+        SynExpr.YieldOrReturnFrom(($1, not $1), $2, (unionRanges (rhs parseState 1) $2.Range), trivia) }
 
   | YIELD recover
      { let mYieldAll = rhs parseState 1
-       SynExpr.YieldOrReturn(($1, not $1), arbExpr ("yield", mYieldAll), mYieldAll) }
+       let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = rhs parseState 1 }
+       SynExpr.YieldOrReturn(($1, not $1), arbExpr ("yield", mYieldAll), mYieldAll, trivia) }
 
   | YIELD_BANG recover
      { let mYieldAll = rhs parseState 1
-       SynExpr.YieldOrReturnFrom(($1, not $1), arbExpr ("yield!", mYieldAll), mYieldAll) }
+       let trivia: SynExprYieldOrReturnFromTrivia = { YieldOrReturnFromKeyword = rhs parseState 1 }
+       SynExpr.YieldOrReturnFrom(($1, not $1), arbExpr ("yield!", mYieldAll), mYieldAll, trivia) }
 
   | BINDER headBindingPattern EQUALS typedSequentialExprBlock IN opt_OBLOCKSEP moreBinders typedSequentialExprBlock %prec expr_let
      { let spBind = DebugPointAtBinding.Yes(rhs2 parseState 1 5)
@@ -4458,7 +4462,8 @@ declExpr:
      { errorR(Error(FSComp.SR.parsArrowUseIsLimited(), lhs parseState))
        let mArrow = rhs parseState 1
        let expr = $2 mArrow
-       SynExpr.YieldOrReturn((true, true), expr, (unionRanges mArrow expr.Range)) }
+       let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = rhs parseState 1 }
+       SynExpr.YieldOrReturn((true, true), expr, (unionRanges mArrow expr.Range), trivia) }
 
   | declExpr COLON_QMARK typ
       { SynExpr.TypeTest($1, $3, unionRanges $1.Range $3.Range) }
@@ -5456,7 +5461,8 @@ arrowThenExprR:
   | RARROW typedSequentialExprBlockR
       { let mArrow = rhs parseState 1
         let expr = $2 mArrow
-        SynExpr.YieldOrReturn((true, false), expr, unionRanges mArrow expr.Range) }
+        let trivia: SynExprYieldOrReturnTrivia = { YieldOrReturnKeyword = mArrow }
+        SynExpr.YieldOrReturn((true, false), expr, (unionRanges mArrow expr.Range), trivia) }
 
 forLoopBinder:
   | parenPattern IN declExpr

diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TypeMismatchTests.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/TypeMismatchTests.fs
@@ -347,7 +347,7 @@ let f4 =
         |> withDiagnostics [
             (Error 1,   Line 6,  Col 9,  Line 6,  Col 16, "This expression was expected to have type\n    'int'    \nbut here has type\n    'string'    ")
             (Error 1,   Line 12, Col 13, Line 12, Col 16, "This expression was expected to have type\n    'int'    \nbut here has type\n    'string'    ")
-            (Error 193,   Line 21, Col 9,  Line 21, Col 24, "Type constraint mismatch. The type \n    'int list'    \nis not compatible with type\n    'string seq'    \n")
+            (Error 193, Line 21, Col 16,  Line 21, Col 24, "Type constraint mismatch. The type \n    'int list'    \nis not compatible with type\n    'string seq'    \n")
             (Error 1,   Line 28, Col 9,  Line 28, Col 12, "This expression was expected to have type\n    'int64'    \nbut here has type\n    'float'    ")
         ]