Fsharp: `GetDeclaredItems` returns wrong symbol inside non-recursive declaration

[auduchinok]

Consider the following code:

module Module

let id =
    {caret}id

When trying to get a symbol for id at the caret location using GetSymbolUseAtLocation it's expected to get FSharp.Core.Operators.Id result since Module.id is not recursive, however Module.id is returned.

It works this way due to the following: GetDeclaredItems takes known name resolution environments at the requested range (GetBestEnvForPos) and tries to resolve requested qualified identifier using the best possible one.

It works properly during type-check itself: when the right hand expression is being type-checked the new id is not yet present in the environment, thus, the id reference expression is resolved to the existing id. After the successful check the new id is added to the environment making the existing one shadowed. Since the same environment is later used by GetDeclaredItems at the range we get the new id instead of the FSharp.Core one. It's used in FCS features like getting completion lookup items which all get the wrong symbol.

[abelbraaksma]

Can this be the cause that jumping to the definition goes wrong if you have same named identifiers?

I mean, it also fails when you have a module and a type with the same name in scope. In fact, the tooltip will show both declarations.

[auduchinok]

@abelbraaksma It sounds related but most likely there's a separate bug somewhere as, for what you're describing, it seems that CallNameResolutionSink is called for both same-named symbols on the same range despite it successfully resolves to a single one.

Here's a repro by @vasily-kirichenko where we get both symbols from GetAllUsesOfAllSymbolsInFile on the same range and IIRC it uses CallNameResolutionSink results: ConsoleApp15/Program.fs#L8.

And there're two tooltips too:

[cartermp]

@auduchinok in that example, do the symbols also have the same RangeAlternate?

[auduchinok]

@cartermp Yes, the range is the same there in the Yes example, it's captured on the first screenshot. It seems the name resolution actually works according to the spec here. It's calling CallNameResolutionSink this way what makes the tooling behave in a wrong way. I'll open another issue for this problem when find out more details.

[dsyme]

This should absolutely be fixed