dotnet
FS0049 isn't reported for short names
There's a warning reported for this code, saying that it may intent to use non-imported symbol, but defines a new local value instead:
match 1 with
| One -> ()
It's, however, not reported for shorter names, which may be the case for various literals, union cases, or active patterns:
match 1 with
| On -> ()
This is the problematic code: https://github.com/dotnet/fsharp/blob/11cb682aa5a376c0f8914667a1ed2824b2666c5a/src/Compiler/Checking/NameResolution.fs#L3395
If there's no objections, I'd like to remove this check.
It is a very very very unfortunate historical hack that seems to related the use country and language codes, which are very common in codebases
Removing this check will cause a lot of new warnings/errors in existing codebases.
I did some investigation here https://github.com/dotnet/fsharp/pull/15816/files
I think we should remove this check in the next mayor version and explain the reasoning.
- Users that have this ignored wont notice the difference
- Users with
TreatWarningsAsErrorwill have the option to ignore it
I think we should remove this check in the next mayor version and explain the reasoning.
* Users that have this ignored wont notice the difference * Users with `TreatWarningsAsError` will have the option to ignore it
I still think it's a huge breaking change, before we do it, we need to assess how much code there's in the nuget libraries/github code.
I still think it's a huge breaking change, before we do it, we need to assess how much code there's in the nuget libraries/github code.
Agreed that this is a breaking change. Not sure about the "huge" part. IMO we have done more severe ones like the AttributeTargets or even requiring seq every where will be more impacting for users.
The compiler should have never check for country codes(US etc). If someone wants this they can use/write an analyser.
I still think it's a huge breaking change, before we do it, we need to assess how much code there's in the nuget libraries/github code.
It's worth noting that this warning isn't produced for bindings, so it shouldn't affect symbols exported from libraries. Consider this module:
let Aaa = ()
match () with
| Bbb -> ()
There's no warning for Aaa, only for Bbb, and Bbb isn't exported anywhere, it's a local value.
Let's consider another case with local values:
match f () with
| Us -> ()
| Uk -> ()
...
In this case Us should either resolve to some union case, literal, or active pattern, or there's a chance it's a mistake due to the needed symbol isn't imported. The warning would help here a lot. In addition to that, there can't be several names in different branches which could all match, because the Us pattern makes Uk and others never match.
We can also adjust the shouldWarn check for other cases like this if we want to keep the warning in some cases:
let f Us Uk = ...
