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

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
@T-Gro T-Gro force-pushed the copilot/fix-9141961-29048891-325391ea-3659-4cac-b659-5bc43ada8952 branch from 36f51b6 to c4ebece Compare April 9, 2026 12:36
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@T-Gro T-Gro force-pushed the copilot/fix-9141961-29048891-325391ea-3659-4cac-b659-5bc43ada8952 branch from c4ebece to 1b60166 Compare April 9, 2026 13:40
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>
@T-Gro T-Gro force-pushed the copilot/fix-9141961-29048891-325391ea-3659-4cac-b659-5bc43ada8952 branch from e0f0b54 to e5198db Compare April 9, 2026 16:08
FSharp.Core 11.0.100 and FSharp.Compiler.Service 43.12.100 are already
published on NuGet, causing Check_Published_Package_Versions CI checks
to fail. Bump to 101 to match other active PRs targeting main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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