Warn if function shadows union case

[Martin521]

Fix for #15559

[Martin521]

@Martin521 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@dotnet-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

[vzarytovskii]

Since it's a potentially quite common case (shadowing in general, and this case in particular), I personally don't think it should be an on-by-default warning even if guarded by language version, since it can break a lot of builds out of the blue.

I think instead, we should have a set of warnings which we can suggest to help people avoid dubious code (this warning included).

Thoughts @T-Gro @KevinRansom @dotnet/fsharp-team-msft

It might be a me thing, will be happy to be proven wrong.

[Martin521]

Yes, tbh I did not expect any intentional shadowing of this particular type, but then found 3 in the compiler code base. All sort of weird though.

[vzarytovskii]

I still think it shouldn't be warning which is on by default, even for next version. Many if not majority of F# projects treat warnings as errors, this will mean that people will get compilation errors just by updating SDK, we try to avoid it as much as possible.

Will wait for others' opinions though.

[smoothdeveloper]

My gut feeling is that the warning by default rubs against the shadowing idioms.

I'd personally prefer it is a non default warning.

edit: the shadowing can be intentional, and it is generally possible to access the union case by using the type qualifier.

[vzarytovskii]

My gut feeling is that the warning by default rubs against the shadowing idioms.

I'd personally prefer it is a non default warning.

edit: the shadowing can be intentional, and it is generally possible to access the union case by using the type qualifier.

Yep, have pretty much the same thoughts.

[Martin521]

Shall I then close this PR in favor of #16064?

[abonie]

Shall I then close this PR in favor of #16064?

I suppose so, I am also in favor of it being off by default

[Martin521]

Closed in favor of #16064