Skip to content

Better error reporting for return, yield, return! and yield! - #17792

Merged
vzarytovskii merged 34 commits into
dotnet:mainfrom
edgarfgp:better-error-reporting-yield
Oct 7, 2024
Merged

Better error reporting for return, yield, return! and yield!#17792
vzarytovskii merged 34 commits into
dotnet:mainfrom
edgarfgp:better-error-reporting-yield

Conversation

@edgarfgp

@edgarfgpedgarfgp commented Sep 25, 2024

Copy link
Copy Markdown
Contributor

Description

Better error reporting for return, yield, return! and yield!

Continuation of #17779

Before

Screenshot 2024-10-04 at 17 41 30

After

letf1=return[3;4]^^^^^^letf2=return![3;4]^^^^^^^

Before

Screenshot 2024-10-04 at 17 44 53

After

letf3=[iftruethen"a""b"yield![3;4]^^^^^^^^]

Before

Screenshot 2024-10-04 at 17 46 23

After

letf4=async{iftruethenyield"a"^^^^^elseyield"b"}

Before

Screenshot 2024-10-04 at 17 48 55

After

letmaybeTask=task{returnfalse}letindexHandler():Task<string>=task{return! maybeTask
^^^^^^^^^}

Checklist

  • Test cases added
  • Release notes entry updated:

@github-actions

github-actionsBot commented Sep 25, 2024

Copy link
Copy Markdown
Contributor

❗ Release notes required


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/Compilerdocs/release-notes/.FSharp.Compiler.Service/9.0.200.md
vsintegration/srcdocs/release-notes/.VisualStudio/17.12.md

@edgarfgpedgarfgp changed the title Better error reporting for yield and yield!Better error reporting for return, yield, return! and yield!Sep 25, 2024
@edgarfgp

Copy link
Copy Markdown
ContributorAuthor

The failing tests in this PR are the FSharp.Editor.Tests.CodeFixes.RemoveReturnOrYieldTests which are VS specific. Unfortunately I no longer own a Windows machine. @psfinaki would you able to help me with these failing test ?

@psfinaki

Copy link
Copy Markdown
Contributor

Hmm looking at the tests, this change indeed breaks the code fix. Although the code fix itself is quite simple, shouldn't be a big deal, hope I'll get to it later today.

@psfinaki

Copy link
Copy Markdown
Contributor

There you go, there is some change in range/span arithmetic there, added a few extra tests just to be sure :)

@edgarfgp

Copy link
Copy Markdown
ContributorAuthor

There you go, there is some change in range/span arithmetic there, added a few extra tests just to be sure :)

Thank you Sir. Enjoy the holidays :)

@edgarfgpedgarfgp reopened this Sep 27, 2024
@edgarfgpedgarfgp reopened this Sep 27, 2024
@edgarfgpedgarfgp reopened this Sep 30, 2024
@edgarfgp

Copy link
Copy Markdown
ContributorAuthor

This is ready

@edgarfgp
edgarfgp marked this pull request as ready for review October 6, 2024 10:08
@edgarfgp
edgarfgp requested a review from a team as a code ownerOctober 6, 2024 10:08
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@edgarfgp@psfinaki@vzarytovskii@T-Gro