Skip to content

Fix regression for FS0725 - #15874

Merged
vzarytovskii merged 6 commits into
dotnet:mainfrom
edgarfgp:fix-regression-on-FS0725
Aug 28, 2023
Merged

Fix regression for FS0725#15874
vzarytovskii merged 6 commits into
dotnet:mainfrom
edgarfgp:fix-regression-on-FS0725

Conversation

@edgarfgp

@edgarfgpedgarfgp commented Aug 27, 2023

Copy link
Copy Markdown
Contributor

As discovered by failing test on FsAutoComplete in F# 8 we no longer show FS0725 in cases where it is supposed to.

moduleTeststypeA={ X:int }typeB= B ofintmatch None with| None 1->()// FS0725match None with| None (1,2)->()// FS0725match None with| None []->()// FS0725match None with| None [||]->()// FS0725match None with| None { X =1}->()// FS0725match None with| None (B 1)->()// FS0725match None with| None (x, y)->()// FS0725match None with| None false->()// FS0725match None with| None x ->()//FS0725match None with| None _->()// F#7 this compiles. In F#8 this shows FS3548typeC=| C
letmyDiscardedArgFunc(C _)=()// F#7 this compiles. In F#8 this shows FS3548letmyDiscardedArgFunc2(C c)=()// FS0725

PR that introduced the regression #14055

@edgarfgp
edgarfgp requested a review from a team as a code ownerAugust 27, 2023 12:57
@edgarfgp

edgarfgp commented Aug 27, 2023

Copy link
Copy Markdown
ContributorAuthor

On a second look while working on this fix. I think for

match None with| None x ->()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should as it was in F#7typeC=| C
letmyDiscardedArgFunc2(C c)=()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should be as it was in F#7

Any thoughts? cc @vzarytovskii

@vzarytovskii

Copy link
Copy Markdown
Member

On a second look while working on this fix. I think for

match None with| None x ->()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should as it was in F#7typeC=| C
letmyDiscardedArgFunc2(C c)=()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should be as it was in F#7

Any thoughts? cc @vzarytovskii

Yeah, I guess we shouldn't downgrade error to warning here.

@edgarfgp

edgarfgp commented Aug 27, 2023

Copy link
Copy Markdown
ContributorAuthor

On a second look while working on this fix. I think for

match None with| None x ->()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should as it was in F#7typeC=| C
letmyDiscardedArgFunc2(C c)=()// In F#8 this shows Warning FS3548 instead of Error FS0725 ? I think is should be as it was in F#7

Any thoughts? cc @vzarytovskii

Yeah, I guess we shouldn't downgrade error to warning here.

Also the FS3548 error message does not really match what is happening here

Update: FS3548 imo is really about discard _

@edgarfgp

Copy link
Copy Markdown
ContributorAuthor

This is ready

@vzarytovskii
vzarytovskii enabled auto-merge (squash) August 28, 2023 10:37
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.

3 participants

@edgarfgp@vzarytovskii@T-Gro