FS0049 isn't reported for short names

[auduchinok]

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 -> ()

[auduchinok]

This is the problematic code:

fsharp/src/Compiler/Checking/NameResolution.fs

Line 3395 in 11cb682

&& id.idText.Length >= 3

If there's no objections, I'd like to remove this check.

[edgarfgp]

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

[edgarfgp]

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

[vzarytovskii]

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.

[edgarfgp]

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.

[auduchinok]

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 = ...

[psfinaki]

@edgarfgp just to make sure, has your change here also addressed this ticket?

[edgarfgp]

@psfinaki Yes. we now report pattern(any length) in a match cases.