Skip to content

Fix wrong warning 20 range in sequential expressions (#5735)#19504

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-9141961-29048891-325391ea-3659-4cac-b659-5bc43ada8952
Draft

Fix wrong warning 20 range in sequential expressions (#5735)#19504
Copilot wants to merge 2 commits intomainfrom
copilot/fix-9141961-29048891-325391ea-3659-4cac-b659-5bc43ada8952

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

Fixes the "expression is implicitly ignored" (warning 20) squiggle pointing at the wrong range when the last expression in a sequential block (e.g. inside for, while loops) is non-unit.

Root Cause

In TcStmt, the range m was set to synExpr.Range which, for a sequential expression like a loop body with multiple statements, covered the entire body. When the non-unit type warning was emitted, it used this overly broad range. The bool type case was already handled correctly by ReportImplicitlyIgnoredBoolExpression which walks the typed expression tree, but non-bool types (int, string, etc.) used the wrong range.

Changes Made

  • src/Compiler/Checking/Expressions/CheckExpressions.fs — Added a lastExprRange helper that walks the SynExpr.Sequential chain to find the range of the last expression, and used it in TcStmt instead of synExpr.Range.
  • tests/FSharp.Compiler.ComponentTests/ErrorMessages/WarnExpressionTests.fs — Added 4 regression tests covering for-in, for-to, and while loops with non-unit last expressions (int, string types).
  • docs/release-notes/.FSharp.Compiler.Service/11.0.100.md — Added release note entry.

Testing

  • All 16 Warn Expression tests pass (12 existing + 4 new)
  • Existing CustomAttributes tests unaffected
  • Formatting checks pass

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

@T-Gro
Copy link
Copy Markdown
Member

T-Gro commented Mar 27, 2026

@copilot : Try again

@T-Gro
Copy link
Copy Markdown
Member

T-Gro commented Mar 27, 2026

@copilot : Try again.

@abonie
Copy link
Copy Markdown
Member

abonie commented Mar 31, 2026

@copilot Try again

In TcStmt, use the range of the last expression in a sequential chain
for warnings, so "expression is ignored" diagnostics point at the
offending expression rather than the entire sequential body.

Add regression tests for for-loops and while-loops with non-unit last
expressions (int, string types). Add release notes.

Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/e2b8a05e-e65e-484e-b039-cc8723adb177

Co-authored-by: abonie <20281641+abonie@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 31, 2026

@copilot Try again

Fixed in 36f51b6. The root cause was that TcStmt used synExpr.Range (the entire sequential body range) for warning 20, instead of the range of the last expression in the sequence. The bool case was already handled correctly by ReportImplicitlyIgnoredBoolExpression, but non-bool types (int, string, etc.) used the wrong range. The fix walks the SynExpr.Sequential chain to find the last expression's range. Added 4 regression tests covering for-in, for-to, and while loops.

Copilot AI changed the title [WIP] Copilot Request Fix wrong warning 20 range in sequential expressions (#5735) Mar 31, 2026
Copilot AI requested a review from abonie March 31, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants