From 5e767e873ba6ba8f76563482c44205e8e6c6ab93 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 9 Apr 2026 14:36:06 +0200 Subject: [PATCH] Fix wrong warning 20 range in sequential expressions (#5735) In TcStmt, the range used for warning 20 was synExpr.Range which, for sequential expressions (e.g. loop bodies with multiple statements), covered the entire body. This made the squiggle highlight the whole block instead of just the offending non-unit expression. Added lastExprRange helper that walks the SynExpr.Sequential chain to find the range of the last expression, so the warning now correctly points at only the expression that produces the ignored value. Added 4 regression tests for for-in, for-to, and while loops. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + .../Checking/Expressions/CheckExpressions.fs | 11 +++- .../ErrorMessages/WarnExpressionTests.fs | 57 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index d5c2087765e..dfdf2f14215 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -3,6 +3,7 @@ * Fix DU case names matching IWSAM member names no longer cause duplicate property entries. (Issue [#14321](https://github.com/dotnet/fsharp/issues/14321), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) * Fix DefaultAugmentation(false) duplicate entry in method table. (Issue [#16565](https://github.com/dotnet/fsharp/issues/16565), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) * Fix abstract event accessors now have SpecialName flag. (Issue [#5834](https://github.com/dotnet/fsharp/issues/5834), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) +* Fix warning 20 ("expression is implicitly ignored") pointing at the wrong range when the last expression in a sequential block (e.g. inside `for`, `while` loops) is non-unit. The squiggle now correctly highlights only the offending expression. ([Issue #5735](https://github.com/dotnet/fsharp/issues/5735), [PR #19504](https://github.com/dotnet/fsharp/pull/19504)) * Fix CLIEvent properties to be correctly recognized as events: `IsEvent` returns `true` and `XmlDocSig` uses `E:` prefix instead of `P:`. ([Issue #10273](https://github.com/dotnet/fsharp/issues/10273), [PR #18584](https://github.com/dotnet/fsharp/pull/18584)) * Fix extra sequence point at the end of match expressions. ([Issue #12052](https://github.com/dotnet/fsharp/issues/12052), [PR #19278](https://github.com/dotnet/fsharp/pull/19278)) * Fix wrong sequence point range for `return`/`yield`/`return!`/`yield!` inside computation expressions. ([Issue #19248](https://github.com/dotnet/fsharp/issues/19248), [PR #19278](https://github.com/dotnet/fsharp/pull/19278)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 635a0dff045..5a6bbf619fa 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -5522,7 +5522,16 @@ and TcStmtThatCantBeCtorBody (cenv: cenv) env tpenv synExpr = and TcStmt (cenv: cenv) env tpenv synExpr = let g = cenv.g let expr, ty, tpenv = TcExprOfUnknownType cenv env tpenv synExpr - let m = synExpr.Range + + // Use the range of the last expression in a sequential chain for warnings, + // so that "expression is ignored" diagnostics point at the offending expression + // rather than the entire sequential body. See https://github.com/dotnet/fsharp/issues/5735 + let rec lastExprRange (e: SynExpr) = + match e with + | SynExpr.Sequential(expr2 = expr2) -> lastExprRange expr2 + | _ -> e.Range + + let m = lastExprRange synExpr let wasUnit = UnifyUnitType cenv env m ty expr if wasUnit then expr, tpenv diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/WarnExpressionTests.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/WarnExpressionTests.fs index 942ab64dfcd..b5cf1bcbccb 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/WarnExpressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/WarnExpressionTests.fs @@ -176,6 +176,63 @@ while x < 1 do |> withSingleDiagnostic (Warning 20, Line 6, Col 5, Line 6, Col 9, "The result of this expression has type 'bool' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.") + // https://github.com/dotnet/fsharp/issues/5735 + [] + let ``Warn On Last Expression In For Loop - int``() = + FSharp """ +module ClassLibrary17 + +for i in 1 .. 10 do + printfn "" + printfn "" |> ignore + 123 + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Warning 20, Line 7, Col 5, Line 7, Col 8, + "The result of this expression has type 'int' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.") + + // https://github.com/dotnet/fsharp/issues/5735 + [] + let ``Warn On Last Expression In For Loop - string``() = + FSharp """ +for i in 1 .. 10 do + printfn "" + "hello" + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Warning 20, Line 4, Col 5, Line 4, Col 12, + "The result of this expression has type 'string' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.") + + // https://github.com/dotnet/fsharp/issues/5735 + [] + let ``Warn On Last Expression In Integer For Loop``() = + FSharp """ +for i = 1 to 10 do + printfn "" + 42 + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Warning 20, Line 4, Col 5, Line 4, Col 7, + "The result of this expression has type 'int' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.") + + // https://github.com/dotnet/fsharp/issues/5735 + [] + let ``Warn On Last Expression In While Loop - non-bool``() = + FSharp """ +let mutable x = 0 +while x < 1 do + printfn "unneeded" + x <- x + 1 + 123 + """ + |> typecheck + |> shouldFail + |> withSingleDiagnostic (Warning 20, Line 6, Col 5, Line 6, Col 8, + "The result of this expression has type 'int' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.") + [] let ``Warn If Possible Property Setter``() = FSharp """